From df6fabd4d9f10a54a979038712d3a9312ee13fbd Mon Sep 17 00:00:00 2001
From: LordMidas <55047920+LordMidas@users.noreply.github.com>
Date: Tue, 15 Apr 2025 01:46:50 +0100
Subject: [PATCH 1/2] feat: re implement onMovementStep to be able to affect
 actor movement

This makes the function actually useful. It allows skills to stop an actor's movement at a particular tile based on conditions defined in the skill's `onMovementStep` function. Note that onMovementStep is called before moving to a tile. As long as all skills' onMovementStep returns true or null, the movement is valid. Otherwise:
- If even a single skill returns false, the movement is stopped at that tile prematurely.
- The skill can also choose to return a function which will then be executed and also cause the movement to stop. This function can be useful for example for refunding complete or partial movement costs or to trigger any other effects.

Note: Only the first skill that returns a movement stopping condition is considered. Therefore, skill order is important.
---
 msu/hooks/entity/tactical/actor.nut  |  9 +-----
 msu/hooks/skills/skill_container.nut | 42 +++++++++++++++++++++++++---
 2 files changed, 39 insertions(+), 12 deletions(-)

diff --git a/msu/hooks/entity/tactical/actor.nut b/msu/hooks/entity/tactical/actor.nut
index 52ad2f60..09105b7d 100644
--- a/msu/hooks/entity/tactical/actor.nut
+++ b/msu/hooks/entity/tactical/actor.nut
@@ -21,14 +21,7 @@
 
 	q.onMovementStep = @(__original) function( _tile, _levelDifference )
 	{
-		local ret = __original(_tile, _levelDifference);
-
-		if (ret)
-		{
-			this.m.Skills.onMovementStep(_tile, _levelDifference);
-		}
-
-		return ret;
+		return __original(_tile, _levelDifference) && this.m.Skills.onMovementStep(_tile, _levelDifference);
 	}
 
 	// VANILLAFIX: http://battlebrothersgame.com/forums/topic/oncombatstarted-is-not-called-for-ai-characters/
diff --git a/msu/hooks/skills/skill_container.nut b/msu/hooks/skills/skill_container.nut
index a9895f98..9074dd39 100644
--- a/msu/hooks/skills/skill_container.nut
+++ b/msu/hooks/skills/skill_container.nut
@@ -132,10 +132,44 @@
 
 	q.onMovementStep <- function( _tile, _levelDifference )
 	{
-		this.callSkillsFunction("onMovementStep", [
-			_tile,
-			_levelDifference
-		], false);
+		local wasUpdating = this.m.IsUpdating;
+		this.m.IsUpdating = true;
+
+		local result = null;
+		local skill_result = null;
+
+		foreach (s in this.m.Skills)
+		{
+			if (s.isGarbage())
+				continue;
+
+			skill_result = s.onMovementStep(_tile, _levelDifference);
+			if (result == null)
+			{
+				switch (skill_result)
+				{
+					case null:
+					case true:
+						break;
+					case false:
+						result = @(_tile, _levelDifference) null;
+						break;
+					default: // skill_result is a function in this case
+						result = skill_result;
+						break;
+				}
+			}
+		}
+
+		this.m.IsUpdating = wasUpdating;
+
+		if (result != null)
+		{
+			result(_tile, _levelDifference);
+			return false;
+		}
+
+		return true;
 	}
 
 	q.onAnySkillExecuted <- function( _skill, _targetTile, _targetEntity, _forFree )

From 0edca27b3b44cc3c62290366322b27126a371e7a Mon Sep 17 00:00:00 2001
From: LordMidas <55047920+LordMidas@users.noreply.github.com>
Date: Wed, 10 Dec 2025 01:13:25 -0500
Subject: [PATCH 2/2] refactor: add some comments and rename variables

---
 msu/hooks/skills/skill_container.nut | 44 ++++++++++++++++++----------
 1 file changed, 28 insertions(+), 16 deletions(-)

diff --git a/msu/hooks/skills/skill_container.nut b/msu/hooks/skills/skill_container.nut
index 9074dd39..91ab75f4 100644
--- a/msu/hooks/skills/skill_container.nut
+++ b/msu/hooks/skills/skill_container.nut
@@ -135,27 +135,34 @@
 		local wasUpdating = this.m.IsUpdating;
 		this.m.IsUpdating = true;
 
-		local result = null;
-		local skill_result = null;
+		local callback = null;
 
+		local skill, skill_callback; // cache vars for foreach loop
 		foreach (s in this.m.Skills)
 		{
 			if (s.isGarbage())
 				continue;
 
-			skill_result = s.onMovementStep(_tile, _levelDifference);
-			if (result == null)
+			skill_callback = s.onMovementStep(_tile, _levelDifference);
+			if (callback == null)
 			{
-				switch (skill_result)
+				switch (skill_callback)
 				{
+					// null is for legacy support, because older versions didn't have a return
+					// for `onMovementStep` and weren't meant to be able to interrupt movement.
 					case null:
 					case true:
 						break;
-					case false:
-						result = @(_tile, _levelDifference) null;
-						break;
-					default: // skill_result is a function in this case
-						result = skill_result;
+
+					// skill_callback is either `false` or a function in this case.
+					// In this case the skill wants to interrupt the movement.
+					// We will only take skill_callback from the first skill that gives it,
+					// so SkillOrder is important.
+					// The skill_callback can be useful to do functionality after the interruption
+					// e.g. refunding AP or triggering an effect etc.
+					default:
+						callback = skill_callback;
+						skill = s;
 						break;
 				}
 			}
@@ -163,13 +170,18 @@
 
 		this.m.IsUpdating = wasUpdating;
 
-		if (result != null)
-		{
-			result(_tile, _levelDifference);
-			return false;
-		}
+		// callback being null means no skill returned `false` or a function
+		// therefore no skill is interrupting this movement. So we return `true`.
+		if (callback == null)
+			return true;
+
+		// If any skill returned `false` or a function it means it wants to
+		// stop the movement. So, we call the skill's provided callback (if any)
+		// and then return `false`.
+		if (callback != false)
+			callback.call(skill, _tile, _levelDifference);
 
-		return true;
+		return false;
 	}
 
 	q.onAnySkillExecuted <- function( _skill, _targetTile, _targetEntity, _forFree )