[boskid][2.0.30] Surface conditions of certain entities share the same table reference - changing one changes others!

[Natha]

In my mod, I want to restrict only rail ramps, supports and elevated rails from being built in my custom surface.
However, applying surface conditions to only these entities makes them apply to all train-related entities like rails, stops, vehicles. I hope this is not intended.

[Loewchen]

Post the mod and add the version of the game to the title please, see 3638.

[Dullstar]

I'm not the OP, but I was the one who notified them that the intended set of restricted entities didn't match what was actually restricted in-game (as far as I'm aware it specifically happens when using the mod with Space Age). I do not know whether the mod or the game is causing the problem, but since the author has reported it here I will contribute what information I can.

The affected mod is this one. It basically allows you to dig underneath a base, creating an underground surface, so I believe the intent of the restriction is "you can't build elevated rails underground" presumably because the tunnels aren't tall enough to fit them. Other than the issue with rails, its basic functionality works correctly in Space Age.

Game version: 2.0.30

I did a little digging through the mod's source code to see if I could find any obvious issues and I can verify that the mod only explicitly attempts to restrict the following rail-related entities: "rail-ramp", "rail-support", and "artillery-wagon". In order to edit the mod to remove the restriction, I had to remove all three from the list of entities to be given the "you can't build this underground" restriction. However, I am not particularly experienced with the mod API/Lua so I can't be fully certain the mod author did it correctly beyond "well I don't see anything obviously wrong with it."

I attempted to add some logging to see exactly what it sets, as it does appear to be attempting to grab categories of items presumably for compatibility with items from other mods, but the only additional rail-related items I saw in the log were "dummy-rail-ramp" and "dummy-rail-support", so I don't really understand where the restrictions are getting applied to things like non-elevated rails, cargo/fluid wagons, stations, etc. All the items I'm seeing in the log look correct and I can't find anywhere else in the mod's code where items are given the specific requirement that appears in game.

If we assume the game is behaving correctly, I have some more ideas on things to try to fix the mod, but that's going to take some more time. If I manage to find a way to fix the mod, I'll post an update here.

[Muche]

When examining what effect some piece of search-and-alter code does (during data phases), it might be useful to dump the whole data structure.
Run "factorio --dump-data", this will create script-output/data-raw-dump.json.
Rename/move it, change the code under investigation, run the command again, compare the two outputs.

[Natha]

@Dullstar is totally right with everything. For rail-related entities, only those of type "rail-ramp", "rail-support" and "artillery-wagon" get the surface condition.

When examining what effect some piece of search-and-alter code does (during data phases), it might be useful to dump the whole data structure.
Run "factorio --dump-data", this will create script-output/data-raw-dump.json.
Rename/move it, change the code under investigation, run the command again, compare the two outputs.

Unfo I can't do that because I do not own the DLC.

[Muche]

I followed Dullstar's example of removing mentions of rail-ramp, rail-support, artillery-wagon from the list of entities altered by the mod.
Running & comparing the dump showed that adding one of them affects (i.e. adds surface condition subsurface-level:max=0) these entities: artillery-wagon, car, cargo-wagon, curved-rail-a, curved-rail-b, elevated-curved-rail-a, elevated-curved-rail-b, elevated-half-diagonal-rail, elevated-straight-rail, fluid-wagon, half-diagonal-rail, legacy-curved-rail, legacy-straight-rail, locomotive, rail-chain-signal, rail-ramp, rail-signal, rail-support, spider-vehicle, straight-rail, tank, train-stop.
This is due to all these entities sharing the surface condition table, set in data/space-age/base-data-updates.lua: data.raw[type][name].surface_conditions = one_gravity_condition.

A solution could be

Code: Select all

--- Subsurface/data-final-fixes.lua
+++ Subsurface/data-final-fixes.lua
@@ -144,7 +144,7 @@
 if feature_flags["space_travel"] then
 	for name, type in pairs(data.subsurface_entity_restrictions) do
 		if data.raw[type][name] then
-			data.raw[type][name].surface_conditions = data.raw[type][name].surface_conditions or {}
+			data.raw[type][name].surface_conditions = table.deepcopy(data.raw[type][name].surface_conditions) or {}
 			table.insert(data.raw[type][name].surface_conditions, {property = "subsurface-level", max = 0})
 		end
 	end

[Natha]

I followed Dullstar's example of removing mentions of rail-ramp, rail-support, artillery-wagon from the list of entities altered by the mod.
Running & comparing the dump showed that adding one of them affects (i.e. adds surface condition subsurface-level:max=0) these entities: artillery-wagon, car, cargo-wagon, curved-rail-a, curved-rail-b, elevated-curved-rail-a, elevated-curved-rail-b, elevated-half-diagonal-rail, elevated-straight-rail, fluid-wagon, half-diagonal-rail, legacy-curved-rail, legacy-straight-rail, locomotive, rail-chain-signal, rail-ramp, rail-signal, rail-support, spider-vehicle, straight-rail, tank, train-stop.
This is due to all these entities sharing the surface condition table, set in data/space-age/base-data-updates.lua: data.raw[type][name].surface_conditions = one_gravity_condition.

A solution could be

Code: Select all

--- Subsurface/data-final-fixes.lua
+++ Subsurface/data-final-fixes.lua
@@ -144,7 +144,7 @@
 if feature_flags["space_travel"] then
 	for name, type in pairs(data.subsurface_entity_restrictions) do
 		if data.raw[type][name] then
-			data.raw[type][name].surface_conditions = data.raw[type][name].surface_conditions or {}
+			data.raw[type][name].surface_conditions = table.deepcopy(data.raw[type][name].surface_conditions) or {}
 			table.insert(data.raw[type][name].surface_conditions, {property = "subsurface-level", max = 0})
 		end
 	end

Well done! Good to know that. The solution you proposed should work.

I changed the title of this topic to a better description.

@Devs: To avoid this issue for other modders and to simplify scripting (no permanent deep-copying), you should do this deep-copying in data/space-age/base-data-updates.lua to make sure all entities have their own table.

[boskid]

Shared tables in data is a slippery topic and i do not think we will ever make them guaranteed independent because there is a lot of helper functions for generating sound definitions, sprite definitions etc. With one_gravity_condition and ten_gravity_condition since those are 2 locals affecting one mod (by being local, its not possible they can be used by other mods) i changed them to be local functions instead that generate independent tables which should fix this specific issue, but there are about 1700 other shared tables that are often reused between prototypes (my tiny script throws about 12000 lines of errors for vanilla+SA). For 2.0.33 surface_conditions are decoupled, but since i am considering this a minor change not affecting anything i am not even adding changelog record for this (in my classification this is not a bug, but i did the change just because i already looked at the lua files). Other mods can easily create prototypes with shared tables as well and the most sane approach i think is to write mods in a way that are working correctly regardless if the data sits in a shared table or in an independent table.