Game crashes if you switch maps while talker window is open

[makrohn]

If the dialog window is open and you switch maps, the game crashes. I found it in Polymorphable, and I've devised the following method to test the fix in FLARE (by which steps I have also confirmed that the bug DOES indeed exist in FLARE). Issue does NOT occur with vendor window open, just talker window.

Recommended troubleshooting steps would be to add

in frontier_outpost.txt

[event]
type=teleport
location=23,15,1,1
intermap=warp_zone.txt,30,36

in warp_zone.txt (not necessary to duplicate the bug, but handy to get back to frontier_outpost to retest the issue)

[event]
type=teleport
location=29,36,1,1
intermap=frontier_outpost.txt,22,15

Then talk to Martigan and move next to the tent. When you reload the game, you'll be in the warp zone.

[stefanbeller]

So I added these 2 warps and while talking to Martigan I get warped. The game doesn't crash here, but the window of the npc stays open.

[stefanbeller]

moving back and forth between the warps, crashes the game.

[clintbellanger]

When maps are changed, NPC Manager unloads all the current NPCs:

https://github.com/clintbellanger/flare/blob/master/src/NPCManager.cpp#L66

So any menu that directly accesses NPC data (or has e.g. an NPC pointer) could crash. Either way, to be sensible, we should at least close the proximity-based menus (Stash, Vendor, Talker) before changing maps.

https://github.com/clintbellanger/flare/blob/master/src/GameStatePlay.cpp#L210

We can add logic here to close those three menus before that map->load() call.

[igorko]

Yeah. And put sfx there like "maan, you say such stupid things, I'm so boored. I'm out!" :D If seriously, when the game writes received by NPC campaign/quest statuses? We can break some statuses if not listen NPC dialog to the end. Am I right?

[stefanbeller]

@clintbellanger: The vendor already sets visibility to false. That's the way to tell them to stop right? The other two, Stash and Talker, have been added.

[stefanbeller]

@igorko: That should be dealt with by the content creator, to set and unset the events at the right points in time. Speaking of the engine, I'd rather document that the dialogs can be closed at any point in time, i.e. while being displayed. It could also happen due to pressing ALT+ F4 so we cannot give any guaranties at all.

[makrohn]

@igorko I was just playing with this in Poly minutes ago - I'm finding it's good to make sure to load ALL rewards, removes, sets, unsets, etc either at the beginning or the end of the dialog - otherwise you can unset or set a status, leave the dialog before it ends, and then not receive the status/item/etc at the end of the dialog that you need in order to proceed. Interleaving dialog with events within a [dialog] is not-so-good. I would agree with Stefan that this should be on the content creator, but it should still be documented somewhere for modders - "Why does my quest break?"

@stefanbeller I did notice that Vendor didn't break maploads.

[clintbellanger]

As makrohn said: It's a good practice to give all rewards and quest statuses right before the first or final him/her talking line. It's the same as when the player walks away from conversation: the modder needs to take care to make the order of events generally work even if interruptions happen.