Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Quickfort created zones not working and can cause DF to crash. #1758

Closed
cjhammel opened this issue Jan 21, 2021 · 4 comments · Fixed by #1759
Closed

Quickfort created zones not working and can cause DF to crash. #1758

cjhammel opened this issue Jan 21, 2021 · 4 comments · Fixed by #1759
Assignees
Projects

Comments

@cjhammel
Copy link
Contributor

I have been doing a DreamFort run through seeing how the features work in a play mode. I have found that zones created are not working. The animal pen on the surface the dwarfs will not move the dogs to the pen on the stairs nor the zone for the grazing animals. In fact these zones were causing the game to crash very quickly within a few game days after I added the animals. When I deleted the zone and added them via manually the crashing went away and animals were properly penned.

Dwarfs would not add the crutches, splints, bandages, thread to hospital zone on the services level. I deleted the zone add it manually then everything started getting added.

@myk002
Copy link
Member

myk002 commented Jan 23, 2021

There is one strange thing DFHack does when it creates zones that might be relevant here. Buildings.cpp's linkRooms() makes all zones parents of themselves, leading to an infinite loop. It does not cause crashes for me, but it may be a platform thing. The fix is one line - check if bld == room, but I haven't investigated whether the current behavior is correct for non-zones yet.

@myk002
Copy link
Member

myk002 commented Jan 24, 2021

ok, fairly good evidence that this is the problem. ldog posted a savegame that has a reproducible crash when the merchants unload. stack trace shows overflow due to infinite recursion.

removing all zones avoids the crash.

let me investigate how linkRooms() is supposed to work (extent-based buildings like farm plots? overlapping room definitions?) and I'll see if I can come up with a proper fix.

@myk002
Copy link
Member

myk002 commented Jan 24, 2021

Ok, I think I see what's going on here, and why this is only a problem with zones.

linkRooms() is intended to link objects that can be part of a room -- like furniture -- to the building that the room is defined from. It only links parents to children if is_room is set on the parent and the child falls within the parent's extent.

linkRooms() is only called from linkBuilding(), which is only called when a building is being constructed by constructAbstract(), constructWithItems(), or constructWithFilters(). In order for is_room to be set when linkRooms() is called it must be manually set by the caller between the call to allocInstance() and the call to one of the construct*() functions.

Quickfort is the only caller to do this, and it only does this for zones (which must have is_room=true). If any caller were to try to construct a building with a room already defined from it, it would run into the same issue.

I think the fix I proposed in my first comment is correct. We just need to avoid adding ourselves to our own child list to allow this function to work as intended.

@lethosor
Copy link
Member

I came up with a rudimentary script to repair the save linked above:

for i, bld in ipairs(df.building.get_vector()) do
    for j = #bld.children - 1, 0, -1 do
        if bld.children[j] == bld then
            print('removing self-child', i, j, bld)
            bld.children:erase(j)
        end
    end
    for j = #bld.parents - 1, 0, -1 do
        if bld.parents[j] == bld then
            print('removing self-parent', i, j, bld)
            bld.parents:erase(j)
        end
    end
end

Unsure if this warrants a fix/ or devel/ script - depends on how widespread the issue is. (Heck, we still have a C++ fix-job-postings command to deal with #741... so there's a precedent for it, I guess)

@lethosor lethosor added this to To do in 0.47.04-r5 via automation Jan 27, 2021
0.47.04-r5 automation moved this from To do to Done Jan 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
0.47.04-r5
  
Done
Development

Successfully merging a pull request may close this issue.

3 participants