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.

Sign up for GitHub

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

Misc improvement: suspendmanager.lua has new suspension reason for unsupported constructions. #839

Merged
merged 11 commits into from
Sep 26, 2023
Merged

Misc improvement: suspendmanager.lua has new suspension reason for unsupported constructions. #839

Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
f061803
suspendmanager now checks for unsupported tiles
master-spike Sep 25, 2023
903f134
Merge branch 'master' into suspendmanager_reason_unsupported
master-spike Sep 25, 2023
f761683
add suspendmanager unsupported condition improvement to changelog
master-spike Sep 25, 2023
e496057
cleaned up the control flow of function constructionIsUnsupported(job…
master-spike Sep 25, 2023
72f6b74
fixed typo df.construction_type.FLOOR -> ...Floor
master-spike Sep 25, 2023
db0f4a3
SuspendManager:refresh() moved constructionIsUnsupported() check afte…
master-spike Sep 25, 2023
2f4a6e5
suspendmanager: added comment and updated doc
master-spike Sep 25, 2023
01481c6
fixed directions for supporting connections
master-spike Sep 25, 2023
7c92421
constructionIsUnsupported() - early return if unreachable to reduce spam
master-spike Sep 25, 2023
4ac36ac
suspendmanager - clarified 'unsupported' reason description
master-spike Sep 25, 2023
6d3bd57
reverted walkable neighbour early return
master-spike Sep 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
209 changes: 208 additions & 1 deletion suspendmanager.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ REASON = {
ERASE_DESIGNATION = 4,
--- Blocks a dead end (either a corridor or on top of a wall)
DEADEND = 5,
UNSUPPORTED = 6,
master-spike marked this conversation as resolved.
Show resolved Hide resolved
}

REASON_TEXT = {
Expand All @@ -44,6 +45,7 @@ REASON_TEXT = {
[REASON.RISK_BLOCKING] = 'blocking',
[REASON.ERASE_DESIGNATION] = 'designation',
[REASON.DEADEND] = 'dead end',
[REASON.UNSUPPORTED] = 'unsupported',
}

--- Description of suspension
Expand All @@ -52,7 +54,8 @@ REASON_TEXT = {
REASON_DESCRIPTION = {
[REASON.RISK_BLOCKING] = 'May block another build job',
[REASON.ERASE_DESIGNATION] = 'Waiting for carve/smooth/engrave',
[REASON.DEADEND] = 'Blocks another build job'
[REASON.DEADEND] = 'Blocks another build job',
[REASON.UNSUPPORTED] = 'Construction is unsupported'
}

--- Suspension reasons from an external source
Expand Down Expand Up @@ -161,6 +164,77 @@ local CONSTRUCTION_IMPASSABLE = utils.invert{
df.construction_type.Fortification,
}

local CONSTRUCTION_WALL_SUPPORT = utils.invert{
df.construction_type.Wall,
df.construction_type.Fortification,
df.construction_type.UpStair,
df.construction_type.UpDownStair,
}

local CONSTRUCTION_FLOOR_SUPPORT = utils.invert{
df.construction_type.FLOOR,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
df.construction_type.FLOOR,
df.construction_type.Floor,

Floor does not seem to be upper-case (tested locally, and in the structures: https://github.com/DFHack/df-structures/blob/e6d83ccaee5b5a3c663b56046ae55a7389742da8/df.buildings.xml#L1007 )

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, that must be why it doesn't work for floors!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've made the change now - should work for that case

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does :)

df.construction_type.DownStair,
df.construction_type.Ramp,
df.construction_type.TrackN,
df.construction_type.TrackS,
df.construction_type.TrackE,
df.construction_type.TrackW,
df.construction_type.TrackNS,
df.construction_type.TrackNE,
df.construction_type.TrackSE,
df.construction_type.TrackSW,
df.construction_type.TrackEW,
df.construction_type.TrackNSE,
df.construction_type.TrackNSW,
df.construction_type.TrackNEW,
df.construction_type.TrackSEW,
df.construction_type.TrackNSEW,
df.construction_type.TrackRampN,
df.construction_type.TrackRampS,
df.construction_type.TrackRampE,
df.construction_type.TrackRampW,
df.construction_type.TrackRampNS,
df.construction_type.TrackRampNE,
df.construction_type.TrackRampNW,
df.construction_type.TrackRampSE,
df.construction_type.TrackRampSW,
df.construction_type.TrackRampEW,
df.construction_type.TrackRampNSE,
df.construction_type.TrackRampNSW,
df.construction_type.TrackRampNEW,
df.construction_type.TrackRampSEW,
df.construction_type.TrackRampNSEW,
}

-- all the tiletype shapes which provide support as if a wall
-- note that these shapes act as if there is a floor above them,
-- (including an up stair with no down stair above) which then connects
-- orthogonally at that level.
-- see: https://dwarffortresswiki.org/index.php/DF2014:Cave-in
local TILETYPE_SHAPE_WALL_SUPPORT = utils.invert{
df.tiletype_shape.WALL,
df.tiletype_shape.FORTIFICATION,
df.tiletype_shape.STAIR_UP,
df.tiletype_shape.STAIR_UPDOWN,
}

-- all the tiletype shapes which provide support as if it were a floor.
-- Tested as of v50.10 - YES, twigs do provide orthogonal support like a floor.
local TILETYPE_SHAPE_FLOOR_SUPPORT = utils.invert{
df.tiletype_shape.FLOOR,
df.tiletype_shape.STAIR_DOWN,
df.tiletype_shape.RAMP,
df.tiletype_shape.BOULDER,
df.tiletype_shape.PEBBLES,
df.tiletype_shape.SAPLING,
df.tiletype_shape.BROOK_BED,
df.tiletype_shape.BROOK_TOP,
df.tiletype_shape.SHRUB,
df.tiletype_shape.TWIG,
df.tiletype_shape.BRANCH,
df.tiletype_shape.TRUNK_BRANCH,
}

local BUILDING_IMPASSABLE = utils.invert{
df.building_type.Floodgate,
df.building_type.Statue,
Expand Down Expand Up @@ -266,6 +340,134 @@ local function neighbours(pos)
}
end

--- list neighbour coordinates of pos which if is a Wall, will support a Wall at pos
---@param pos coord
---@return table<number, coord>
local function neighboursWallSupportsWall(pos)
return {
{x=pos.x-1, y=pos.y, z=pos.z},
{x=pos.x+1, y=pos.y, z=pos.z},
{x=pos.x, y=pos.y-1, z=pos.z},
{x=pos.x, y=pos.y+1, z=pos.z},
{x=pos.x-1, y=pos.y, z=pos.z-1},
{x=pos.x+1, y=pos.y, z=pos.z-1},
{x=pos.x, y=pos.y-1, z=pos.z-1},
{x=pos.x, y=pos.y+1, z=pos.z-1},
{x=pos.x-1, y=pos.y, z=pos.z+1},
{x=pos.x+1, y=pos.y, z=pos.z+1},
{x=pos.x, y=pos.y-1, z=pos.z+1},
{x=pos.x, y=pos.y+1, z=pos.z+1},
{x=pos.x, y=pos.y, z=pos.z-1},
{x=pos.x, y=pos.y, z=pos.z+1},
}
end

--- list neighbour coordinates of pos which if is a Floor, will support a Wall at pos
---@param pos coord
---@return table<number, coord>
local function neighboursFloorSupportsWall(pos)
return {
{x=pos.x-1, y=pos.y, z=pos.z},
{x=pos.x+1, y=pos.y, z=pos.z},
{x=pos.x, y=pos.y-1, z=pos.z},
{x=pos.x, y=pos.y+1, z=pos.z},
{x=pos.x, y=pos.y, z=pos.z+1},
{x=pos.x-1, y=pos.y, z=pos.z+1},
{x=pos.x+1, y=pos.y, z=pos.z+1},
{x=pos.x, y=pos.y-1, z=pos.z+1},
{x=pos.x, y=pos.y+1, z=pos.z+1},
}
end

--- list neighbour coordinates of pos which if is a Wall, will support a Floor at pos
---@param pos coord
---@return table<number, coord>
local function neighboursWallSupportsFloor(pos)
return {
{x=pos.x-1, y=pos.y, z=pos.z},
{x=pos.x+1, y=pos.y, z=pos.z},
{x=pos.x, y=pos.y-1, z=pos.z},
{x=pos.x, y=pos.y+1, z=pos.z},
}
end

--- list neighbour coordinates of pos which if is a Floor, will support a Floor at pos
---@param pos coord
---@return table<number, coord>
local function neighboursFloorSupportsFloor(pos)
return {
{x=pos.x-1, y=pos.y, z=pos.z},
{x=pos.x+1, y=pos.y, z=pos.z},
{x=pos.x, y=pos.y-1, z=pos.z},
{x=pos.x, y=pos.y+1, z=pos.z},
{x=pos.x, y=pos.y, z=pos.z+1},
{x=pos.x-1, y=pos.y, z=pos.z+1},
{x=pos.x+1, y=pos.y, z=pos.z+1},
{x=pos.x, y=pos.y-1, z=pos.z+1},
{x=pos.x, y=pos.y+1, z=pos.z+1},
}
end

local function tileHasSupportBuilding(pos)
local bld = dfhack.buildings.findAtTile(pos)
if bld then
return bld:getType() == df.building_type.Support and bld.flags.exists
end
return false
end

---
local function constructionIsUnsupported(job)
if job.job_type ~= df.job_type.ConstructBuilding then return false end

local building = dfhack.job.getHolder(job)
if not building or building:getType() ~= df.building_type.Construction then return false end

local pos = {x=building.centerx, y=building.centery,z=building.z}

master-spike marked this conversation as resolved.
Show resolved Hide resolved
-- find out what type of construction
local constr_type = building:getSubtype()
if CONSTRUCTION_FLOOR_SUPPORT[constr_type] then
for _,n in pairs(neighboursWallSupportsFloor(pos)) do
local tt = dfhack.maps.getTileType(n)
if tt then
local attrs = df.tiletype.attrs[tt]
if TILETYPE_SHAPE_WALL_SUPPORT[attrs.shape] then return false end
end
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might consider breaking these bits out into a function.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I agree that wasn't very clean of me. I've broken those bits out now and cleaned up the control flow of the function

end
for _,n in pairs(neighboursFloorSupportsFloor(pos)) do
local tt = dfhack.maps.getTileType(n)
if tt then
local attrs = df.tiletype.attrs[tt]
if TILETYPE_SHAPE_FLOOR_SUPPORT[attrs.shape] then return false end
end
end
-- check for a support building below the tile
if tileHasSupportBuilding({x=pos.x, y=pos.y, z=pos.z-1}) then return false end
return true
elseif CONSTRUCTION_WALL_SUPPORT[constr_type] then
for _,n in pairs(neighboursWallSupportsWall(pos)) do
local tt = dfhack.maps.getTileType(n)
if tt then
local attrs = df.tiletype.attrs[tt]
if TILETYPE_SHAPE_WALL_SUPPORT[attrs.shape] then return false end
end
end
for _,n in pairs(neighboursFloorSupportsWall(pos)) do
local tt = dfhack.maps.getTileType(n)
if tt then
local attrs = df.tiletype.attrs[tt]
if TILETYPE_SHAPE_FLOOR_SUPPORT[attrs.shape] then return false end
end
end
-- check for a support building below and above the tile
if tileHasSupportBuilding({x=pos.x, y=pos.y, z=pos.z-1}) then return false end
if tileHasSupportBuilding({x=pos.x, y=pos.y, z=pos.z+1}) then return false end
return true
end
return false
end

--- Get the amount of risk a tile is to be blocked
--- -1: There is a nearby walkable area with no plan to build a wall
--- >=0: Surrounded by either unwalkable tiles, or tiles that will be constructed
Expand Down Expand Up @@ -433,6 +635,11 @@ function SuspendManager:refresh()
end
end

-- Check for construction jobs which may be unsupported
Copy link
Contributor

@plule plule Sep 25, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This block should be moved under below Internal reasons to suspend a job, after the self.preventBlocking check.

The terminology is not very good, but everything before this check is what is already suspended by dwarf fortress, and suspendmanager (/unsuspend) will never unsuspend it, and everything after is what suspendmanager is actively suspending.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I misunderstood what preventBlocking meant in this context. I think maybe that might warrant a rename due to the expanded scope of this script? perhaps something like preventRiskyJobs or something?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's actually even wider than that, the suspended jobs on smoothing designation also fall in that category, they are not risky, they would just erase a smoothing job. Maybe falling back on the broader "smart" term would be best.

This issue is spread all across the suspension script, even in the UI and the suspend/unsuspend command line options, I suggest not to do that in that pr, maybe a separate issue?

if constructionIsUnsupported(job) then
self.suspensions[job.id]=REASON.UNSUPPORTED
end

if not self.preventBlocking then goto continue end

-- Internal reasons to suspend a job
Expand Down