BG issues in scripts

[jastey]

In AR2700.bcs, the last script block is looping:

IF
	GlobalGT("Chapter","GLOBAL",2)
	Exists("Xzar")  // Xzar
	!InParty("Xzar")  // Xzar
	!BeenInParty("Xzar")  // Xzar
THEN
	RESPONSE #100
		ActionOverride("Xzar",DestroySelf())
END

Is there anything we want to do about this? Would it be a "fix" to add a Continue() here?

EDIT: wait, wouldn't it be better anyhow to change this to "InMyArea()"? The reason why these blocks are looping is because "Exists()" give true for joinable NPCs even if they are not in the area. That's not what these script blocks are supposed to check though?

It is only of importance if mods add to the script.

Here is the complete script for a non-moddded game, it's the same for BG:EE and BG:SoD:

Spoiler

IF
    GlobalGT("Chapter","GLOBAL",2)
    Exists("Montaron")  // Montaron
    !InParty("Montaron")  // Montaron
    !BeenInParty("Montaron")  // Montaron
THEN
    RESPONSE #100
        ActionOverride("Montaron",DestroySelf())
        Continue()
END

IF
    Dead("Imoen")  // Imoen
    Exists("Imoen")  // Imoen
    !InParty("Imoen")  // Imoen
    !BeenInParty("Imoen")  // Imoen
THEN
    RESPONSE #100
        ActionOverride("Imoen",DestroySelf())
        Continue()
END

IF
    GlobalGT("Chapter","GLOBAL",2)
    Exists("Xzar")  // Xzar
    !InParty("Xzar")  // Xzar
    !BeenInParty("Xzar")  // Xzar
THEN
    RESPONSE #100
        ActionOverride("Xzar",DestroySelf())
END

 

 

[lynx]

It's the last block, so a Continue wouldn't do anything.

[jastey]

Is a bug only a bug if it shows when a mod is installed? That's kind of philosophical.

[lynx]

Ah, true, there is a tiny race condition of sorts. If the Xzar block fired the same tick you wanted your mod block(s) to fire, it wouldn't work. Only for that one tick, so very unlikely, but yeah, better safe than sorry.

[jastey]

5 minutes ago, lynx said:

Only for that one tick, so very unlikely, but yeah, better safe than sorry.

I'm not sure what you mean. The Xzar block loops and prevents any script block added after it to run at all.

[lynx]

I guess we have different expectations of DestroySelf and I treated it as if it also did something like UnmakeGlobal.

[jastey]

Ah, ok. No it don't, you can DestroySelf() NPCs and then later MoveGlobal just fine. And this script block looping is experience from a player.

[morpheus562]

I think since it is not a bug unless a mod is added, that it should be incumbent on the modder to correct the area script to work with their mod. As is, in the base game, I do not see an issue arising with the script.

[jmerry]

If it is indeed looping, then it's a bug. Clearly, the script was written with the belief that the actions would cause the NPCs to not exist anymore, and thus not trigger again. And then that didn't quite work, so they added "Continue" to the first two blocks as a patch...

Yeah, changing that to an InMyArea() check so that these blocks only remove their NPCs once looks best.

[morpheus562]

Scripts constantly loop, that is what they do. I see no issue with it as written since it will constantly check and remove those three NPCs if the condition is met within the area. Adding a Continue() will allow the script to continue further down if there are more conditions to be met, which there are none in this case. Is there a reported issue with the script removing an NPC when it shouldn't?

[Graion Dilach]

Firmly disagree. This is exactly the legacy cruft is which makes mod maintenance a PITA and this change would need to be tracked in a lot of mods (this breaks BG1NPC (if you delay burying Gorion to Chapter 2), Ascalon's Questpack and I even have a mod which adds some random spawning to this area). It is more straightforward to fix this here and allow other mods to just request for this one.

[jastey]

1 hour ago, morpheus562 said:

I think since it is not a bug unless a mod is added, that it should be incumbent on the modder to correct the area script to work with their mod. As is, in the base game, I do not see an issue arising with the script.

I see what you mean and I would agree in principle. But these kind of "not a bug unless a mod innocently adds something" kind of issues cause a lot of pain and sorrow (and tears and blood!!1) for modders who did not get the memo about "issues in the game you need to consider or your mod won't work properly" <-- which would be exactly what a fixpack is for imho. I agree that it's a border case.

 

57 minutes ago, Graion Dilach said:

this change would need to be tracked in a lot of mods (this breaks BG1NPC (if you delay burying Gorion to Chapter 2), Ascalon's Questpack and I even have a mod which adds some random spawning to this area)

I don't understand what you are saying, do you mean adding a Continue() here would break these mods?

[morpheus562]

12 minutes ago, jastey said:

I see what you mean and I would agree in principle. But these kind of "not a bug unless a mod innocently adds something" kind of issues cause a lot of pain and sorrow (and tears and blood!!1) for modders who did not get the memo about "issues in the game you need to consider or your mod won't work properly" <-- which would be exactly what a fixpack is for imho. I agree that it's a border case.

 

I don't understand what you are saying, do you mean adding a Continue() here would break these mods?

I take it, and please keep me honest @Graion Dilach, he means we should update the script to include the Continue(). I actually have a game past Bandit Camp and forgot about one of the quest packs that adds "the great karlini" or some such. Quest did not work when entering Lion's Way; however, I manually added a Continue() as described above to the area script and the quest triggered. Right place and right time with my game to test, and I can say I had my mind changed.

On another note, what is the script aiming to accomplish with checking if Imoen is dead and has never been in the party? When would that theoretically trigger?

Edited April 12, 2022 by morpheus562

[jastey]

5 minutes ago, morpheus562 said:

one of the quest packs that adds "the great karlini" or some such. Quest did not work when entering Lion's Way; however, I manually added a Continue() as described above to the area script and the quest triggered.

Yes, I just fixed this locally and it's the reason I posted this request/suggestion.

5 minutes ago, morpheus562 said:

Right place and right time with my game to test, and I can say I had my mind changed.

Ha! That's what I meant - of course mods would need to take this into account, I for example changed all my mod additions for this area to an EXTEND_TOP and added Continue()s to my mods' script blcoks for the next updates, but these kind of issues are so cumbersome to debug, especially if every (new or old and forgetful) modder needs to find them anew.

EDIT: I totally agree that adding a Continue() to the last script block looks stupid.

[jastey]

7 minutes ago, morpheus562 said:

what is the script aiming to accomplish with checking if Imoen is dead and has never been in the party? When would that theoretically trigger?

That could be a remnant since she automatically joins the group at the beginning.