search

cataclysmbnteam / Cataclysm-BN

Cataclysm: Bright Nights, A fork/variant of Cataclysm:DDA by CleverRaven.
https://docs.cataclysmbn.org
Other
686 stars 269 forks source link

npc_at_om_location along with u_at_om_location don't register overmap terrains #4122

Open Kenan2000 opened 9 months ago

Kenan2000 commented 9 months ago

Describe the bug

Based on code documentation along with code in https://github.com/cataclysmbnteam/Cataclysm-BN/blob/main/src/condition.cpp#L383 which I presume is supposed to accept overmap terrains as input values JSON parameters npc_at_om_location along with u_at_om_location should accept overmap terrains with like overmap_terrain or without directional specifications like overmap_terrain_north but neither overmap_terrain format nor overmap_terrain_north format work correctly, failing to meet condition making NPCs dialogues that literally depend on that broken along with throwing errors that overmap_terrain is invalid even though it clearly exists which it should not do, but either accept overmap_terrain format or overmap_terrain_north format which is it's intended behavior

Steps To Reproduce

  1. Make an NPC dialogue with a condition that either NPC or one of our main characters or both is at a certain in-game location like [ { "id": "TALK_NPC_GO_TO_EVAC_CENTER1", "type": "talk_topic", "dynamic_line": I will follow you to Evac Center", "responses": [ { "text": "Okay", "topic": "TALK_DONE", "effect": "follow_only" }, { "text": "We are at Evac Center", "topic": "TALK_DONE", "condition": { "npc_at_om_location": "evac_center_3" }, "effect": "stop_following" } ] } ] whereas replacing evac_center_3 with evac_center_3_north makes no difference at all with regards to this
  2. Now try to spawn NPC with this dialogue then go to this in-game location
  3. NPC will follow as expected but error will be thrown that this is an invalid location along with NPC not stopping to follow one our main characters

Screenshots

No response

Versions and configuration

I believe that this is present on all versions as it's a hardcode issue so this is not important, at the same time it's very hard to trace if there were any changes to this file on any of versions to find one which broke it but it seems like this code was not working correctly since it's implementation

Additional context

Yeah, not that much to add to this except for the fact that it has been proven that this is not a JSON error or incorrect usage of it at all

Kenan2000 commented 9 months ago

@Coolthulhu, @chaosvolt, @olanti-p This pull request fixes it that we need to port https://github.com/CleverRaven/Cataclysm-DDA/pull/44844

Kenan2000 commented 9 months ago

Yeah, feel free to add it to list of stuff to port if you guys want to