MSUTeam/MSU

[BUG] onBeforeAnySkillExecuted and onAfterAnySkillExecuted do not guarantee that the skill was actually executed

Opened this issue · 0 comments

Versions

  • MSU: 1.6.0

Describe the bug

onBeforeAnySkillExecuted is a new event introduced by MSU
It's described as

The first function is called before the used skill's use function triggers, whereas the second function is called after the use function is complete.

That is correct with its implementation.

From the name itself one would assume that this only triggers when the skill in question is guaranteed to be used afterwards. It is "about to be executed" afterall.
In Reality there are still some checks in the vanilla use function which can prevent the skill from actually being executed.
Like if the skill is not actually affordable or the targeting does not check out.

Now the problem is that mods using this event can't know if the skill in question is actually used for sure.
I would even go so far as to say there is no Event in Vanilla/MSU that guarantees fire when a skill is executed (it's onUsed is called). Currently we can only rely on the two MSU functions and those have false positives

Expected behavior

The onBeforeAnySkillExecuted event should only fire when the skill is really executed and not cancelled last second (red arrow)
The onAfterAnySkillExecuted event should only fire when the skill is really executed and not cancelled last second (green arrow)

image

Possible Solution

Do a hookTree with the latest possible timing around the onUse function of skill.nut.
Then you guarantee that the skill actually executed.

However some mods might hard-call this function. Then MSU would fire new types of false-positves. So this solution still needs some code around the initial use function flipping some variable.

Alternative I considered

Do a normal hook of the setPreviewSkillID function from actor.nut and check the stack whether it was called by use. If so then we trigger onBeforeAnySkillExecuted and set a global skillActuallyExecuted variable to true.
Then after use from skill.nut ended, we check whether our global variable skillActuallyExecuted is true. If it is, we trigger onAfterAnySkillExecuted