[3pre-307-g20e61ead4] Abort with LevelCompat

[Dinosaur_Nerd]

Levelcompat causes a VM abort when calling to a handler and attempting to write to variables on that handler, when the game is loaded from a save while the player is dead.

To repeat this bug:
1. run this addon with DOOM2.wad from steam
abort test
2. start at MAP01
3. save the game
4. use the kill console command to die
5. press the use key, or load the save
6. VM will abort

notes:
- Calling ACS does not cause any issues.
- If the game is closed while at the abort screen, the game will close to a Very Fatal Error when running asm_jit, this does not happen with asm_jit disabled
- the handler and ACS can be controlled with the CVARS:

Code: Select all

enable_handler
enable_ACS

these are booleans

This was tested using the GZDoom development build found here

[phantombeta]

I don't think this is a bug. I'm pretty sure LevelCompatibility runs before things like deserializing saved classes, so by that point the EventHandler doesn't exist yet.

[Graf Zahl]

LevelCompatibility runs before the level is fully initialized. You cannot run any code here that depends on a level being fully loaded, and calling an event handler from inside this code can trigger so much undefined behavior that even a real crash is something that cannot be avoided. All the LevelCompatibility handler may do is alter the map data in a deterministic fashion, i.e. when you put in a specific map, the output must always be the same.

[Dinosaur_Nerd]

maybe I'm missing something.

Why is this only behaving badly if the game is saved?
Why doesn't this abort on dying and then just restarting back to an autosave at the start of the map?
Why are just manual saves causing trouble here?
Why wouldn't this just abort every single time the level is loaded, dead player or not, if the handler doesn't exist when LevelCompat detects the level and runs the code?
It seems like the handler is getting the message and printing the debug info to the console in this test.

I apologize, I'm just very confused and trying to understand exactly what is going on here.

[_mental_]

When map is (re)started without a saved game, registration of event handlers happens before compatibility processing.
When a saved game is loaded, the same registration happens after compatibility processing.
I would say it's a bug that your sample works when starting without a saved game. VM abort should happen regardless on that.

[phantombeta]

It's behaving like this because it's undefined behaviour. When something is undefined behaviour, that means pretty much anything can happen, and there's basically no way to know just what will happen if you do it.
As you can see from the confusing results you're getting, undefined behaviour makes little sense, and trying to figure out what happens is pretty much a waste of time - the only thing you can do is identify what causes said undefined behaviour.

[Dinosaur_Nerd]

Alright, thanks for the explanations.
Seems a bit strange to me that the order in which these things loads would change just because a player is starting a save game.

Maybe a feature request that the order in which handlers/compat/thinkers and all their functions like OnRegister/CheckReplacement/etc... could be very clearly set and documented? I find it hard to figure out what happens when... but then again, I dunno if setting a clearly marked, hard and fast, no exceptions order for all this in all situations is a good idea or not for gzdoom.

[Graf Zahl]

Alright, thanks for the explanations.
Seems a bit strange to me that the order in which these things loads would change just because a player is starting a save game.

That's because starting from a savegame is not *just*. It's actually a very complex process in which the map gets fully loaded first, then partially uninitialized and overwritten with data from the savegame. Keeping that in sync with stuff like event handlers is not an easy task.

[Marisa the Magician]

Would it work with static handlers, though? Those pretty much exist since engine startup, after all.

[Graf Zahl]

They'd be present, surely - but don't forget that during LevelCompatibility you do not have a working level! All you got is partially initialized data in a state a map manipulator can work with. For example, no actors have been spawned and no static int linedef types been set up, not to mention that the ACS VM is in a state of semi-limbo.
The actual level init happens after the compatibility processing is done.

In short: Running an event handler from in there most definitely qualifies as very much undefined behavior in the purest sense of that term. You may try but you have no guarantees what you get and even less guarantees that the behavior remains stable in future releases.

Better treat this class as a black box - using anything but its own function interface and the basic level structures (vertices, linedefs, sectors, sidedefs) should be considered unsafe and even with the level data, not all fields are guaranteed to produce useful results. It lies in the nature of its reason for existence that it is so limited, but with ZScript being closer to a real programming language than what game scripting normally allows, it is not possible to plug all these holes, at some point the safeguards will stand in the way of legitimate use cases.