dfhack icon indicating copy to clipboard operation
dfhack copied to clipboard

`preserve-rooms`: hangs if there is a cycle in entity position "replaced-by" definitions

Open ab9rf opened this issue 6 months ago • 3 comments

The preserve-rooms tool will hang during fortress startup in an infinite loop if there is a cycle in the "replaced-by" definitions of the fortress entity's position definitions, in plugins/lua/preserve-rooms.lua at lines 324-326:

            while parent.replaced_by ~= -1 do
                parent = positions[parent.replaced_by]
            end

We need to add some sort of protection so that this doesn't hang if the fortress entity has a cycle in its replaced-by definitions. The "Long Night" mod apparently defines positions such that there is a cycle. Whether this is a good idea or not isn't the point; we need to not hang even in the face of what may well be very poor decisions by a modder.

ab9rf avatar Aug 05 '25 13:08 ab9rf

I am torn between simply aborting the search for the terminal node if the search takes too long versus actually detecting cycles and nominating an (arbitrary) member of each cycle as the "terminal" node for that cycle. Unfortunately I don't understand how the terminal node is being used elsewhere to be able to make a good choice as to how to proceed. @myk002, if you have a moment to review this and recommend a solution (as the original developer), that would be of assistance.

ab9rf avatar Aug 06 '25 20:08 ab9rf

I'm going to mitigate this in the short term by having it "break safely" in the presence of a cycle. The mod will probably operate poorly but it should not hang. A long term fix is still needed.

ab9rf avatar Aug 06 '25 23:08 ab9rf

leaving this open as #5542 isn't really a fix, it's just enough to make it not hang. a proper fix is needed but i wanted something we could put out in the next release, which is likely to be imminent

ab9rf avatar Aug 07 '25 00:08 ab9rf