buildings: make windows masterable like other demandable furnishings

[Alistair-Afton]

Summary

• mitigate a DF bug where building_windowst::masterablebuilding returns false even though windows are the only noble-demandable room furnishing that is not masterable
• interpose canMakeRoom on building_window_gemst and building_window_glassst (the abstract base's vtable cannot be hooked), so windows can be made part of a room on all code paths
• this fixes both link directions uniformly: a window placed inside an existing room and a room designated over an existing window, through both the vanilla UI and the Buildings module
• add fortress tests covering both link directions for both window types

Fixes #5693.

Testing

• test -d C:/Users/Alistair Afton/source/repos/dfhack/test -t 'window' — 4/4 pass
• verified live: a planned gem window inside an existing zone reports canMakeRoom() == true and is linked in both directions (bld.relations + zone.contained_buildings)
• Release build and install

[ab9rf]

I'm holding this one until I'm convinced that this is the correct approach. This basically bypasses the DF masterablebuilding virtual method for these two building subclasses, and I want to understand that this is consistent with what the game actually does, or if we should be using a different methodology to determine this property for a given building.

I'm concerned that this is papering over a defect elsewhere, either in DF or DFHack, and I want to understand what is going on before committing to this fix

[Alistair-Afton]

Did some in-game verification of what vanilla actually does, on top of what chdoc already confirmed in #5693:

What vanilla does (confirmed empirically):

• UI-placed gem window inside a pre-existing zone gets linked both directions (bld.relations + zone.contained_buildings) — chdoc's reproduction in the issue.
• Order-dependent: designating a zone around an existing window does not pick it up (quietust's 52.05 analysis, confirmed by chdoc on guildhalls).
• The link is established at placement-confirm time, not at completion. I placed a WindowGem via constructWithFilters inside an existing zone on the unpatched build, posted the job, and let a dwarf haul 3 gems and complete it — after completion the building still had no relations entry and the zone's contained_buildings was unchanged. So there is no deferred/completion-time relinking pass.

Why canMakeRoom() is the wrong gate: the vmethod is masterablebuilding — "can this building be the anchor you designate a room from". Fresh instances return true for beds/chairs/tables/doors/statues/cabinets/etc. and false for windows, workshops, and all machine components. But contained_buildings (original name subord) is documented as "includes eg workshops and beds" — vanilla's membership set is strictly broader than the masterable set, so gating membership on the anchor predicate was simply wrong. This fort's live data is consistent: every unlinked building overlapping a zone either has masterablebuilding=false or sits on a tile excluded by the zone's extent mask.

So bypassing masterablebuilding for these types does match observed game behavior — the gate itself was the defect, not the bypass. What I can't tell you without more RE is vanilla's exact membership criterion; windows are confirmed, workshops implied by the subord comment. If it turns out vanilla links essentially every actual building inside a zone's extent, the right long-term fix may be inverting this to an exclusion list rather than a two-type bypass — happy to widen it if you'd prefer, but I kept it narrow since windows are the only type with verified vanilla behavior.

[ab9rf]

So after looking into this a bit more, the issue appears to me to be that there's a vanilla bug here, in that whether certain building types can be added to a room varies depending on whether the room or the building was placed first. On one code path, masterablebuilding is used to gate this decision, on the other it is not. My concern here is that this change has the effect of enforcing Hyrum's law: we are here preserving an unintended behavior solely because consumers of that behavior have come to treat it as an expected behavior. I'm not inclined to intentionally program DFHack to be "bug for bug" compatible with DF. My best guess is that Toady does not intend windows to be "part of a room", and the behavior that allows them to become such is a defect, but my confidence in this guess is relatively low. Given all this, I'm inclined not to make windows able to be made part of the room, at least not until I have some sort of statement from Bay12 on this issue.

@quietust did recently point out on Discord that windows are the only building that a noble can demand added to their room which is not masterable. This suggests to me that the implementation of the masterablebuilding vmethod on building_windowst is the core problem here. Basically, as Quietust recently said on Discord, "[T]he game itself seems to be confused."

@Putnam3145, any thoughts here?

[ab9rf]

Let me add that if our conclusion is that windows should be masterable, we should resolve this by using masterablebuilding in the building planner (as we already do), and by interposing masterablebuilding for the two concrete subclasses of building_windowst (we can't interpose abstract base classes because their vtables are often deleted). That is, we should simply directly mitigate the DF bug. This is, of course, provided that we actually do conclude that windows not being masterable in the vanilla game is a bug in the game.

[Alistair-Afton]

Reworked per your suggestion — the fix is now a vmethod interpose instead of a bypass in the zoning gate.

Changes in d404edf:

• window_gem_masterable_hook and window_glass_masterable_hook interpose canMakeRoom on the two concrete building_windowst subclasses to return true, applied on SC_MAP_LOADED and removed on SC_MAP_UNLOADED in buildings_onStateChange
• is_suitable_building_for_zoning is back to a plain canMakeRoom() call, so the Buildings module keeps gating on the vmethod as before
• since the game's own assert_mastery consults the same vmethod, this also fixes the vanilla order-dependency (zone painted over an existing window now picks it up), not just the planner path
• the test now covers both directions for both window types: window constructed in an existing zone, and an extent-shaped civzone placed over an existing window

Verified live on 53.16: a planned WindowGem inside an existing zone reports canMakeRoom() == true and links in both directions; all 4 tests pass. stderr.log shows no interpose apply failures at map load.

[ab9rf]

Because our conclusion that this is a DF bug may be at least somewhat contentious, this should be a toggleable tweak. It can default to on via the control panel, but player should have the option to turn it off if they want the vanilla behavior.

[quietust]

the vmethod is masterablebuilding — "can this building be the anchor you designate a room from".

For what it's worth, I believe that description is incorrect - we originally named the vmethod "canMakeRoom", but Toady's name "masterablebuilding" makes it more clear that it's actually "can this building become subordinate to another building" (i.e. can it become part of a room).

Background: in 0.47 and earlier, rooms were designated from specific buildings (e.g. bedrooms from a bed, dining rooms from a table, offices from a chair), but 50.01 got rid of that and made civzones the "parent" building in all cases.