`DaedalusVM: base address of C_Class is nullptr: C_NPC.AIVAR` when loading up Gothic 2

[lmichaelis]

Description

When loading Gothic II Gold Classic from Steam and starting a new game, the following warnings are printed to the console:

OpenGothic v1.0 dev
GPU = AMD RADV SIENNA_CICHLID
Depth format = 13 Shadow format = 13
Info: ZEN: Reading presets...
Info: ZEN: Reading world...
Info: oCWorld reading chunk: MeshAndBsp
Info: ZEN: Reading mesh...
Info: Reading mesh '' (Version: 265)
Info: Found 260696 vertices
Info: ZEN: Done reading mesh!
Info: oCWorld reading chunk: VobTree
Info: oCWorld reading chunk: WayNet
Info: Loading 1 freepoints
Info: Loading 3402 edges
Info: Done loading edges!
Info: ZEN: Reading presets...
unable to load sound fx: ENV_NIGHT_TONSOFINSECTS
unable to load sound fx: OW_BIRD11
Info: Reading 20826 blocks
lib/ZenLib/daedalus/DATFile.h(338):[T& Daedalus::PARSymbol::getValue(size_t, void*) [with T = int; size_t = long unsigned int]] DaedalusVM: base address of C_Class is nullptr: C_NPC.AIVAR
lib/ZenLib/daedalus/DATFile.h(338):[T& Daedalus::PARSymbol::getValue(size_t, void*) [with T = int; size_t = long unsigned int]] DaedalusVM: base address of C_Class is nullptr: C_NPC.AIVAR
lib/ZenLib/daedalus/DATFile.h(338):[T& Daedalus::PARSymbol::getValue(size_t, void*) [with T = int; size_t = long unsigned int]] DaedalusVM: base address of C_Class is nullptr: C_NPC.AIVAR
lib/ZenLib/daedalus/DATFile.h(338):[T& Daedalus::PARSymbol::getValue(size_t, void*) [with T = int; size_t = long unsigned int]] DaedalusVM: base address of C_Class is nullptr: C_NPC.AIVAR
lib/ZenLib/daedalus/DATFile.h(338):[T& Daedalus::PARSymbol::getValue(size_t, void*) [with T = int; size_t = long unsigned int]] DaedalusVM: base address of C_Class is nullptr: C_NPC.AIVAR
lib/ZenLib/daedalus/DATFile.h(338):[T& Daedalus::PARSymbol::getValue(size_t, void*) [with T = int; size_t = long unsigned int]] DaedalusVM: base address of C_Class is nullptr: C_NPC.AIVAR
lib/ZenLib/daedalus/DATFile.h(338):[T& Daedalus::PARSymbol::getValue(size_t, void*) [with T = int; size_t = long unsigned int]] DaedalusVM: base address of C_Class is nullptr: C_NPC.AIVAR
lib/ZenLib/daedalus/DATFile.h(338):[T& Daedalus::PARSymbol::getValue(size_t, void*) [with T = int; size_t = long unsigned int]] DaedalusVM: base address of C_Class is nullptr: C_NPC.AIVAR
lib/ZenLib/daedalus/DATFile.h(338):[T& Daedalus::PARSymbol::getValue(size_t, void*) [with T = int; size_t = long unsigned int]] DaedalusVM: base address of C_Class is nullptr: C_NPC.AIVAR

This is an issue related to calls to ZS_DEAD (GOTHIC.DAT) from within Npc::tickRoutine calling GameScript::invokeState and caused by OTHER not being set in the script. While ZenLib avoids a crash by simply returning 0 if the instance is not set, this behavior seems incorrect and should be fixed by setting the OTHER instance correctly.

Additionally, this behavior can not and should not be implemented by phoenix, so a it should be fixed in OpenGothic to avoid crashes in #271.

Possible Solution

I have tried the uncommenting this line. At least in #271, this fixes this particular problem and allows for the opening dialog to play out as intended. It does not seem to help with the warning messages when on master, however.

Another solution might be to set OTHER to owner.player(). In the context of this particular issue, that would make sense since the player is the only person relevant to the conversation as far as I can tell.

Versions

• Gothic II Gold Classic (Steam; German Version)
• OpenGothic@5485f84
• ZenLib@22a0f1f

[Try]

Hi, @lmichaelis !

Additionally, this behavior can not and should not be implemented by phoenix,

I think it's better to have this type of behavior: null-values for instances can pop-in, if script poorly written. In ZS_Attack_Loop there is a buggy line, when script does: if(Hlp_IsValidNpc(other) && .. && (other.aivar[AIV_INVINCIBLE] == FALSE) ..) What is a bug, since short-expression evaluation is not supported by Daedalus language.

Another solution might be to set OTHER to owner.player()

This will misbehave with ZS_DEAD script. Npc that are dead due to the story will give XP to player:

func void ZS_Dead()
{
	self.aivar[AIV_RANSACKED] = FALSE;
	self.aivar[AIV_PARTYMEMBER] = FALSE;
	B_StopLookAt(self);
	AI_StopPointAt(self);
	if((Npc_IsPlayer(other) || (other.aivar[AIV_PARTYMEMBER] == TRUE)) && (self.aivar[AIV_VictoryXPGiven] == FALSE))
	{
		B_GivePlayerXP(self.level * XP_PER_VICTORY); // <-----
		self.aivar[AIV_VictoryXPGiven] = TRUE;
	};

I have tried the uncommenting this line.

Seems to be broken link - pointing just to the file

[lmichaelis]

Oh, I'm sorry this is the correct link.

Then I believe we're dealing with two bugs here. First the bug in the script itself and second another bug in the original game's VM implementation.

This is basically undefined behaviour like in the C++-world. It doesn't make sense to me to just generate random values (in this case zeroes) when accessing members on invalid instances. There has to be another way around this.

I know this is not the greatest solution but could we not add a special case in invokeState where we check for the called function being ZS_DEAD and OTHER being NULL and then just not call the script? In other words: Does the script do anything important if OTHER is invalid?

I shouldn't have said it can not be implemented. What I was trying to say was that it would require a breaking API change which I guess isn't that big of an issue at the moment.

[Try]

This is basically undefined behaviour like in the C++-world.

Yes, it is. But script is not a C++, and player shouldn't hit game-crash(or volubility risk), if script is written poorly.

being ZS_DEAD and OTHER being NULL and then just not call the script? Does the script do anything important if OTHER is invalid?

We don't know generally, since ZS_DEAD can be overwritten. It's better not to assume anything about script code. Also existence of ZS_DEAD in engine code is issue on it's own.

I shouldn't have said it can not be implemented.

Can you go in detail why this is a difficult to implement? Can it be handled by something like, exception throwing?

[lmichaelis]

Can it be handled by something like, exception throwing?

Yes, that is how it is handled right now. I am concerned about performance using exceptions, however since this kind of thing happens quite often it seems.

After thinking about it a bit more I might have found two possible workarounds which I am currently testing:

1. Allow for specification of certain flags in the VM which allow for controlling its behaviour from the outside (for example, popping zeroes when the stack is empty or not).
2. Allow the engine to register an error handler (not using exceptions) which can change the VM state (ie. push a 0 onto the stack). This is very powerful but goes against the design of the system. It is designed to be type safe which can't be guaranteed at such a low level.

Is there any reason to use one over the other from your point of view? Would it be helpful to you if you were able to change the VM's state after script errors are encountered?

Regardless, since this issue isn't really related to OpenGothic any more I'll close it. Thanks for the information :)

[Try]

Option 2 makes more sense to me. Specially, if we would expand to Ikarus/LeGo side eventually, with need to intercept 'bad' access.

Since you mention stack: want to point out a few other cases, that can also be problematic:

1. Throwing exception(and any early-stop) in VM may leave stack with more element, that were before calling routine. In OpenGothic there is CallStackFrame object, that manages those cases.
2. Some functions of original game lacking of return statement, that also causes stack corruption.