[Not a bug] [3pre-307-g20e61ead4] Abort with LevelCompat

Bugs that have been investigated and resolved somehow.

Moderator: GZDoom Developers

[3pre-307-g20e61ead4] Abort with LevelCompat

Postby Dinosaur_Nerd » Fri Nov 08, 2019 12:40 am

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 allExpand view
enable_handler
enable_ACS

these are booleans

This was tested using the GZDoom development build found here
User avatar
Dinosaur_Nerd
RAWR!
 
Joined: 21 Feb 2018
Location: timelocked USA 1994
Discord: https://discord.gg/7Dd8GCD
Operating System: Windows 10/8.1/8 64-bit
Graphics Processor: nVidia (Modern GZDoom)

Re: [3pre-307-g20e61ead4] Abort with LevelCompat

Postby phantombeta » Fri Nov 08, 2019 1:20 am

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.
User avatar
phantombeta
In the meadow of sinful thoughts, every flower's a perfect one
 
Joined: 02 May 2013
Location: The United Soviet Socialist Dictatorship of Hueland
Discord: phantombeta#2461
Twitch ID: phantombeta_
Github ID: Doom2fan
Operating System: Windows 10/8.1/8 64-bit
OS Test Version: No (Using Stable Public Version)
Graphics Processor: nVidia with Vulkan support

Re: [3pre-307-g20e61ead4] Abort with LevelCompat

Postby Graf Zahl » Fri Nov 08, 2019 2:18 am

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.
User avatar
Graf Zahl
Lead GZDoom Developer
Lead GZDoom Developer
 
Joined: 19 Jul 2003
Location: Germany

Re: [3pre-307-g20e61ead4] Abort with LevelCompat

Postby Dinosaur_Nerd » Fri Nov 08, 2019 2:49 am

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.
User avatar
Dinosaur_Nerd
RAWR!
 
Joined: 21 Feb 2018
Location: timelocked USA 1994
Discord: https://discord.gg/7Dd8GCD
Operating System: Windows 10/8.1/8 64-bit
Graphics Processor: nVidia (Modern GZDoom)

Re: [3pre-307-g20e61ead4] Abort with LevelCompat

Postby _mental_ » Fri Nov 08, 2019 3:50 am

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.
_mental_
 
 
 
Joined: 07 Aug 2011

Re: [3pre-307-g20e61ead4] Abort with LevelCompat

Postby phantombeta » Fri Nov 08, 2019 3:53 am

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.
User avatar
phantombeta
In the meadow of sinful thoughts, every flower's a perfect one
 
Joined: 02 May 2013
Location: The United Soviet Socialist Dictatorship of Hueland
Discord: phantombeta#2461
Twitch ID: phantombeta_
Github ID: Doom2fan
Operating System: Windows 10/8.1/8 64-bit
OS Test Version: No (Using Stable Public Version)
Graphics Processor: nVidia with Vulkan support

Re: [3pre-307-g20e61ead4] Abort with LevelCompat

Postby Dinosaur_Nerd » Fri Nov 08, 2019 5:26 am

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.
User avatar
Dinosaur_Nerd
RAWR!
 
Joined: 21 Feb 2018
Location: timelocked USA 1994
Discord: https://discord.gg/7Dd8GCD
Operating System: Windows 10/8.1/8 64-bit
Graphics Processor: nVidia (Modern GZDoom)

Re: [3pre-307-g20e61ead4] Abort with LevelCompat

Postby Graf Zahl » Fri Nov 08, 2019 5:49 am

Dinosaur_Nerd wrote: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.
User avatar
Graf Zahl
Lead GZDoom Developer
Lead GZDoom Developer
 
Joined: 19 Jul 2003
Location: Germany

Re: [3pre-307-g20e61ead4] Abort with LevelCompat

Postby Marisa Kirisame » Fri Nov 08, 2019 6:02 am

Would it work with static handlers, though? Those pretty much exist since engine startup, after all.
User avatar
Marisa Kirisame
ZScript Magician
 
 
 
Joined: 08 Feb 2008
Location: Vigo, Galicia
Discord: Marisa Kirisame#1666
Twitch ID: magusmarisa
Github ID: OrdinaryMagician
Operating System: Other Linux 64-bit
OS Test Version: No (Using Stable Public Version)
Graphics Processor: nVidia with Vulkan support

Re: [3pre-307-g20e61ead4] Abort with LevelCompat

Postby Graf Zahl » Fri Nov 08, 2019 6:32 am

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.
User avatar
Graf Zahl
Lead GZDoom Developer
Lead GZDoom Developer
 
Joined: 19 Jul 2003
Location: Germany


Return to Closed Bugs

Who is online

Users browsing this forum: DotBot, Trendiction.de [Bot] and 2 guests