SCI: QFG1: Running on ice till health points depletion won't kill hero.

[OmerMor]

First reported here:
​https://twitter.com/darthhelmet86/status/796973009558913024

Appears to be a script bug.

When in room 58, and type "run", the hero will fall and his HP will decrease by 1 point. This can be repeated, but will never cause the hero to die.

[OmerMor]

This is reproducible in QFG1VGA as well.

[OmerMor]

When typing "run" the ego will be assigned with the script egoRuns.
egoRuns::changeState calls proc0_36 in script 0 which is deducing damage from the hero's HP.
This procedure returns TRUE if the hero is still alive, but the return value is never observed in egoRuns.

[OmerMor]

In QFG1VGA, it's pretty much the same.
The main difference is that the procedure that takes the damage is proc814_19 in script 814.

[OmerMor]

Here's the content of the ​twitter report, by ​DarthWizard86:

Uhhhh still alive have 0 health. Running on the ice takes off 1 health till you have 0 then you don't die?

[sluicebox]

I'm putting this bug "on notice" for being closed as WONTFIX if we can't come up with something in a reasonable amount of time.

This is a very real script bug. Omer did a great job in documenting this; they called their function that deducted health and then didn't write any code to test the return value and handle it, presumably, by calling the death function with an appropriate message.

So what are we to do? Which message should we display when dying? That question assumes there's even room to patch in all that new code, which we'd have to do twice for the original and the remake, and it's likely there's not enough room to do that in either.

I hope we can fix this! At the same time, I see that QFG1VGA calls proc814_19 frequently without testing the return value. I really like this game, but it seems like they screwed up basic stat stuff like this all the time.

[OmerMor]

So what are we to do? Which message should we display when dying?

How about message 57 "That was a little too much for you. You collapse from exhaustion and die." ?

[ZvikaZ]

I have added a new patching mechanism, hooks to VM, that allows adding new instructions.
It was merged at ​https://github.com/scummvm/scummvm/pull/2109

Now engines\sci\engine\vm_hooks.cpp contains a patch that fixes QFG1 (not VGA) according to @OmerMor's detailed description.

However, the question is how do we continue?
@sluicebox said that there are more scenerios like this at QFG1VGA, so I see two alternatives:

• go over all these places, and add a patch similar to the one that I already did
• or, patch proc814_19, and if needed, kill the hero from there (instead of checking all places that call proc814_19 for return value, and kill him if needed). However, maybe there are places that call proc814_19, and the hero *shouldn't* die, on purpose?

Also, it's not clear if there are more problems at legacy QFG1, or is it the only one.

@sluicebox, @OmerMor, I understand that you've already investigated this bug, so, what do you think?

[OmerMor]

Very nice work @ZvikaZ!

Patching proc814_19 seems wrong to me: if you kill the hero, you need context to show the right message to the player.

The right approach IMO is to go over each call-site of proc814_19, see if it does not check the return value, and handle it appropriately.

[Tucky27]

I tried the running on the ice myself and my character died and then had a death message so all good. One thing I noticed through was the animation. Every time the character slips he gets up, including the last time. He literally dies on his feet from slipping. If the animation could be halted when the hero is on the ground it would at least not look weird.