Massive Monster Delay/AI Refactor (#9093)

- Refactored the monster Delay/AI so that the setting/execution order is much closer to official
  * This makes it much easier to have a stable implementation without having to write hundreds of exception
  * It will now be much easier to do further improvements without breaking other things
  * For detailed information, see points below
- A monster will now always check for skills at the beginning of its hard AI and cancel out if it used a skill
  * For BERSERK state it will check for attack delay instead of the skill interval
  * Removed all the special "skill use" checks that now work automatically due to having the right code structure
  * Cleaned up functions by moving unrelated AI logic into the main AI function (e.g. randomly walking)
  * Not refactored yet are skills-while-moving, event-triggered skills and dead state skills
- Setting of monster delays at the end of attacks and skills now works as on official servers
  * A monster's AI will be inactive for its attack motion, this applies even to bosses now
  * A monster cannot attack for its attack delay (this applies even if the skill was cast-canceled)
  * A monster will no longer get skill-based walk delays unless the skill used has one defined
  * Skill delays now work like cooldowns and are set at cast end and cast-cancel rather than at cast begin
  * Skill delays are set based on the status the monster is in at cast end
  * The custom feature to set delays per monster skill entry still works as previously
  * Removed all other special delay settings all over the code as they are not needed anymore
  * Disabling the AI when not needed will also save a lot of resources!
- Setting of various "tick" variables is now a lot more accurate
  * Spawned monsters will now act immediately, but cannot cast skills immediately
  * Summoned/recalled monsters will now act immediately without requiring a low MIN_MOBLINKTIME (now back at 1000ms)
  * Attack-linking in general will now be more reactive while requiring less resources in total
  * A good part of the code allows for frame-perfect behavior now (especially if MIN_MOBTHINKTIME is set to 20)
- Implemented a "mob_setstate" function which is now consistently used to set a monster's state
  * This fixes a lot of issues where the aggressive bit was not set correctly
  * Just makes setting of the state much more comfortable
  * Makes it easier to add actions in the future that should always happen when setting a certain state
  * Removed set_mobstate function from unit.cpp
- The attacktimer event for monsters now loops through the monster's AI instead of executing an attack directly
  * This allows for consistent and correct implementation in just a single location
  * The monster AI will no longer block itself from executing within MIN_MOBTHINKTIME to allow earlier reaction to events
  * This also allows for frame-perfect attack behavior even if the MIN_MOBTHINKTIME is higher than 20
  * Moved the "end of attack timer" check from unit.cpp into mob.cpp to have less AI logic in unit.cpp
- Monsters will no longer start walking when still able to cast skills
- A blind monster (e.g. Elder) will now drop target exactly at the moment it runs out of spells to cast
- Fixed an issue that a monster would sometimes switch into chase mode while randomly walking
- A monster that finds an enemy in attack range will now start off with a normal attack and then switch to Berserk
  * This still overwrites angry mode as before
- A monster that reaches an enemy will no longer start with a normal attack if it still has attack delay
- The refactor probably fixes a lot more issues we don't even know about due to them being fixed automatically
- Fixes #9075
- Fixes #9088 (enhancement)

src/map/mob.cpp

@@ -1121,7 +1121,7 @@ int32 mob_spawn (struct mob_data *md)
 	int32 i=0;
 	t_tick tick = gettick();
 
-	md->last_thinktime = tick;
+	md->next_thinktime = tick;
 	if (md->bl.prev != nullptr)
 		unit_remove_map(&md->bl,CLR_RESPAWN);
 	else
@@ -1187,21 +1187,18 @@ int32 mob_spawn (struct mob_data *md)
 //	md->master_id = 0;
 	md->master_dist = 0;
 
-	md->state.aggressive = status_has_mode(&md->status,MD_ANGRY)?1:0;
-	md->state.skillstate = MSS_IDLE;
+	mob_setstate(*md, MSS_IDLE);
 	md->ud.state.blockedmove = false;
 	md->next_walktime = tick+rnd()%1000+MIN_RANDOMWALKTIME;
-	md->last_linktime = tick;
+	md->last_linktime = 0;
 	md->dmgtick = tick - 5000;
 	md->last_pcneartime = 0;
 	md->last_canmove = tick;
-	md->last_skillcheck = 0;
+	md->last_skillcheck = tick;
 	md->trickcasting = 0;
 
-	t_tick c = tick - MOB_MAX_DELAY;
-
 	for (i = 0; i < MAX_MOBSKILL; i++)
-		md->skilldelay[i] = c;
+		md->skilldelay[i] = 0;
 	for (i = 0; i < DAMAGELOG_SIZE; i++)
 		md->spotted_log[i] = 0;
 
@@ -1509,7 +1506,6 @@ static int32 mob_ai_sub_hard_slavemob(struct mob_data *md,t_tick tick)
 	if (DIFF_TICK(tick, md->last_linktime) >= MIN_MOBLINKTIME && !md->target_id)
   	{
 		struct unit_data *ud = unit_bl2ud(bl);
-		md->last_linktime = tick;
 
 		if (ud) {
 			struct block_list *tbl=nullptr;
@@ -1523,9 +1519,19 @@ static int32 mob_ai_sub_hard_slavemob(struct mob_data *md,t_tick tick)
 				if (tbl && battle_check_target(&md->bl, tbl, BCT_ENEMY) <= 0)
 					tbl = nullptr;
 			}
-			if (tbl && status_check_skilluse(&md->bl, tbl, 0, 0)) {
-				md->target_id=tbl->id;
-				return 1;
+			else if (bl->type == BL_MOB) {
+				// If master is a monster, it might still have a target after using a skill
+				mob_data& mmd = reinterpret_cast<mob_data&>(*bl);
+				if (mmd.target_id > 0)
+					tbl = map_id2bl(mmd.target_id);
+			}
+
+			if (tbl != nullptr) {
+				md->last_linktime = tick;
+				if (status_check_skilluse(&md->bl, tbl, 0, 0)) {
+					md->target_id = tbl->id;
+					return 1;
+				}
 			}
 		}
 	}
@@ -1550,36 +1556,16 @@ int32 mob_unlocktarget(struct mob_data *md, t_tick tick)
 		if (md->ud.walktimer != INVALID_TIMER)
 			break;
 		//Because it is not unset when the mob finishes walking.
-		md->state.skillstate = MSS_IDLE;
+		mob_setstate(*md, MSS_IDLE);
 		[[fallthrough]];
 	case MSS_IDLE:
-		// When walking we want to trigger the idle skills through the walk routine so we can prevent stopping
-		// This situation happens when an immobile monster uses a skill to move
-		if (md->ud.walktimer != INVALID_TIMER)
-			break;
-		if (md->idle_event[0] && npc_event_do_id(md->idle_event, md->bl.id) > 0) {
+		if (md->ud.walktimer == INVALID_TIMER && md->idle_event[0] && npc_event_do_id(md->idle_event, md->bl.id) > 0)
 			md->idle_event[0] = 0;
-			break;
-		}
-		// Idle skill.
-		if (DIFF_TICK(tick, md->last_skillcheck) >= MOB_SKILL_INTERVAL && mobskill_use(md, tick, -1))
-			break;
-		//Random walk.
-		if (!md->master_id &&
-			DIFF_TICK(md->next_walktime, tick) <= 0 &&
-			!mob_randomwalk(md,tick))
-			//Delay next random walk when this one failed.
-			if (md->next_walktime < md->ud.canmove_tick)
-				md->next_walktime = md->ud.canmove_tick;
-			else
-				md->next_walktime = tick+rnd()%1000;
 		break;
 	default:
 		unit_stop_attack( &md->bl );
 		unit_stop_walking_soon(md->bl); //Stop chasing.
-		if (status_has_mode(&md->status,MD_ANGRY) && !md->state.aggressive)
-			md->state.aggressive = 1; //Restore angry state when switching to idle
-		md->state.skillstate = MSS_IDLE;
+		mob_setstate(*md, MSS_IDLE);
 		if(battle_config.mob_ai&0x8) //Walk instantly after dropping target
 			md->next_walktime = tick+rnd()%1000;
 		else
@@ -1624,6 +1610,7 @@ int32 mob_randomwalk(struct mob_data *md,t_tick tick)
 		return 0;
 
 	// Make sure the monster has no target anymore, otherwise the walkpath will check for it
+	md->ud.state.attack_continue = 0;
 	md->ud.target_to = 0;
 
 	r=rnd();
@@ -1694,7 +1681,7 @@ int32 mob_randomwalk(struct mob_data *md,t_tick tick)
 		}
 		return 0;
 	}
-	md->state.skillstate=MSS_WALK;
+	mob_setstate(*md, MSS_WALK);
 	md->move_fail_count=0;
 	md->next_walktime = tick+rnd()%1000+MIN_RANDOMWALKTIME + unit_get_walkpath_time(md->bl);
 	return 1;
@@ -1723,6 +1710,29 @@ int32 mob_warpchase(struct mob_data *md, struct block_list *target)
 	return 0;
 }
 
+/**
+ * Sets a mob's state considering the aggressive bit
+ * @param md: Mob to set state on
+ * @param skillstate: Target state of the monster
+ */
+void mob_setstate(mob_data& md, MobSkillState skillstate) {
+	switch (skillstate) {
+		case MSS_BERSERK:
+		case MSS_ANGRY:
+			md.state.skillstate = md.state.aggressive?MSS_ANGRY:MSS_BERSERK;
+			break;
+		case MSS_RUSH:
+		case MSS_FOLLOW:
+			md.state.skillstate = md.state.aggressive?MSS_FOLLOW:MSS_RUSH;
+			break;
+		default:
+			// When going to any state other than BERSERK/RUSH, aggressive should be reset
+			md.state.aggressive = status_has_mode(&md.status, MD_ANGRY)?1:0;
+			md.state.skillstate = skillstate;
+			break;
+	}
+}
+
 /*==========================================
  * AI of MOB whose is near a Player
  *------------------------------------------*/
@@ -1739,10 +1749,11 @@ static bool mob_ai_sub_hard(struct mob_data *md, t_tick tick)
 	if (md->ud.state.force_walk)
 		return false;
 
-	if (DIFF_TICK(tick, md->last_thinktime) < MIN_MOBTHINKTIME)
+	if (DIFF_TICK(tick, md->next_thinktime) < 0)
 		return false;
 
-	md->last_thinktime = tick;
+	// This prevents the lazy AI from being executed at the same time
+	md->next_thinktime = tick;
 
 	if (md->ud.skilltimer != INVALID_TIMER)
 		return false;
@@ -1753,6 +1764,31 @@ static bool mob_ai_sub_hard(struct mob_data *md, t_tick tick)
 		return false;
 	}
 
+	// Before a monster processes its AI, it will check for a skill
+	// It it uses a skill it will not process its AI further until the next interval
+	// Until we implement subcell movement, casting while moving still needs to be done in unit.cpp
+	// Dead and Berserk states have special interval rules
+	// TODO: Monster AI for dead state is not implemented at the moment
+	if (md->ud.walktimer == INVALID_TIMER) {
+		bool skill_ready = false;
+		switch (md->state.skillstate) {
+			case MSS_BERSERK:
+				skill_ready = (DIFF_TICK(tick, md->ud.attackabletime) >= 0);
+				break;
+//			case MSS_DEAD:
+//				skill_ready = (DIFF_TICK(tick, md->last_skillcheck) >= 300);
+//				break;
+			default:
+				skill_ready = (DIFF_TICK(tick, md->last_skillcheck) >= MOB_SKILL_INTERVAL);
+				break;
+		}
+		if (skill_ready && mobskill_use(md, tick, -1))
+			return true;
+	}
+
+	// Note: Anything below this point should actually be using a Finite-state machine (FSM)
+	// But we don't have the FSM data in the database at the moment, so we cannot get a fully correct execution order
+
 	if (md->sc.getSCE(SC_BLIND))
 		view_range = 1;
 	else
@@ -1780,6 +1816,19 @@ static bool mob_ai_sub_hard(struct mob_data *md, t_tick tick)
 			mob_unlocktarget(md, tick); //Unlock target
 			tbl = nullptr;
 		}
+		// Monsters in chase state that are not moving will recheck their target
+		// This usually happens when they used a skill that stopped them or they were hit while chasing
+		// Items are handled later in this function
+		if (tbl != nullptr && tbl->type != BL_ITEM && md->ud.walktimer == INVALID_TIMER && mob_is_chasing(md->state.skillstate)) {
+			// But don't do this if they stopped because target is already in attack range
+			if (!battle_check_range(&md->bl, tbl, md->status.rhw.range)) {
+				// If target is no longer visible, target is dropped and the monster waits for next iteration to continue
+				if (!status_check_visibility(&md->bl, tbl, true)) {
+					mob_unlocktarget(md, tick);
+					return true;
+				}
+			}
+		}
 	}
 
 	// Check for target change.
@@ -1872,11 +1921,11 @@ static bool mob_ai_sub_hard(struct mob_data *md, t_tick tick)
 	{
 		int32 prev_id = md->target_id;
 		map_foreachinallrange (mob_ai_sub_hard_activesearch, &md->bl, view_range, DEFAULT_ENEMY_TYPE(md), md, &tbl, mode);
-		// If a monster finds a target through search that is already in attack range it immediately switches to berserk mode
+		// If a monster finds a new target that is already in attack range it immediately switches to rush mode
 		// This behavior overrides even angry mode and other mode-specific behavior
 		if (tbl != nullptr && prev_id != md->target_id && battle_check_range(&md->bl, tbl, md->status.rhw.range)) {
 			md->state.aggressive = 0;
-			md->state.skillstate = MSS_BERSERK;
+			mob_setstate(*md, MSS_RUSH);
 		}
 	}
 	else
@@ -1899,8 +1948,20 @@ static bool mob_ai_sub_hard(struct mob_data *md, t_tick tick)
 			}
 		}
 
-		//This handles triggering idle/walk skill.
+		// Make sure target is unlocked
 		mob_unlocktarget(md, tick);
+
+		// Random walk
+		if (!md->master_id &&
+			DIFF_TICK(md->next_walktime, tick) <= 0 &&
+			!mob_randomwalk(md, tick)) {
+			// Delay next random walk when this one failed
+			if (md->next_walktime < md->ud.canmove_tick)
+				md->next_walktime = md->ud.canmove_tick;
+			else
+				md->next_walktime = tick + rnd()%1000;
+		}
+
 		return true;
 	}
 
@@ -1924,7 +1985,7 @@ static bool mob_ai_sub_hard(struct mob_data *md, t_tick tick)
 			}
 			if (!can_move) //Stuck. Wait before walking.
 				return true;
-			md->state.skillstate = MSS_LOOT;
+			mob_setstate(*md, MSS_LOOT);
 			if (!unit_walktobl(&md->bl, tbl, 0, 0))
 				mob_unlocktarget(md, tick); //Can't loot...
 			else
@@ -1965,20 +2026,6 @@ static bool mob_ai_sub_hard(struct mob_data *md, t_tick tick)
 
 	// At this point we know the target is attackable, attempt to attack
 
-	// Monsters in angry state, after having used a normal attack, will always attempt a skill
-	if (md->ud.walktimer == INVALID_TIMER && md->state.skillstate == MSS_ANGRY && md->ud.skill_id == 0)
-	{
-		// Only use skill if able to walk on next tick and not attempted a skill the last second
-		if (DIFF_TICK(md->ud.canmove_tick, tick) <= MIN_MOBTHINKTIME && DIFF_TICK(tick, md->last_skillcheck) >= MOB_SKILL_INTERVAL){
-			if (mobskill_use(md, tick, -1)) {
-				// After the monster used an angry skill, it will not attack for aDelay
-				// Setting the delay here because not all monster skill use situations will cause an attack delay
-				md->ud.attackabletime = tick + md->status.adelay;
-				return true;
-			}
-		}
-	}
-
 	// Normal attack / berserk skill is only used when target is in range
 	if (battle_check_range(&md->bl, tbl, md->status.rhw.range))
 	{
@@ -1986,7 +2033,8 @@ static bool mob_ai_sub_hard(struct mob_data *md, t_tick tick)
 		md->ud.target_to = 0;
 
 		// Hiding is a special case because it prevents normal attacks but allows skill usage
-		// TODO: Some other states also have this behavior and should be investigated (e.g. NPC_SR_CURSEDCIRCLE)
+		// TODO: Some other states also have this behavior and should be investigated
+		// TODO: EFST_AUTOCOUNTER, EFST_BLADESTOP, NPC_SR_CURSEDCIRCLE
 		if (!(md->sc.option&OPTION_HIDE)) {
 			// Target within range and potentially able to use normal attack, engage
 			if (md->ud.target != tbl->id || md->ud.attacktimer == INVALID_TIMER)
@@ -2003,14 +2051,10 @@ static bool mob_ai_sub_hard(struct mob_data *md, t_tick tick)
 				}
 			}
 		}
-		else if (md->state.skillstate == MSS_BERSERK && DIFF_TICK(md->ud.attackabletime, tick) <= 0) {
-			// Target within range and no attack delay, but unable to use normal attack, check for skill
-			// On official servers this check happens every 20ms
-			// If you want fully official behavior you will need to set MIN_MOBTHINKTIME to 20
-			if (mobskill_use(md, tick, -1)) {
-				// When a skill was used, the attack delay is applied
-				md->ud.attackabletime = tick + md->status.adelay;
-			}
+		else {
+			// These status changes pretend the attack was successful without attempting it
+			// This results in the monster going into an attack state despite not attacking
+			mob_setstate(*md, MSS_BERSERK);
 		}
 		//Target still in attack range, no need to chase the target
 		return true;
@@ -2027,14 +2071,12 @@ static bool mob_ai_sub_hard(struct mob_data *md, t_tick tick)
 		if (md->ud.attacktimer == INVALID_TIMER)
 		{ // Only switch mode if no more attack delay left
 			if (DIFF_TICK(tick, md->last_canmove) > battle_config.mob_unlock_time) {
-				// Unlock target or use idle/walk skill
+				// Unlock target
 				mob_unlocktarget(md, tick);
 			}
 			else {
-				// Use idle skill but keep target for now
-				md->state.skillstate = MSS_IDLE;
-				if (DIFF_TICK(tick, md->last_skillcheck) >= MOB_SKILL_INTERVAL)
-					mobskill_use(md, tick, -1);
+				// Set to use idle skill next interval but keep target for now
+				mob_setstate(*md, MSS_IDLE);
 			}
 		}
 		return true;
@@ -2053,20 +2095,6 @@ static bool mob_ai_sub_hard(struct mob_data *md, t_tick tick)
 
 	// Follow up if possible.
 
-	// Officially when a monster cannot act, move or attack, the AI isn't processed at all.
-	// The attacked ID remains unprocessed, so that once the monster can act again it will process it at this point.
-	// Our code works quite different from that and we try to plan a chase ahead of time.
-	// If we would go ahead now and initiate a chase, it will kill the attack timer which is not good because we
-	// need to check for visibility at the end of it for monsters because that's when they actually drop target.
-	// However, we only process a monster's AI every 100ms instead of every 20ms like on official servers, so
-	// without planning a chase, a monster is easy to hitlock.
-	// For now we prevent processing this part of the code if an attack timer is still running and the monster
-	// cannot move yet.
-	if (md->ud.walktimer == INVALID_TIMER
-		&& md->ud.attacktimer != INVALID_TIMER
-		&& DIFF_TICK(md->ud.canmove_tick, tick) > 0)
-		return true;
-
 	// Monsters in Angry state only start chasing when target is in chase range
 	if (md->state.skillstate == MSS_ANGRY && !mob_can_reach(md, tbl, md->db->range3)) {
 		mob_unlocktarget(md, tick);
@@ -2079,6 +2107,32 @@ static bool mob_ai_sub_hard(struct mob_data *md, t_tick tick)
 	return true;
 }
 
+/**
+ * End of attack timer event
+ * This is so we can call the mob AI at the end of an attack timer and don't need to wait until the next AI interval
+ * Also, mob AI checks that are triggered at the end of attack delay are currently handled here
+ * @param md Monster to execute the AI for
+ * @param tick Current tick
+ * @return Whether the AI was executed (true) or the attack should be cancelled (false)
+ */
+bool mob_ai_sub_hard_attacktimer(mob_data &md, t_tick tick)
+{
+	block_list* target = map_id2bl(md.ud.target);
+
+	// No target anymore
+	if (target == nullptr)
+		return false;
+
+	// Monsters have a special visibility check at the end of their attack delay
+	// If they still have a target, but it is not visible, they drop the target
+	if (target != nullptr && !status_check_visibility(&md.bl, target, true))
+		return false;
+
+	// Go through the whole monster AI
+	mob_ai_sub_hard(&md, tick);
+	return true;
+}
+
 static int32 mob_ai_sub_hard_timer(struct block_list *bl,va_list ap)
 {
 	struct mob_data *md = (struct mob_data*)bl;
@@ -2128,7 +2182,7 @@ static int32 mob_ai_sub_lazy(struct mob_data *md, va_list args)
 	if(battle_config.mob_active_time &&
 		md->last_pcneartime &&
  		!status_has_mode(&md->status,MD_STATUSIMMUNE) &&
-		DIFF_TICK(tick,md->last_thinktime) > MIN_MOBTHINKTIME)
+		DIFF_TICK(tick,md->next_thinktime) > 0) // No need to trigger on the same tick again
 	{
 		if (DIFF_TICK(tick,md->last_pcneartime) < battle_config.mob_active_time)
 			return (int32)mob_ai_sub_hard(md, tick);
@@ -2138,7 +2192,7 @@ static int32 mob_ai_sub_lazy(struct mob_data *md, va_list args)
 	if(battle_config.boss_active_time &&
 		md->last_pcneartime &&
 		status_has_mode(&md->status,MD_STATUSIMMUNE) &&
-		DIFF_TICK(tick,md->last_thinktime) > MIN_MOBTHINKTIME)
+		DIFF_TICK(tick,md->next_thinktime) > 0) // No need to trigger on the same tick again
 	{
 		if (DIFF_TICK(tick,md->last_pcneartime) < battle_config.boss_active_time)
 			return (int32)mob_ai_sub_hard(md, tick);
@@ -2148,10 +2202,14 @@ static int32 mob_ai_sub_lazy(struct mob_data *md, va_list args)
 	//Clean the spotted log
 	mob_clean_spotted(md);
 
-	if(DIFF_TICK(tick,md->last_thinktime) < MOB_SKILL_INTERVAL)
+	// Don't continue if the hard AI was executed recently
+	// We use twice the hard AI interval to account for lag
+	if (DIFF_TICK(tick, md->next_thinktime) < MIN_MOBTHINKTIME*2)
 		return 0;
 
-	md->last_thinktime=tick;
+	// Don't continue if skills cannot be used yet
+	if (DIFF_TICK(tick, md->last_skillcheck) < MOB_SKILL_INTERVAL)
+		return 0;
 
 	if (md->master_id) {
 		if (!mob_is_spotted(md)) {
@@ -2169,7 +2227,7 @@ static int32 mob_ai_sub_lazy(struct mob_data *md, va_list args)
 
 	if (md->ud.walktimer == INVALID_TIMER) {
 		// Because it is not unset when the mob finishes walking.
-		md->state.skillstate = MSS_IDLE;
+		mob_setstate(*md, MSS_IDLE);
 		if (md->idle_event[0] && npc_event_do_id( md->idle_event, md->bl.id ) > 0) {
 			md->idle_event[0] = 0;
 			return 0;
@@ -2684,7 +2742,7 @@ int32 mob_dead(struct mob_data *md, struct block_list *src, int32 type)
 
 	if( src ) { // Use Dead skill only if not killed by Script or Command
 		md->status.hp = 1;
-		md->state.skillstate = MSS_DEAD;
+		mob_setstate(*md, MSS_DEAD);
 		mobskill_use(md,tick,-1);
 		md->status.hp = 0;
 	}
@@ -3345,8 +3403,7 @@ int32 mob_dead(struct mob_data *md, struct block_list *src, int32 type)
 void mob_revive(struct mob_data *md, uint32 hp)
 {
 	t_tick tick = gettick();
-	md->state.skillstate = MSS_IDLE;
-	md->last_thinktime = tick;
+	mob_setstate(*md, MSS_IDLE);
 	md->next_walktime = tick+rnd()%1000+MIN_RANDOMWALKTIME;
 	md->last_linktime = tick;
 	md->last_pcneartime = 0;
@@ -3538,10 +3595,8 @@ int32 mob_class_change (struct mob_data *md, int32 mob_id)
 		if(md->status.hp < 1) md->status.hp = 1;
 	}
 
-	t_tick c = tick - MOB_MAX_DELAY;
-
 	for(i=0;i<MAX_MOBSKILL;i++)
-		md->skilldelay[i] = c;
+		md->skilldelay[i] = 0;
 
 	if (md->lootitems == nullptr && status_has_mode(&md->db->status,MD_LOOTER))
 		md->lootitems = (struct s_mob_lootitem *)aCalloc(LOOTITEM_SIZE,sizeof(struct s_mob_lootitem));
@@ -3907,6 +3962,48 @@ bool mob_chat_display_message(mob_data &md, uint16 msg_id) {
 	return false;
 }
 
+
+/**
+ * This function handles actions that should be done at the end of a skill
+ * This needs to happen whether the skill was cast successfully or cast-cancelled
+ * It handles setting the skill delays and the normal attack wait time
+ * @param bl: Mob data
+ */
+void mobskill_end(mob_data& md, t_tick tick)
+{
+	std::vector<std::shared_ptr<s_mob_skill>>& ms = md.db->skill;
+
+	if (ms.empty())
+		return;
+
+	// Officially the skill delay is per skill rather than per skill db entry
+	// We simulate this by setting the delay for all entries of the same skill
+	// We have an optional mob AI setting to only set the delay for one entry
+	if (!(battle_config.mob_ai&0x200)) {
+		// Even though we already know which skill was picked as we stored that in skill_idx
+		// Officially it actually tries to find the skill again using the monster's current state
+		// If the skill cannot be found anymore because the monster's state has changed no delay will be applied
+		int32 delay = 0;
+		for (int32 i = 0; i < ms.size(); i++) {
+			if (ms[i]->state == md.state.skillstate && ms[i]->skill_id == ms[md.skill_idx]->skill_id) {
+				// State and skill match, use the first delay found
+				delay = ms[i]->delay;
+				break;
+			}
+		}
+		// Apply delay found to all entries of the skill
+		for (int32 i = 0; i < ms.size(); i++)
+			if (ms[i]->skill_id == ms[md.skill_idx]->skill_id)
+				md.skilldelay[i] = tick + delay;
+	}
+	else
+		md.skilldelay[md.skill_idx] = tick + ms[md.skill_idx]->delay;
+
+	// After a skill a monster cannot attack for its attack delay
+	// We make sure to not reduce it in case it was set by a skill for another purpose
+	md.ud.attackabletime = i64max(tick + md.status.adelay, md.ud.attackabletime);
+}
+
 /*==========================================
  * Skill use judging
  *------------------------------------------*/
@@ -3938,7 +4035,7 @@ bool mobskill_use(struct mob_data *md, t_tick tick, int32 event, int64 damage)
 		if (i == ms.size())
 			i = 0;
 
-		if (DIFF_TICK(tick, md->skilldelay[i]) < ms[i]->delay)
+		if (DIFF_TICK(tick, md->skilldelay[i]) < 0)
 			continue;
 
 		c2 = ms[i]->cond2;
@@ -4137,12 +4234,6 @@ bool mobskill_use(struct mob_data *md, t_tick tick, int32 event, int64 damage)
 		if ( ms[i]->msg_id ){ //Display color message [SnakeDrak]
 			mob_chat_display_message(*md, ms[i]->msg_id);
 		}
-		if(!(battle_config.mob_ai&0x200)) { //pass on delay to same skill.
-			for (j = 0; j < ms.size(); j++)
-				if (ms[j]->skill_id == ms[i]->skill_id)
-					md->skilldelay[j]=tick;
-		} else
-			md->skilldelay[i]=tick;
 		map_freeblock_unlock();
 		return true;
 	}

src/map/mob.hpp

@@ -33,11 +33,11 @@ struct guardian_data;
 //Min time between AI executions
 const t_tick MIN_MOBTHINKTIME = 100;
 //Min time before mobs do a check to call nearby friends for help (or for slaves to support their master)
-const t_tick MIN_MOBLINKTIME = 300;
+const t_tick MIN_MOBLINKTIME = 1000;
 //Min time between random walks
 const t_tick MIN_RANDOMWALKTIME = 4000;
 
-// How often a monster will check for using a skill on non-attack states (in ms)
+// How often a monster will check for using a skill on non-berserk and non-dead states (in ms)
 const t_tick MOB_SKILL_INTERVAL = 1000;
 
 //Distance that slaves should keep from their master.
@@ -365,7 +365,7 @@ struct mob_data {
 	int32 areanpc_id; //Required in OnTouchNPC (to avoid multiple area touchs)
 	int32 bg_id; // BattleGround System
 
-	t_tick next_walktime,last_thinktime,last_linktime,last_pcneartime,dmgtick,last_canmove,last_skillcheck;
+	t_tick next_walktime,next_thinktime,last_linktime,last_pcneartime,dmgtick,last_canmove,last_skillcheck;
 	t_tick trickcasting; // Special state where you show a fake castbar while moving
 	int16 move_fail_count;
 	int16 lootitem_count;
@@ -510,6 +510,8 @@ int32 mob_guardian_guildchange(struct mob_data *md); //Change Guardian's ownersh
 
 int32 mob_randomwalk(struct mob_data *md,t_tick tick);
 int32 mob_warpchase(struct mob_data *md, struct block_list *target);
+void mob_setstate(mob_data& md, MobSkillState skillstate);
+bool mob_ai_sub_hard_attacktimer(mob_data &md, t_tick tick);
 int32 mob_target(struct mob_data *md,struct block_list *bl,int32 dist);
 int32 mob_unlocktarget(struct mob_data *md, t_tick tick);
 struct mob_data* mob_spawn_dataset(struct spawn_data *data);
@@ -537,6 +539,7 @@ int32 mob_warpslave(struct block_list *bl, int32 range);
 int32 mob_linksearch(struct block_list *bl,va_list ap);
 
 bool mob_chat_display_message (mob_data &md, uint16 msg_id);
+void mobskill_end(mob_data& md, t_tick tick);
 bool mobskill_use(struct mob_data *md,t_tick tick,int32 event, int64 damage = 0);
 int32 mobskill_event(struct mob_data *md,struct block_list *src,t_tick tick, int32 flag, int64 damage = 0);
 int32 mob_summonslave(struct mob_data *md2,int32 *value,int32 amount,uint16 skill_id);

src/map/skill.cpp

@@ -10366,9 +10366,9 @@ int32 skill_castend_nodamage_id (struct block_list *src, struct block_list *bl,
 
 				if (time) {
 					// Need to set state here as it's not set otherwise
-					md->state.skillstate = MSS_WALK;
+					mob_setstate(*md, MSS_WALK);
 					// Set AI to inactive for the duration of this movement
-					md->last_thinktime = tick + time;
+					md->next_thinktime = tick + time;
 				}
 			}
 		}
@@ -14013,7 +14013,9 @@ TIMER_FUNC(skill_castend_id){
 			break;
 
 		if(md) {
-			md->last_thinktime=tick +MIN_MOBTHINKTIME;
+			// When a monster uses a skill, its AI will be inactive for its attack motion
+			// This is also the reason why it doesn't move during this time
+			md->next_thinktime = tick + status_get_amotion(src);
 			if(md->skill_idx >= 0 && md->db->skill[md->skill_idx]->emotion >= 0)
 				clif_emotion( *src, static_cast<emotion_type>( md->db->skill[md->skill_idx]->emotion ) );
 		}
@@ -14114,6 +14116,10 @@ TIMER_FUNC(skill_castend_id){
 					skill_blockpc_start(*sd, ud->skill_id, cooldown);
 			}
 			break;
+			case BL_MOB:
+				// Sets cooldowns and attack delay
+				mobskill_end(*md, tick);
+			break;
 			case BL_HOM:
 			{
 				homun_data &hd = reinterpret_cast<homun_data &>(*src);
@@ -14167,10 +14173,9 @@ TIMER_FUNC(skill_castend_id){
 			}
 		}
 		if (skill_get_state(ud->skill_id) != ST_MOVE_ENABLE) {
-			// When monsters used a skill they won't walk for amotion, this does not apply to players
-			// This is also important for monster skill usage behavior
+			// Monsters don't need a default walk delay as their AI is inactive after using a skill in general
 			if (src->type == BL_MOB)
-				unit_set_walkdelay(src, tick, max((int32)status_get_amotion(src), skill_get_walkdelay(ud->skill_id, ud->skill_lv)), 1);
+				unit_set_walkdelay(src, tick, skill_get_walkdelay(ud->skill_id, ud->skill_lv), 1);
 			else
 				unit_set_walkdelay(src, tick, battle_config.default_walk_delay + skill_get_walkdelay(ud->skill_id, ud->skill_lv), 1);
 		}
@@ -14352,7 +14357,9 @@ TIMER_FUNC(skill_castend_pos){
 			break;
 
 		if(md) {
-			md->last_thinktime=tick +MIN_MOBTHINKTIME;
+			// When a monster uses a skill, its AI will be inactive for its attack motion
+			// This is also the reason why it doesn't move during this time
+			md->next_thinktime = tick + status_get_amotion(src);
 			if(md->skill_idx >= 0 && md->db->skill[md->skill_idx]->emotion >= 0)
 				clif_emotion( *src, static_cast<emotion_type>( md->db->skill[md->skill_idx]->emotion ) );
 		}
@@ -14370,6 +14377,10 @@ TIMER_FUNC(skill_castend_pos){
 			int32 cooldown = pc_get_skillcooldown(sd,ud->skill_id, ud->skill_lv);
 			if(cooldown) skill_blockpc_start(*sd, ud->skill_id, cooldown);
 		}
+		else if (md != nullptr) {
+			// Sets cooldowns and attack delay
+			mobskill_end(*md, tick);
+		}
 		if( battle_config.display_status_timers && sd )
 			clif_status_change(src, EFST_POSTDELAY, 1, skill_delayfix(src, ud->skill_id, ud->skill_lv), 0, 0, 0);
 //		if( sd )
@@ -14381,10 +14392,9 @@ TIMER_FUNC(skill_castend_pos){
 //				break;
 //			}
 //		}
-		// When monsters used a skill they won't walk for amotion, this does not apply to players
-		// This is also important for monster skill usage behavior
+		// Monsters don't need a default walk delay as their AI is inactive after using a skill in general
 		if (src->type == BL_MOB)
-			unit_set_walkdelay(src, tick, max((int32)status_get_amotion(src), skill_get_walkdelay(ud->skill_id, ud->skill_lv)), 1);
+			unit_set_walkdelay(src, tick, skill_get_walkdelay(ud->skill_id, ud->skill_lv), 1);
 		else
 			unit_set_walkdelay(src, tick, battle_config.default_walk_delay + skill_get_walkdelay(ud->skill_id, ud->skill_lv), 1);
 		map_freeblock_lock();

src/map/status.cpp

@@ -2158,8 +2158,9 @@ bool status_check_skilluse(struct block_list *src, struct block_list *target, ui
 		}
 
 		if (sc->option) {
+			// We do not check it for non-players here as hide will not stop monsters from scanning for new targets and use skills
+			// The logic that normal attacks will not actually be executed when hidden needs to be put in the AI code instead
 			if ((sc->option&OPTION_HIDE) && src->type == BL_PC && (skill_id == 0 || !skill_get_inf2(skill_id, INF2_ALLOWWHENHIDDEN))) {
-				// Non players can use all skills while hidden.
 				return false;
 			}
 			if (sc->option&OPTION_CHASEWALK && skill_id != ST_CHASEWALK)

src/map/unit.cpp

@@ -78,20 +78,6 @@ struct unit_data* unit_bl2ud(struct block_list *bl)
 	}
 }
 
-/**
- * Sets a mob's CHASE/FOLLOW state
- * This should not be done if there's no path to reach
- * @param bl: Mob to set state on
- * @param flag: Whether to set state or not
- */
-static inline void set_mobstate(struct block_list* bl, int32 flag)
-{
-	struct mob_data* md = BL_CAST(BL_MOB, bl);
-
-	if (md != nullptr && flag)
-		md->state.skillstate = md->state.aggressive ? MSS_FOLLOW : MSS_RUSH;
-}
-
 /**
  * Updates chase depending on situation:
  * If target in attack range -> attack
@@ -286,10 +272,12 @@ int32 unit_walktoxy_sub(struct block_list *bl)
 		unit_refresh( bl, true );
 	}
 #endif
-
 	// Set mobstate here already as chase skills can be used on the first frame of movement
 	// If we don't set it now the monster will always move a full cell before checking
-	set_mobstate(bl, ud->state.attack_continue);
+	else if (bl->type == BL_MOB && ud->state.attack_continue) {
+		mob_data& md = reinterpret_cast<mob_data&>(*bl);
+		mob_setstate(md, MSS_RUSH);
+	}
 
 	unit_walktoxy_nextcell(*bl, true, gettick());
 
@@ -589,11 +577,8 @@ static TIMER_FUNC(unit_walktoxy_timer)
 		//Needs to be done here so that rudeattack skills are invoked
 		md->walktoxy_fail_count++;
 		clif_fixpos( *bl );
-		// Monsters in this situation will unlock target and then attempt an idle skill
-		// When they start chasing again, they will check for a chase skill before returning here
+		// Monsters in this situation will unlock target
 		mob_unlocktarget(md, tick);
-		if (DIFF_TICK(tick, md->last_skillcheck) >= MOB_SKILL_INTERVAL)
-			mobskill_use(md, tick, -1);
 		return 0;
 	}
 
@@ -962,7 +947,12 @@ int32 unit_walktobl(struct block_list *bl, struct block_list *tbl, int32 range,
 
 	if(ud->walktimer != INVALID_TIMER) {
 		ud->state.change_walk_target = 1;
-		set_mobstate(bl, flag&2);
+
+		// New target, make sure a monster is still in chase state
+		if (bl->type == BL_MOB && ud->state.attack_continue) {
+			mob_data& md = reinterpret_cast<mob_data&>(*bl);
+			mob_setstate(md, MSS_RUSH);
+		}
 
 		return 1;
 	}
@@ -2678,6 +2668,13 @@ int32 unit_attack(struct block_list *src,int32 target_id,int32 continuous)
 	else // Attack NOW.
 		unit_attack_timer(INVALID_TIMER, gettick(), src->id, 0);
 
+	// Monster state is set regardless of whether the attack is executed now or later
+	// The check is here because unit_attack can be called from both the monster AI and the walking logic
+	if (src->type == BL_MOB) {
+		mob_data& md = reinterpret_cast<mob_data&>(*src);
+		mob_setstate(md, MSS_BERSERK);
+	}
+
 	return 0;
 }
 
@@ -2898,17 +2895,23 @@ static int32 unit_attack_timer_sub(struct block_list* src, int32 tid, t_tick tic
 
 	sd = BL_CAST(BL_PC, src);
 	md = BL_CAST(BL_MOB, src);
+
+	// Make sure attacktimer is removed before doing anything else
 	ud->attacktimer = INVALID_TIMER;
+
+	// Note: Officially there is no such thing as an attack timer. All actions are driven by the client or the AI.
+	// We use the continuous attack timers to have accurate attack timings that don't depend on the AI interval.
+	// However, for a clean implementation we still should channel through the whole AI code so the same rules
+	// apply as usual and we don't need to code extra rules. Currently we resolved this only for monsters.
+	// We don't want this to trigger on direct calls of the timer function as that should just execute the attack.
+	if (md != nullptr && tid != INVALID_TIMER)
+		return mob_ai_sub_hard_attacktimer(*md, tick);
+
 	target = map_id2bl(ud->target);
 
 	if( src == nullptr || src->prev == nullptr || target==nullptr || target->prev == nullptr )
 		return 0;
 
-	// Monsters have a special visibility check at the end of their attack delay
-	// We don't want this to trigger on direct calls of the timer function
-	if (src->type == BL_MOB && tid != INVALID_TIMER && !status_check_visibility(src, target, true))
-		return 0;
-
 	if( status_isdead(*src) || status_isdead(*target) ||
 			battle_check_target(src,target,BCT_ENEMY) <= 0 || !status_check_skilluse(src, target, 0, 0)
 #ifdef OFFICIAL_WALKPATH
@@ -2990,18 +2993,6 @@ static int32 unit_attack_timer_sub(struct block_list* src, int32 tid, t_tick tic
 			unit_stop_walking( src, USW_FIXPOS );
 
 		if(md) {
-			// Berserk skills can replace normal attacks except for the first attack
-			// If this is the first attack, the state is not Berserk yet, so the skill check is skipped
-			if(md->state.skillstate == MSS_BERSERK) {
-				if (mobskill_use(md, tick, -1)) {
-					// Setting the delay here because not all monster skill use situations will cause an attack delay
-					ud->attackabletime = tick + sstatus->adelay;
-					return 1;
-				}
-			}
-			// Set mob's ANGRY/BERSERK states.
-			md->state.skillstate = md->state.aggressive?MSS_ANGRY:MSS_BERSERK;
-
 			if (status_has_mode(sstatus,MD_ASSIST) && DIFF_TICK(tick, md->last_linktime) >= MIN_MOBLINKTIME) { 
 				// Link monsters nearby [Skotlex]
 				md->last_linktime = tick;
@@ -3033,9 +3024,14 @@ static int32 unit_attack_timer_sub(struct block_list* src, int32 tid, t_tick tic
 		if (ud->skilltimer == INVALID_TIMER)
 			ud->skill_id = 0;
 
-		// You can't move if you can't attack neither.
-		if (src->type&battle_config.attack_walk_delay)
-			unit_set_walkdelay(src, tick, sstatus->amotion, 1);
+		// You can't move during your attack motion
+		if (src->type&battle_config.attack_walk_delay) {
+			// Monsters set their AI inactive instead of having a walkdelay
+			if (md != nullptr)
+				md->next_thinktime = tick + status_get_amotion(src);
+			else
+				unit_set_walkdelay(src, tick, sstatus->amotion, 1);
+		}
 	}
 
 	if(ud->state.attack_continue) {
@@ -3161,8 +3157,13 @@ int32 unit_skillcastcancel(struct block_list *bl, char type)
 		}
 	}
 
-	if(bl->type==BL_MOB)
-		((TBL_MOB*)bl)->skill_idx = -1;
+	if (bl->type == BL_MOB) {
+		mob_data& md = reinterpret_cast<mob_data&>(*bl);
+		// Sets cooldowns and attack delay
+		// This needs to happen even if the cast was cancelled
+		mobskill_end(md, tick);
+		md.skill_idx = -1;
+	}
 
 	clif_skillcastcancel( *bl );
 
@@ -3440,7 +3441,7 @@ int32 unit_remove_map_(struct block_list *bl, clr_type clrtype, const char* file
 				md->spotted_log[i] = 0;
 
 			md->attacked_id=0;
-			md->state.skillstate= MSS_IDLE;
+			mob_setstate(*md, MSS_IDLE);
 			break;
 		}
 		case BL_PET: {