MSUTeam / MSU

Modding Standards and Utilities for Battle Brothers
21 stars 4 forks source link

refactor: change certain functions to only when alive #338

Open LordMidas opened 6 months ago

LordMidas commented 6 months ago

Some mods may kill certain entities during these functions, e.g. during onMovementFinished because of a certain skill/effect on the entity, which may then cause subsequently updated skills to crash. This change prevents such cases.

Example of a problem that is intended to be fixed by this PR: https://discord.com/channels/855921501286039582/855921501286039585/1210475458165084241

LordMidas commented 5 months ago

Actually on further deliberation I am not sure if this needs to be done. Looking at vanilla skill event functions, there are very few which have the isAlive check. I think ideally the modder should not kill the character during these functions.

In any case, there's also the issue of for example if a skill does the following

function onMovementFinished( _tile )
{
    ::Const.SomeGlobalCounter++;
}

If we make this event alive only then this function won't be called for this skill if a skill before this skill kills the target during that skill's onMovementFinished.

An idea could be that at the start of the skill_container's function we check for the character being alive, but during the call to the skills themselves we don't add that check. But that fails when a skill would kill the actor in onMovementFinished.

So it isn't immediately clear what the best course of action here is. So I'd wait on this PR for now.