Fix issues with Auto Shadow Spell

src/map/clif.c

@@ -20828,39 +20828,57 @@ static int clif_poison_list(struct map_session_data *sd, uint16 skill_lv)
 
 	return 1;
 }
+
 static int clif_autoshadowspell_list(struct map_session_data *sd)
 {
-	int fd, i, c;
 	nullpo_ret(sd);
-	fd = sd->fd;
-	if( !fd ) return 0;
 
-	if( sd->menuskill_id == SC_AUTOSHADOWSPELL )
+	int fd = sd->fd;
+	if (fd == 0)
+		return 0;
+
+	if (sd->menuskill_id == SC_AUTOSHADOWSPELL)
 		return 0;
 
-	WFIFOHEAD(fd, 2 * 6 + 4);
-	WFIFOW(fd,0) = 0x442;
-	for (i = 0, c = 0; i < MAX_SKILL_DB; i++)
+	// Max number of skills shown. This number should never go above 2, but let's leave some space for customization.
+	const int max_count = 10;
+
+	struct PACKET_ZC_SKILL_SELECT_REQUEST *p;
+	int len = max_count * sizeof(*p->skillIds);
+	WFIFOHEAD(fd, len);
+	p = WFIFOP(fd, 0);
+	p->packetType = HEADER_ZC_SKILL_SELECT_REQUEST;
+
+	int c = 0;
+	for (int i = 0; i < MAX_SKILL_DB && c < max_count; i++) {
 		if (sd->status.skill[i].flag == SKILL_FLAG_PLAGIARIZED && sd->status.skill[i].id > 0 && sd->status.skill[i].id < GS_GLITTERING
 		    && skill->get_type(sd->status.skill[i].id, sd->status.skill[i].lv) == BF_MAGIC) {
 			// Can't auto cast both Extended class and 3rd class skills.
-			WFIFOW(fd,8+c*2) = sd->status.skill[i].id;
+			p->skillIds[c] = sd->status.skill[i].id;
 			c++;
 		}
+	}
 
-	if( c > 0 ) {
-		WFIFOW(fd,2) = 8 + c * 2;
-		WFIFOL(fd,4) = c;
-		WFIFOSET(fd,WFIFOW(fd,2));
+	if (c == max_count)
+		ShowError("%s: max_count shadow spells was reached, some skills may not be shown.\n", __func__);
+
+	if (c > 0) {
 		sd->menuskill_id = SC_AUTOSHADOWSPELL;
 		sd->menuskill_val = c;
+
+		len = c * sizeof(*p->skillIds) + sizeof(*p);
+		p->packetLength = len;
+		p->flag = 1; // 1 = auto shadow spell
+
+		WFIFOSET(fd, len);
 	} else {
 		status_change_end(&sd->bl,SC_STOP,INVALID_TIMER);
 		clif->skill_fail(sd, SC_AUTOSHADOWSPELL, USESKILL_FAIL_IMITATION_SKILL_NONE, 0, 0);
 	}
 
 	return 1;
 }
+
 /*===========================================
  * Skill list for Four Elemental Analysis
  * and Change Material skills.
@@ -20910,7 +20928,21 @@ static void clif_parse_SkillSelectMenu(int fd, struct map_session_data *sd)
 		return;
 	}
 
-	skill->select_menu(sd,RFIFOW(fd,6));
+	const struct PACKET_CZ_SKILL_SELECT_RESPONSE *p = RP2PTR(fd);
+
+	/* selectedSkillId is 0 when cancel is clicked.
+	 *
+	 * Some clients (observed in PACKETVER < 2020) sends random skill ids if you click "ok"
+	 * without selecting a skill. This check prevents the skill logic from running and generating bad reports.
+	 */
+	if (p->selectedSkillId == 0 || skill->get_index_sub(p->selectedSkillId, false) == 0) {
+		status_change_end(&sd->bl, SC_STOP, INVALID_TIMER);
+		clif->skill_fail(sd, sd->ud.skill_id, 0, 0, 0);
+		clif_menuskill_clear(sd);
+		return;
+	}
+
+	skill->select_menu(sd, p->selectedSkillId);
 
 	clif_menuskill_clear(sd);
 }

src/map/packets_struct.h

@@ -3319,6 +3319,20 @@ struct PACKET_ZC_MAKINGARROW_LIST {
 } __attribute__((packed));
 DEFINE_PACKET_HEADER(ZC_MAKINGARROW_LIST, 0x01ad);
 
+struct PACKET_ZC_SKILL_SELECT_REQUEST {
+	int16 packetType;
+	int16 packetLength;
+	int32 flag; //< 0 = old code compatibility; 1 = Auto Shadow Spell; same value is received in CZ_SKILL_SELECT_RESPONSE
+	int16 skillIds[];
+} __attribute__((packed));
+DEFINE_PACKET_HEADER(ZC_SKILL_SELECT_REQUEST, 0x0442);
+
+struct PACKET_CZ_SKILL_SELECT_RESPONSE {
+	int16 packetType;
+	int32 flag; //< currently unused, matches ZC_SKILL_SELECT_REQUEST.flag
+	int16 selectedSkillId;
+} __attribute__((packed));
+
 #if PACKETVER_MAIN_NUM >= 20200916 || PACKETVER_RE_NUM >= 20200723 || PACKETVER_ZERO_NUM >= 20221024
 #define REPAIRITEM_INFO REPAIRITEM_INFO2
 struct PACKET_ZC_REPAIRITEMLIST {
@@ -4768,7 +4782,7 @@ struct PACKET_ZC_NOTIFY_SKILL {
 	int8 action;
 } __attribute__((packed));
 DEFINE_PACKET_HEADER(ZC_NOTIFY_SKILL, 0x01de);
-#endif 
+#endif
 
 #if PACKETVER_MAIN_NUM >= 20130731 || PACKETVER_RE_NUM >= 20130724 || defined(PACKETVER_ZERO)
 struct PACKET_ZC_USE_SKILL {

src/map/skill.c

@@ -124,16 +124,23 @@ static int skill_name2id(const char *name)
 	return strdb_iget(skill->name2id_db, name);
 }
 
-/// Maps skill ids to skill db offsets.
-/// Returns the skill's array index, or 0 (Unknown Skill).
-static int skill_get_index(int skill_id)
+/**
+ * Maps skill ids to skill db offsets.
+ *
+ * @param  skill_id      skill to search
+ * @param  report_errors if the skill is not found, report an error to help solving it?
+ * @return Returns the skill's array index, or 0 (Unknown Skill).
+ */
+static int skill_get_index_sub(int skill_id, bool report_errors)
 {
 	int length = ARRAYLENGTH(skill_idx_ranges);
 
 
 	if (skill_id < skill_idx_ranges[0].start || skill_id > skill_idx_ranges[length - 1].end) {
-		ShowWarning("skill_get_index: skill id '%d' is not being handled!\n", skill_id);
-		Assert_report(0);
+		if (report_errors) {
+			ShowWarning("skill_get_index: skill id '%d' is not being handled!\n", skill_id);
+			Assert_report(0);
+		}
 		return 0;
 	}
 
@@ -152,19 +159,35 @@ static int skill_get_index(int skill_id)
 	}
 
 	if (!found) {
-		ShowWarning("skill_get_index: skill id '%d' (idx: %d) is not handled as it lies outside the defined ranges!\n", skill_id, skill_idx);
-		Assert_report(0);
+		if (report_errors) {
+			ShowWarning("skill_get_index: skill id '%d' (idx: %d) is not handled as it lies outside the defined ranges!\n", skill_id, skill_idx);
+			Assert_report(0);
+		}
 		return 0;
 	}
 	if (skill_idx >= MAX_SKILL_DB) {
-		ShowWarning("skill_get_index: skill id '%d'(idx: %d) is not being handled as it exceeds MAX_SKILL_DB!\n", skill_id, skill_idx);
-		Assert_report(0);
+		if (report_errors) {
+			ShowWarning("skill_get_index: skill id '%d'(idx: %d) is not being handled as it exceeds MAX_SKILL_DB!\n", skill_id, skill_idx);
+			Assert_report(0);
+		}
 		return 0;
 	}
 
 	return skill_idx;
 }
 
+/**
+ * Maps skill ids to skill db offsets.
+ * If something goes wrong, errors will be reported to console.
+ *
+ * @param  skill_id      skill to search not found, report an error to help solving it?
+ * @return Returns the skill's array index, or 0 (Unknown Skill).
+ */
+static int skill_get_index(int skill_id)
+{
+	return skill->get_index_sub(skill_id, true);
+}
+
 static const char *skill_get_name(int skill_id)
 {
 	return skill->dbs->db[skill->get_index(skill_id)].name;
@@ -10445,15 +10468,19 @@ static int skill_castend_nodamage_id(struct block_list *src, struct block_list *
 			}
 			break;
 		case SC_AUTOSHADOWSPELL:
-			if( sd ) {
-				int idx1 = skill->get_index(sd->reproduceskill_id), idx2 = skill->get_index(sd->cloneskill_id);
-				if( sd->status.skill[idx1].id || sd->status.skill[idx2].id ) {
+			if (sd != NULL) {
+				int reproduceIdx = sd->reproduceskill_id > 0 ? skill->get_index(sd->reproduceskill_id) : -1;
+				int cloneIdx = sd->cloneskill_id > 0 ? skill->get_index(sd->cloneskill_id) : -1;
+
+				bool hasReproduceSkill = reproduceIdx >= 0 && sd->status.skill[reproduceIdx].id != 0;
+				bool hasCloneSkill = cloneIdx >= 0 && sd->status.skill[cloneIdx].id != 0;
+				if (hasReproduceSkill || hasCloneSkill) {
 					sc_start(src, src, SC_STOP, 100, skill_lv, INFINITE_DURATION, skill_id); // The skill_lv is stored in val1 used in skill_select_menu to determine the used skill lvl [Xazax]
 					clif->autoshadowspell_list(sd);
-					clif->skill_nodamage(src,bl,skill_id,1,1);
-				}
-				else
+					clif->skill_nodamage(src, bl, skill_id, 1, 1);
+				} else {
 					clif->skill_fail(sd, skill_id, USESKILL_FAIL_IMITATION_SKILL_NONE, 0, 0);
+				}
 			}
 			break;
 
@@ -25195,6 +25222,7 @@ void skill_defaults(void)
 	skill->unit_group_newid = 0;
 	/* accessors */
 	skill->get_index = skill_get_index;
+	skill->get_index_sub = skill_get_index_sub;
 	skill->get_type = skill_get_type;
 	skill->get_hit = skill_get_hit;
 	skill->get_inf = skill_get_inf;

src/map/skill.h

@@ -143,7 +143,7 @@ enum e_skill_inf2 {
 	INF2_HIDDEN_TRAP        = 0x00080000, ///< Traps that are hidden (based on trap_visiblity battle conf)
 	INF2_IS_COMBO_SKILL     = 0x00100000, ///< Sets whether a skill can be used in combos or not
 	INF2_NO_STASIS          = 0x00200000,
-	INF2_NO_KAGEHUMI        = 0x00400000, 
+	INF2_NO_KAGEHUMI        = 0x00400000,
 	INF2_RANGE_VULTURE      = 0x00800000, ///< Range is modified by AC_VULTURE
 	INF2_RANGE_SNAKEEYE     = 0x01000000, ///< Range is modified by GS_SNAKEEYE
 	INF2_RANGE_SHADOWJUMP   = 0x02000000, ///< Range is modified by NJ_SHADOWJUMP
@@ -2067,6 +2067,7 @@ struct skill_interface {
 	int unit_group_newid;
 	/* accesssors */
 	int (*get_index) (int skill_id);
+	int (*get_index_sub) (int skill_id, bool report_errors);
 	int (*get_type) (int skill_id, int skill_lv);
 	int (*get_hit) (int skill_id, int skill_lv);
 	int (*get_inf) (int skill_id);

src/map/status.c

@@ -7418,6 +7418,7 @@ static int status_change_start_sub(struct block_list *src, struct block_list *bl
 			case SC_RESIST_PROPERTY_WIND:
 			case SC_FLASHKICK:
 			case SC_SOULUNITY:
+			case SC__AUTOSHADOWSPELL: // otherwise you can't change your shadow spell to a lower skill_id
 				break;
 			case SC_GOSPEL:
 				//Must not override a casting gospel char.