search

CleverRaven / Cataclysm-DDA

Cataclysm - Dark Days Ahead. A turn-based survival game set in a post-apocalyptic world.
http://cataclysmdda.org
Other
10.26k stars 4.12k forks source link

ATMs spawned en masse #76200

Closed JohnHenryEton closed 14 hours ago

JohnHenryEton commented 1 week ago

Describe the bug

After downloading a newer version from the experimental branch, ATMs are appearing in unexpected places. Possibly only in previously explored areas. Going through walls, other unoccupied areas, etc. At first I noticed it in a newer area I was looting, but as I was returning home I found a spot where I was sure there weren't loads of ATMs before. I'm guessing it's a bug.

Attach save file

test-trimmed.tar.gz

Steps to reproduce

Drive around to previously explored places that spawn ATMs. or teleport to one like gas stations? Get invaded by the ATM hoard?

Expected behavior

Not being invaded by the ATMtide

Screenshots

Screenshot 2024-09-04 151048

Versions and configuration

Additional context

No response

GuardianDll commented 1 week ago

Can't reproduce on the most recent experimental i suspect the issue is in some of out of repo mod, please confirm it happens in vanilla

JohnHenryEton commented 1 week ago

I couldn't get it to happen on a new save at all. Either with my current modlist or otherwise. Only happened with an ongoing save, and I don't have another ongoing save.

HadeanLake commented 1 week ago
GuardianDll commented 1 week ago

something fishy going on is it ter_furn_migration from #76051 causes some issues on game load?

JohnHenryEton commented 1 week ago

I was about to comment that I had just replicated the issue by downgrading to a build from the 2nd, finding an atm, upgrading again with a repo only list.

HadeanLake commented 1 week ago

is it ter_furn_migration from https://github.com/CleverRaven/Cataclysm-DDA/pull/76051 causes some issues on game load?

Yes. "to_ter" is missing in ter_furn_migration JSON (causes terrain to be nothing under first one), and code processing it does not reset furniture after placing it once

Here is rough and only briefly tested draft patch to fix this:

--- a/data/json/obsoletion_and_migration_0.I/obsolete_furniture.json
+++ b/data/json/obsoletion_and_migration_0.I/obsolete_furniture.json
@@ -91,6 +91,7 @@
   {
     "type": "ter_furn_migration",
     "from_ter": "t_atm",
+    "to_ter": "t_thconc_floor",
     "to_furn": "f_atm_off"
   }
 ]  

--- a/src/savegame_json.cpp
+++ b/src/savegame_json.cpp
@@ -4925,6 +4925,9 @@ void submap::load( const JsonValue &jv, const std::string &member_name, int vers
                                     iid_furn = it->second.second.id();
                                 }
                             }
+                            else {
+                                iid_furn = furn_str_id::NULL_ID().id();
+                            }
                             if( terstr.is_valid() ) {
                                 iid_ter = terstr.id();
                             } else {

there is probably a better way to reset iid_furn

JohnHenryEton commented 1 week ago

Is there a good way to remove these ATMs from a save when this gets fixed?

CoroNaut commented 1 week ago

to remove furniture:

  1. debug menu
  2. map
  3. map editor (disables achievements)
  4. scroll overtop your ATM
  5. 'r' furniture
  6. '/' filter for 'null' (optional to make it easier to get to the 'nothing' furniture)
  7. select "nothing" furniture.
  8. repeat for all other ATM's

Alternatively, you can just regenerate the whole overmap chunk.

  1. debug menu
  2. map
  3. map editor
  4. 'o' overmap/mapgen
  5. '/' filter for "s_gas_1_north" (im pretty sure this is the exact gas station in your picture)
  6. '3' Apply
mqrause commented 1 week ago

there is probably a better way to reset iid_furn

To me it looks like both iid_ter and iid_furn should only be declared inside the loops, so they reset automatically.

HadeanLake commented 1 week ago

should only be declared inside the loops,

Their value is used outside of the iteration it was assigned. Game checks if next tiles are same, writes amount of same tiles into remaining var and uses it to skip some logic

checking !remaining again before iteration ends and resetting iid_furn there (where we had run out of same tiles to assign it to) fixes it, but the whole thing probably needs a rewrite

--- a/src/savegame_json.cpp
+++ b/src/savegame_json.cpp
@@ -4951,6 +4951,9 @@ void submap::load( const JsonValue &jv, const std::string &member_name, int vers
                     if( iid_furn ) {
                         m->frn[i][j] = iid_furn;
+                        if ( !remaining ) {
+                            iid_furn = furn_str_id::NULL_ID().id();
+                        }
                     }
                 }
             }
             if( remaining ) {

When testing fix for this, it is important to have multiple ATM's in a row and one on the next line. I tested using this save: 2024-09-05-124101_1920x1080_scrot ATM corruption-trimmed.tar.gz

mqrause commented 1 week ago

Their value is used outside of the iteration it was assigned. Game checks if next tiles are same, writes amount of same tiles into remaining var and uses it to skip some logic

Oh, right, I was misreading things. So instead of resetting iid_furn, I'd suggest simply always setting it, and not just if it's not the NULL_ID. (And maybe also add a check if the furniture id is actually valid.)

JohnHenryEton commented 1 week ago

to remove furniture:

1. debug menu

2. map

3. map editor (disables achievements)

4. scroll overtop your ATM

5. 'r' furniture

6. '/' filter for 'null' (optional to make it easier to get to the 'nothing' furniture)

7. select "nothing" furniture.

8. repeat for all other ATM's

Alternatively, you can just regenerate the whole overmap chunk.

1. debug menu

2. map

3. map editor

4. 'o' overmap/mapgen

5. '/' filter for "s_gas_1_north" (im pretty sure this is the exact gas station in your picture)

6. '3' Apply

The latter is probably better given the number of spawned ATMs, thanks.