Interkarma / daggerfall-unity

Open source recreation of Daggerfall in the Unity engine
http://www.dfworkshop.net
MIT License
2.7k stars 327 forks source link

Fixed TalkManager issue causing missing people #2610

Closed John-Leitch closed 2 months ago

John-Leitch commented 6 months ago

This PR covers the following issue: #2604 (1.0: Person fails to appear in "where is" talk UI for quest)

A few details related to the issue can be found in my comments on the bug. The appears to stem from Person.GetAssignedPlaceSymbol returning null. Diving a little deeper, apart from the serialization code, the assigned place symbol is only set via the Person.SetAssignedPlaceSymbol method. In turn, that method is called in two spots:

  1. The PlaceNpc ActionTemplate, which handles place npc statements.
  2. In Person.PlaceAtHome, which is itself called in two spots: a. In Person.Tick when home.IsPlayerHere returns true. This explains the NPC begins resolving once the player enters its home. b. The CreateNpc ActionTemplate, which handles create npc statements.

Both T0C00Y00 and 10C00Y00 (and maybe others) lack place npc or create npc statements for the NPC in question, thus their assigned place is always null.

While this is apparently a regression, searching through the history, I was not able to easily identify the exact revision that introduced this behavior, so was not entirely sure how this case was meant to be handled. Reviewing the code, I noted that Person.homePlaceSymbol was set, despite the NPC not being created. As this value is not public, I added the Person.GetAssignedOrHomePlaceSymbol method, which null coalesces the values. I then updated the appropriate calls in TalkManager, replacing Person.GetAssignedPlaceSymbol with Person.GetAssignedOrHomePlaceSymbol. This appears to work as expected, with the NPC being resolved by map id.

KABoissonneault commented 2 months ago

I have to reject this solution, as it seems intended for NPCs without an assigned place to appear under "Tell Me About" rather than "Where Is", even if they have a Home Place. I'd take a fix to prevents non-placed NPCs from appearing after its building has been loaded once, but that's pretty minor given the complexity