Fixed skill conditions check and <flag> parameter in itemskill() script command.

 * itemskill() script command should check for the skill's conditions and also consumes them. SP are not consumed.
 * The same applies to Hocus-pocus skill. Conditions should be checked and consumed, SP are not consumed.
 * This was bugged for more than 6 years now. See linked bug report and commits.

Related bug:
 * https://herc.ws/oldboard/tracker/issue-7210-itemskill-command-does-not-check-for-required-items/

Related commits:
 * https://github.com/HerculesWS/Hercules/commit/b864056b8d088660fca9129bddad477732ed8df9
 * https://github.com/HerculesWS/Hercules/commit/07272f7a16db87970583286db03167ca79604a69

5 files changed, 98 insertions, 49 deletions

diff --git a/src/map/pc.h b/src/map/pc.h
index a3a3ee48d..4d7a7eef0 100644
--- a/src/map/pc.h
+++ b/src/map/pc.h
@@ -240,6 +240,8 @@ struct map_session_data {
 		unsigned int refine_ui : 1;
 		unsigned int npc_unloaded : 1; ///< The player is talking with an unloaded NPCs (respawned tombstones)
 		unsigned int lapine_ui : 1;
+		unsigned int itemskill_conditions_checked : 1; // Used by itemskill() script command, to prevent second check of conditions after target was selected.
+		unsigned int itemskill_no_conditions : 1; // Used by itemskill() script command, to ignore skill conditions and don't consume them.
 	} state;
 	struct {
 		unsigned char no_weapon_damage, no_magic_damage, no_misc_damage;
@@ -643,6 +645,15 @@ END_ZEROED_BLOCK;
 	bool achievements_received;
 	// Title
 	VECTOR_DECL(int) title_ids;
+
+	/*
+	 * itemskill_conditions_checked/itemskill_no_conditions abuse prevention.
+	 * If a skill, casted by itemskill() script command, is aborted while target selection,
+	 * the map server gets no notification where these states could be unset.
+	 * Thus we need this helper variables to prevent abusing these states for next skill cast.
+	 */
+	int itemskill_id;
+	int itemskill_lv;
 };
 
 #define EQP_WEAPON EQP_HAND_R
diff --git a/src/map/script.c b/src/map/script.c
index b77e6a376..470ad9408 100644
--- a/src/map/script.c
+++ b/src/map/script.c
@@ -11000,15 +11000,26 @@ static BUILDIN(itemskill)
 
 	id = ( script_isstringtype(st,2) ? skill->name2id(script_getstr(st,2)) : script_getnum(st,2) );
 	lv = script_getnum(st,3);
-/* temporarily disabled, awaiting for kenpachi to detail this so we can make it work properly */
-#if 0
-	if( !script_hasdata(st, 4) ) {
-		if( !skill->check_condition_castbegin(sd,id,lv) || !skill->check_condition_castend(sd,id,lv) )
-			return true;
-	}
-#endif
 	sd->skillitem=id;
 	sd->skillitemlv=lv;
+
+	/// itemskill_conditions_checked/itemskill_no_conditions abuse prevention.
+	/// Unset in unit_skilluse_id()/unit_skilluse_pos() if skill was not aborted while target selection.
+	sd->itemskill_id = id;
+	sd->itemskill_lv = lv;
+
+	int flag = script_hasdata(st, 4) ? script_getnum(st, 4) : ISF_NONE;
+
+	sd->state.itemskill_conditions_checked = 0; /// Skill casting items will check the conditions prior to the target selection in AEGIS. Thus we need a flag to prevent checking them twice.
+	sd->state.itemskill_no_conditions = ((flag & ISF_IGNORECONDITIONS) == ISF_IGNORECONDITIONS) ? 1 : 0; /// Unset in unit_skilluse_id()/unit_skilluse_pos() if skill was not aborted while target selection.
+
+	if (sd->state.itemskill_no_conditions == 0) {
+		if (skill->check_condition_castbegin(sd, id, lv) == 0 || skill->check_condition_castend(sd, id, lv) == 0)
+			return true;
+
+		sd->state.itemskill_conditions_checked = 1; /// Unset in unit_skilluse_id()/unit_skilluse_pos() if skill was not aborted while target selection.
+	}
+
 	clif->item_skill(sd,id,lv);
 	return true;
 }
diff --git a/src/map/script.h b/src/map/script.h
index 8d7669d68..c981a895d 100644
--- a/src/map/script.h
+++ b/src/map/script.h
@@ -565,6 +565,14 @@ enum mado_type {
 };
 
 /**
+ * Option flags for itemskill() script command.
+ **/
+enum itemskill_flag {
+	ISF_NONE = 0x00,
+	ISF_IGNORECONDITIONS = 0x01, // Ignore skill conditions and don't consume them.
+};
+
+/**
  * Structures
  **/
 
diff --git a/src/map/skill.c b/src/map/skill.c
index 0f0a72dce..2b4f87a11 100644
--- a/src/map/skill.c
+++ b/src/map/skill.c
@@ -13997,9 +13997,17 @@ static int skill_check_condition_castbegin(struct map_session_data *sd, uint16 s
 
 	nullpo_ret(sd);
 
+	if (skill_lv < 1 || skill_lv > MAX_SKILL_LEVEL)
+		return 0;
+
 	if (sd->chat_id != 0)
 		return 0;
 
+	if ((sd->state.itemskill_conditions_checked == 1 || sd->state.itemskill_no_conditions == 1)
+	    && sd->itemskill_id == sd->skillitem && sd->itemskill_lv == sd->skillitemlv) {
+		return 1;
+	}
+
 	if (pc_has_permission(sd, PC_PERM_SKILL_UNCONDITIONAL) && sd->skillitem != skill_id) {
 		//GMs don't override the skillItem check, otherwise they can use items without them being consumed! [Skotlex]
 		sd->state.arrow_atk = skill->get_ammotype(skill_id)?1:0; //Need to do arrow state check.
@@ -14041,24 +14049,32 @@ static int skill_check_condition_castbegin(struct map_session_data *sd, uint16 s
 			if( (i = sd->itemindex) == -1 ||
 				sd->status.inventory[i].nameid != sd->itemid ||
 				sd->inventory_data[i] == NULL ||
-				!sd->inventory_data[i]->flag.delay_consume ||
+				(sd->inventory_data[i]->flag.delay_consume == 0 && skill_id == WZ_EARTHSPIKE) || // TODO: See below. [Kenpachi]
 				sd->status.inventory[i].amount < 1
 			) {
 				//Something went wrong, item exploit?
 				sd->itemid = sd->itemindex = -1;
 				return 0;
 			}
+
+			/**
+			 * [Kenpachi] TODO:
+			 * - No skill casting item should be of type IT_DELAYCONSUME, they are all consumed immediately, even before the skill cursor appears.
+			 *   The WZ_EARTHSPIKE check for TK_SPTIME skill should be moved to pc_useitem(), once the type of all skill casting items is updated.
+			 *
+			 * - The consumption of 10 SP when using Earth_Scroll_1_3 or Earth_Scroll_1_5 while SC_EARTHSCROLL is active needs to be implemented.
+			 *
+			 **/
 			//Consume
 			sd->itemid = sd->itemindex = -1;
 			if( skill_id == WZ_EARTHSPIKE && sc && sc->data[SC_EARTHSCROLL] && rnd()%100 > sc->data[SC_EARTHSCROLL]->val2 ) // [marquis007]
 				; //Do not consume item.
-			else if( sd->status.inventory[i].expire_time == 0 ) // Rental usable items are not consumed until expiration
+			else if (sd->status.inventory[i].expire_time == 0 && sd->inventory_data[i]->flag.delay_consume == 1) // Rental usable items are not consumed until expiration
 				pc->delitem(sd, i, 1, 0, DELITEM_NORMAL, LOG_TYPE_CONSUME);
 		}
-		return 1;
 	}
 
-	if( pc_is90overweight(sd) ) {
+	if (pc_is90overweight(sd) && sd->skillitem != skill_id) { /// Skill casting items ignore the overweight restriction. [Kenpachi]
 		clif->skill_fail(sd, skill_id, USESKILL_FAIL_WEIGHTOVER, 0, 0);
 		return 0;
 	}
@@ -14182,9 +14198,6 @@ static int skill_check_condition_castbegin(struct map_session_data *sd, uint16 s
 			}
 	}
 
-	if( skill_lv < 1 || skill_lv > MAX_SKILL_LEVEL )
-		return 0;
-
 	require = skill->get_requirement(sd,skill_id,skill_lv);
 
 	//Can only update state when weapon/arrow info is checked.
@@ -14932,7 +14945,7 @@ static int skill_check_condition_castbegin(struct map_session_data *sd, uint16 s
 		return 0;
 	}
 
-	if( require.sp > 0 && st->sp < (unsigned int)require.sp) {
+	if (require.sp > 0 && st->sp < (unsigned int)require.sp && sd->skillitem != skill_id) { /// Skill casting items and Hocus-Pocus skills don't consume SP. [Kenpachi]
 		clif->skill_fail(sd, skill_id, USESKILL_FAIL_SP_INSUFFICIENT, 0, 0);
 		return 0;
 	}
@@ -14990,6 +15003,11 @@ static int skill_check_condition_castend(struct map_session_data *sd, uint16 ski
 	if (sd->chat_id != 0)
 		return 0;
 
+	if ((sd->state.itemskill_conditions_checked == 1 || sd->state.itemskill_no_conditions == 1)
+	    && sd->itemskill_id == sd->skillitem && sd->itemskill_lv == sd->skillitemlv) {
+		return 1;
+	}
+
 	if( pc_has_permission(sd, PC_PERM_SKILL_UNCONDITIONAL) && sd->skillitem != skill_id ) {
 		//GMs don't override the skillItem check, otherwise they can use items without them being consumed! [Skotlex]
 		sd->state.arrow_atk = skill->get_ammotype(skill_id)?1:0; //Need to do arrow state check.
@@ -15017,14 +15035,8 @@ static int skill_check_condition_castend(struct map_session_data *sd, uint16 ski
 				return 0;
 			break;
 	}
-	/* temporarily disabled, awaiting for kenpachi to detail this so we can make it work properly */
-#if 0
-	if( sd->state.abra_flag ) // Casting finished (Hocus-Pocus)
-		return 1;
-#endif
-	if( sd->skillitem == skill_id )
-		return 1;
-	if( pc_is90overweight(sd) ) {
+
+	if (pc_is90overweight(sd) && sd->skillitem != skill_id) { /// Skill casting items ignore the overweight restriction. [Kenpachi]
 		clif->skill_fail(sd, skill_id, USESKILL_FAIL_WEIGHTOVER, 0, 0);
 		return 0;
 	}
@@ -15197,6 +15209,9 @@ static int skill_consume_requirement(struct map_session_data *sd, uint16 skill_i
 
 	nullpo_ret(sd);
 
+	if (sd->state.itemskill_no_conditions == 1 && sd->itemskill_id == sd->skillitem && sd->itemskill_lv == sd->skillitemlv)
+		return 1;
+
 	req = skill->get_requirement(sd,skill_id,skill_lv);
 
 	if (type&1) {
@@ -15206,8 +15221,9 @@ static int skill_consume_requirement(struct map_session_data *sd, uint16 skill_i
 				req.sp = 0;
 				break;
 			default:
-				if( sd->state.autocast )
+				if (sd->state.autocast == 1 || sd->skillitem == skill_id) /// Skill casting items and Hocus-Pocus skills don't consume SP. [Kenpachi]
 					req.sp = 0;
+
 				break;
 		}
 
@@ -15285,12 +15301,6 @@ static struct skill_condition skill_get_requirement(struct map_session_data *sd,
 
 	if( !sd )
 		return req;
-#if 0 /* temporarily disabled, awaiting for kenpachi to detail this so we can make it work properly */
-	if( sd->state.abra_flag )
-#else // not 0
-	if( sd->skillitem == skill_id )
-#endif // 0
-		return req; // Hocus-Pocus don't have requirements.
 
 	sc = &sd->sc;
 	if( !sc->count )
diff --git a/src/map/unit.c b/src/map/unit.c
index 482440978..923438d78 100644
--- a/src/map/unit.c
+++ b/src/map/unit.c
@@ -1041,11 +1041,19 @@ static int unit_stop_walking(struct block_list *bl, int flag)
 
 static int unit_skilluse_id(struct block_list *src, int target_id, uint16 skill_id, uint16 skill_lv)
 {
-	return unit->skilluse_id2(
-		src, target_id, skill_id, skill_lv,
-		skill->cast_fix(src, skill_id, skill_lv),
-		skill->get_castcancel(skill_id)
-	);
+	int casttime = skill->cast_fix(src, skill_id, skill_lv);
+	int castcancel = skill->get_castcancel(skill_id);
+	int ret = unit->skilluse_id2(src, target_id, skill_id, skill_lv, casttime, castcancel);
+	struct map_session_data *sd = BL_CAST(BL_PC, src);
+
+	if (sd != NULL) {
+		sd->itemskill_id = 0;
+		sd->itemskill_lv = 0;
+		sd->state.itemskill_conditions_checked = 0;
+		sd->state.itemskill_no_conditions = 0;
+	}
+
+	return ret;
 }
 
 static int unit_is_walking(struct block_list *bl)
@@ -1418,15 +1426,8 @@ static int unit_skilluse_id2(struct block_list *src, int target_id, uint16 skill
 		}
 	}
 
-	if (sd) {
-		/* temporarily disabled, awaiting for kenpachi to detail this so we can make it work properly */
-#if 0
-		if (sd->skillitem != skill_id && !skill->check_condition_castbegin(sd, skill_id, skill_lv))
-#else
-		if (!skill->check_condition_castbegin(sd, skill_id, skill_lv))
-#endif
-			return 0;
-	}
+	if (sd != NULL && skill->check_condition_castbegin(sd, skill_id, skill_lv) == 0)
+		return 0;
 
 	if (src->type == BL_MOB) {
 		const struct mob_data *src_md = BL_UCCAST(BL_MOB, src);
@@ -1678,11 +1679,19 @@ static int unit_skilluse_id2(struct block_list *src, int target_id, uint16 skill
 
 static int unit_skilluse_pos(struct block_list *src, short skill_x, short skill_y, uint16 skill_id, uint16 skill_lv)
 {
-	return unit->skilluse_pos2(
-		src, skill_x, skill_y, skill_id, skill_lv,
-		skill->cast_fix(src, skill_id, skill_lv),
-		skill->get_castcancel(skill_id)
-	);
+	int casttime = skill->cast_fix(src, skill_id, skill_lv);
+	int castcancel = skill->get_castcancel(skill_id);
+	int ret = unit->skilluse_pos2(src, skill_x, skill_y, skill_id, skill_lv, casttime, castcancel);
+	struct map_session_data *sd = BL_CAST(BL_PC, src);
+
+	if (sd != NULL) {
+		sd->itemskill_id = 0;
+		sd->itemskill_lv = 0;
+		sd->state.itemskill_conditions_checked = 0;
+		sd->state.itemskill_no_conditions = 0;
+	}
+
+	return ret;
 }
 
 static int unit_skilluse_pos2(struct block_list *src, short skill_x, short skill_y, uint16 skill_id, uint16 skill_lv, int casttime, int castcancel)