DFHack / dfhack

Memory hacking library for Dwarf Fortress and a set of tools that use it
Other
1.86k stars 468 forks source link

Buildings:: constructAbstract seems to exclude trees from gathering zones when those are created #1412

Closed PatrikLundell closed 3 years ago

PatrikLundell commented 5 years ago

I'm not completely sure what's going on, or whether I am the one doing something incorrectly.

I've tried to use a Lua script to create a gathering zone around a tree, centered on the tree. However, I either end up with a failure to create the zone, or the tree itself removed from the zone, which ought to cause fruit growing on the trunk to not be gathered.

The script essentially does this:

local zone = dfhack.buildings.allocInstance (plant.pos, df.building_type.Civzone, df.civzone_type.ActivityZone, -1)
dfhack.println (dfhack.buildings.setSize (zone, 3, 3, 0))
dfhack.println ("Specifying " .. zone.name, dfhack.buildings.constructAbstract (zone))

(there are some other fields set as well, but those are probably not of interest here). The first "println" prints "true 3 3 9 8", where the '8' at the end is likely to be an indication of the issue, as it indicates the tree has been removed from the zone by setSize in that stage. However, the building resulting at the end looks the same as that of a manually created zone, except for zone.room.extents [4] (which probably is "[1][1]" if addressed properly), which has a value of 0 for the script created zone and 1 for the manually created one. Setting this field to 1 on the script created one causes "constructAbstract" to return false, however, likely because it contains the check

    if (!checkBuildingTiles(bld, false))
        return false;

but setting it after the abstract building has been constructed seems to work.

Thus, I think I can work around the issue for my purposes, but I suspect there is still an issue here.

Edit: After further thinking, I believe the issue is that zones differ from other kinds of buildings in that the do not exclude blocked tiles, so setSize should allow for blocked tiles in its call to to set up the tiles if the building is a Civzone, and constructAbstract should only perform the checkBuildingTiles check for non zone buildings.

myk002 commented 4 years ago

I think I fixed this in #1624 when I ran into the same issue in the new quickfort zone mode

PatrikLundell commented 4 years ago

As far as I can see myk002's change doesn't fix this issue as that change doesn't seem to change any calls to the modified operation, and the operation changed doesn't seem to be exactly the same one (I haven't actually looked at the code). However, the logic applied ought to apply to this situation as well.

myk002 commented 4 years ago

You may have missed the change to checkBuildingTiles, which sets the new boolean. Does your code go through that path?

PatrikLundell commented 4 years ago

Sorry, yes, you're completely correct in that I did miss that change (despite looking for it...).

This means it ought to take care of this case as well.

lethosor commented 3 years ago

Is this fixed in 0.47.04-r3, then?

myk002 commented 3 years ago

yes, I believe it is