Derpumu / Pacifist

A Factorio mod to remove military items, recipes, and technologies so you can concentrate on building useful things in peaceful mode.
MIT License
2 stars 1 forks source link

Hide orphaned entities #58

Closed dperelman closed 3 months ago

dperelman commented 3 months ago

Search through data.raw to find everything that was referenced before Pacifist removed whatever it removes and is no longer referenced after and remove all of those things as well. This effectively removes things like corpses/remnants of removed entities and particles/explosions related to weapons/combat robots that get removed. This PR also adds some new dummies to cover some categories that now end up empty. (Note the dummies change may conflict with #57, but they should be straight-forward to merge cleanly as the changes are actually either identical or just next to each other, not overlapping.)

Note the way this checks for references just looks for any strings matching the names of entries in data.raw, which technically is an overapproximation, meaning that it may think something is referenced when it isn't, which would just result in that thing actually being left in (and therefore visible in the rich text icon selection GUI) accidentally. This is unlikely to happen in practice. The "right" solution would be to determine which strings are EntityIDs or whatever *IDs and only treat those as references, but that seems like overkill here. (I might write the code to do that anyway since someone asked for it in the Factorio Discord when I asked about this, but would probably not bother using it for this project.)

After this pass, there are some explosions left for friendly buildings getting damaged and the atomic bomb explosion for the reactor being destroyed. Those explosions started showing up in the rich text icon selection GUI, so I added a couple lines to PacifistMod.remove_misc() to hide them. I then confirmed that destroying a reactor still produces an atomic bomb explosion as expected.

Together with #57, this should fix #56.

Derpumu commented 3 months ago

I tried to resolve the conflicts after merging the other PR, but now I get this and am not quite sure what the reason is: image

dperelman commented 3 months ago

I couldn't reproduce that error message. But when investigating I noticed that damage-type references are in fields of name type so they weren't getting handled properly anyway since I was ignoring all fields of name type because they're usually the type of that object, not a reference to something else. So I changed the code to never delete damage-types. DamageType can't reference other things anyway, so this won't result in keeping other objects around.

At the same time, it occurred to me that this code shouldn't delete anything labeled as an exception due to the mod rules, so I added a check for that as well. Not sure it does anything, but it would be confusing if it accidentally overrode the exceptions.

Derpumu commented 3 months ago

I'll have a look at it again in the next days.

Derpumu commented 3 months ago

Testing this locally I get errors (tip of your branch, revision 0e0a647):

AAI Industry (runtime, directly after starting a new game):

image apparently the algorithm removes the "fire-flame" entity that is no longer referenced in the data stage but used at runtime

Exotic Industries (uncovering the map):

image The mod spawns alien sites that contain at least destroyer remnants and distractor remnants To reproduce, start a game with Exotic Industries and Editor Extensions mods and fly around a while (zoom out a lot to fly faster and uncover a lot of map chunks)

Seablock modpack (reskins-bobs mod, while loading the mods):

image Apparently the algorithm did not find the defender-capsule projectile that references the defender entity

dperelman commented 3 months ago

Okay, sounds like this strategy doesn't work in general as we can't tell which entities mods will reference by name in their own code. The options I see are:

  1. Abandon this.
  2. Hide instead of remove. (I just pushed this version)
  3. For each mod with compatibility issues, hard code a list of entities that have references (these would be added to the exceptions collection).

(3) sounds brittle and like a lot of work for minor gain, so I'd lean towards (2). Noting that if it accidentally hides extra things, we can add them to the exceptions, but I doubt that's likely to be a problem.

One problem with (2) is that it doesn't hide combat robots, so I want to extend it with an option to "pretend delete" entities, so it will hide combat robots and everything they reference since removing them and everything they reference isn't working for all mods. (EDIT: 69ff220d7ac3f7ee56f3d5f8f0814f43260b7042 does that.)

(I saw that SeaBlock error before on a different branch and fixed it by marking SeaBlock as a dependency so it would load before Pacifist... but then I got a different error, which I why I never pushed that change.)

dperelman commented 3 months ago

With these changes, I got a similar error on Exotic Industries about stone-wall (that is, once I made sure destroyer-remnants wasn't being removed. Adding it to mods_require_walls fixed it, but I'm not sure what's going on if you had tested it before and not needed that, so I did not push that change.

EDIT: Confirmed I get this same error on Pacifist main.

Derpumu commented 3 months ago

I'll run it against my test save games later this week when I'm back home. I am thinking about logging which types of things get hidden by this and maybe restrict the algorithm to things like remnants etc that actually show up in the signals and RTF icons.

The defender remnants maybe should be added to the exceptions for Exotic Industries either way, but I'll look into that later as well.

Could you tell me how to reproduce the stone-wall issue with EI? I've been running a save with EI and Pacifist for a while without issues. Maybe you have different mods or mod settings?

In any case, thank you again for the work!