From ac3e5d6b1fa15497a3af82d427b05cfd2d8f2fcc Mon Sep 17 00:00:00 2001 From: Alistair-Afton Date: Thu, 17 Sep 2026 19:42:29 +0200 Subject: [PATCH 1/3] dev: add a generated checklist of DF defect mitigations DFHack works around a number of defects in DF itself, and those mitigations need to be re-reviewed whenever Bay 12 releases a new DF version. Marking them with DF-MITIGATION: comments where they live keeps the checklist from drifting, and ci/list-df-mitigations.py generates the review list (the fix/* scripts are included automatically). The Watch DF Releases workflow appends the checklist to its job summary when it detects a new release. --- .github/workflows/watch-df-release.yml | 9 + ci/list-df-mitigations.py | 166 ++++++++++++++++++ docs/dev/Contributing.rst | 16 ++ docs/dev/github-workflows.rst | 4 +- docs/dev/release-process.rst | 24 +++ library/modules/Buildings.cpp | 3 +- library/modules/Gui.cpp | 4 +- library/modules/MapCache.cpp | 3 +- library/modules/Screen.cpp | 1 + library/modules/World.cpp | 4 +- .../lua/buildingplan/unlink_mechanisms.lua | 4 +- plugins/lua/sort/slab.lua | 1 + plugins/reveal.cpp | 3 +- plugins/tweak/tweaks/reaction-gloves.h | 2 +- plugins/zone.cpp | 4 +- 15 files changed, 238 insertions(+), 10 deletions(-) create mode 100644 ci/list-df-mitigations.py diff --git a/.github/workflows/watch-df-release.yml b/.github/workflows/watch-df-release.yml index 50950140d1..9e569d5f78 100644 --- a/.github/workflows/watch-df-release.yml +++ b/.github/workflows/watch-df-release.yml @@ -77,6 +77,15 @@ jobs: with: webhook-url: ${{ secrets.DISCORD_TEAM_PRIVATE_WEBHOOK_URL }} content: "<@&${{ secrets.DISCORD_TEAM_ROLE_ID }}> Steam ${{ matrix.df_steam_branch }} branch updated (build id: ${{ env.BUILDID }})" + - name: Check out dfhack develop for mitigation checklist + uses: actions/checkout@v4 + if: env.BUILDID + with: + ref: develop + submodules: true + - name: Emit DF mitigation checklist to job summary + if: env.BUILDID + run: python3 ci/list-df-mitigations.py >> "$GITHUB_STEP_SUMMARY" - name: Launch symbol generation workflow if: env.BUILDID && matrix.dfhack_ref env: diff --git a/ci/list-df-mitigations.py b/ci/list-df-mitigations.py new file mode 100644 index 0000000000..5cf0a17299 --- /dev/null +++ b/ci/list-df-mitigations.py @@ -0,0 +1,166 @@ +#!/usr/bin/env python3 +"""List DF defect mitigations that should be re-reviewed on a new DF release. + +DFHack works around a number of defects in Dwarf Fortress itself. When Bay 12 +releases a new version of DF, some of those mitigations may need to be adjusted +or removed. This script generates the review checklist so the release +coordinator does not have to track the mitigations by hand. + +Mitigations are marked where they live in the code with a comment containing +the ``DF-MITIGATION:`` marker, e.g.:: + + // DF-MITIGATION: site_id is not assigned on reclaim until the first save + +In addition, every ``fix/*`` script in the scripts repo is a mitigation by +definition, so they are listed automatically without needing a marker. Their +descriptions come from the ``:summary:`` field of their documentation. + +The checklist is printed to stdout. If the GITHUB_STEP_SUMMARY environment +variable is set (i.e. when running in a GitHub Actions job), it is also +appended to the job summary so it is visible on the workflow run page. +""" + +import os +import re + +REPO_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) +SELF = relpath_self = os.path.relpath(os.path.abspath(__file__), + REPO_ROOT).replace(os.sep, '/') + +MARKER_RE = re.compile(r'DF-MITIGATION:?\s*(.*?)\s*(?:\*+/)?\s*$') +SUMMARY_RE = re.compile(r'^\s*:summary:\s*(.*)$') + +# directories that never contain our own code +SKIP_DIRS = {'.git', 'build', 'depends', 'package'} + +SOURCE_EXTS = { + '.c', '.cc', '.cpp', '.cxx', '.h', '.hh', '.hpp', + '.lua', '.py', '.rb', '.js', '.ts', '.sh', '.ps1', +} + + +def iter_source_files(root): + for dirpath, dirnames, filenames in os.walk(root): + dirnames[:] = [d for d in dirnames + if d not in SKIP_DIRS and not d.startswith('.')] + for name in filenames: + if os.path.splitext(name)[1].lower() in SOURCE_EXTS: + yield os.path.join(dirpath, name) + + +def relpath(path): + return os.path.relpath(path, REPO_ROOT).replace(os.sep, '/') + + +def collect_markers(): + entries = [] + for path in iter_source_files(REPO_ROOT): + if relpath(path) == SELF: + continue + try: + with open(path, encoding='utf-8', errors='replace') as f: + for lineno, line in enumerate(f, 1): + match = MARKER_RE.search(line) + if match: + entries.append((relpath(path), lineno, + match.group(1) or '(no description)')) + except OSError: + continue + return sorted(entries) + + +def doc_summary(script_path): + """Look up the :summary: field in the script's documentation.""" + rel = relpath(script_path) + name = os.path.splitext(os.path.basename(rel))[0] + for doc in (os.path.join(REPO_ROOT, 'scripts', 'docs', 'fix', + name + '.rst'), + os.path.join(REPO_ROOT, 'scripts', 'docs', name + '.rst')): + try: + with open(doc, encoding='utf-8', errors='replace') as f: + for line in f: + match = SUMMARY_RE.match(line) + if match: + return match.group(1).strip() + except OSError: + continue + return None + + +def script_description(path): + """Fall back to the first substantive comment line of the script.""" + try: + with open(path, encoding='utf-8', errors='replace') as f: + for line in f: + line = line.strip() + if line.startswith('--'): + desc = line.lstrip('-').strip() + if desc and not desc.startswith('@'): + return desc + elif line: + return None + except OSError: + pass + return None + + +def collect_fix_scripts(): + """fix/* scripts (and the top-level fix* scripts) are all DF bug + mitigations.""" + entries = [] + scripts_dir = os.path.join(REPO_ROOT, 'scripts') + if not os.path.isdir(scripts_dir): + return entries + for dirpath, _dirnames, filenames in os.walk(scripts_dir): + if relpath(dirpath) not in ('scripts', 'scripts/fix'): + continue + for name in filenames: + if not name.endswith('.lua'): + continue + if relpath(dirpath) == 'scripts' and not name.startswith('fix'): + continue + path = os.path.join(dirpath, name) + desc = doc_summary(path) or script_description(path) \ + or '(no description)' + entries.append((relpath(path), desc)) + return sorted(entries) + + +def main(): + lines = [ + '## DF defect mitigations', + '', + 'A new DF release may have fixed some of the defects that DFHack works', + 'around. Review each entry and adjust or remove mitigations (and their', + '`DF-MITIGATION` markers) that are no longer needed.', + '', + '### Code sites', + '', + ] + markers = collect_markers() + if markers: + for path, lineno, desc in markers: + lines.append(f'- [ ] `{path}:{lineno}`: {desc}') + else: + lines.append('- (none found)') + + lines += ['', '### `fix/*` scripts', ''] + fixes = collect_fix_scripts() + if fixes: + for path, desc in fixes: + lines.append(f'- [ ] `{path}`: {desc}') + else: + lines.append('- (scripts repo not checked out)') + + lines.append('') + output = '\n'.join(lines) + print(output) + + summary_path = os.environ.get('GITHUB_STEP_SUMMARY') + if summary_path: + with open(summary_path, 'a', encoding='utf-8') as f: + f.write(output) + + +if __name__ == '__main__': + main() diff --git a/docs/dev/Contributing.rst b/docs/dev/Contributing.rst index e9600bbce9..646a0b6a95 100644 --- a/docs/dev/Contributing.rst +++ b/docs/dev/Contributing.rst @@ -79,6 +79,22 @@ General C++ code guidelines * Prefer range for loops to traditional for loops when iterating over a container. * Avoid macros when possible; prefer ``constexpr`` variables for constants and functions or templates for code generation. +Marking mitigations for DF bugs +------------------------------- +DFHack works around a number of defects in Dwarf Fortress itself. When Bay 12 +fixes one of these defects in a new DF release, the corresponding mitigation +may need to be adjusted or removed. To keep track of these mitigations, mark +them with a ``DF-MITIGATION:`` comment where they live in the code, e.g.:: + + // DF-MITIGATION: site_id is not assigned on reclaim until the first save + +Briefly describe the defect being worked around and include a reference to the +DF bug or DFHack issue/PR when one exists. The markers are collected by +:source:`ci/list-df-mitigations.py` into a checklist that is reviewed on every +new DF release; see `release-process-df-mitigations`_. The ``fix/*`` scripts are +all DF bug mitigations by definition and are listed automatically, so they do +not need markers. + .. _contributing-pr-guidelines: Pull request guidelines diff --git a/docs/dev/github-workflows.rst b/docs/dev/github-workflows.rst index ea71e1a2b6..7b7fa93248 100644 --- a/docs/dev/github-workflows.rst +++ b/docs/dev/github-workflows.rst @@ -107,7 +107,9 @@ Watch DF Releases This workflow runs every 8 minutes and checks the Steam metadata, the Itch website, and the Bay 12 website for evidence of new releases. If a new release is found, it generates an announcement in a private channel on the DFHack -Discord server. +Discord server and appends the `DF defect mitigation checklist +`_ to the job summary for the release +coordinator to review. Inside the ``watch-df-releases.yml`` workflow, there are separate jobs for watching Steam branches and watching the websites. For the Steam watcher, it diff --git a/docs/dev/release-process.rst b/docs/dev/release-process.rst index bef09be412..df33bd36a0 100644 --- a/docs/dev/release-process.rst +++ b/docs/dev/release-process.rst @@ -6,6 +6,30 @@ This page details the process we follow for beta and stable releases. For documentation on the related GitHub workflows, see `workflows-release-automation`. +.. _release-process-df-mitigations: + +New DF releases +--------------- + +When Bay 12 releases a new version of DF, the mitigations we maintain for +defects in DF itself may need to be adjusted or removed. These mitigations are +marked with ``DF-MITIGATION:`` comments in the code (see +`contributing`_ for the convention) and the ``fix/*`` scripts in +the scripts repo are all mitigations by definition. + +To review them: + +1. Run ``ci/list-df-mitigations.py`` in a DFHack checkout (with submodules) to + generate the checklist. The Watch DF Releases workflow also appends the + checklist to its job summary when it detects a new release. + +2. For each entry, determine whether the new DF version still exhibits the + defect. Some entries can be checked by code inspection; others need a save + that reproduces the defect. + +3. Remove or adjust mitigations that are no longer needed, and remove their + ``DF-MITIGATION`` markers. Keep entries that still apply. + Beta release ------------ diff --git a/library/modules/Buildings.cpp b/library/modules/Buildings.cpp index ddedbbc1b6..9852f57b53 100644 --- a/library/modules/Buildings.cpp +++ b/library/modules/Buildings.cpp @@ -128,7 +128,8 @@ static df::building_extents_type *getExtentTile(const df::building::T_room &room } /* - * A monitor to work around this bug, in its application to buildings: + * DF-MITIGATION: monitor works around DF bug 1416 in its application to + * buildings: * * http://www.bay12games.com/dwarves/mantisbt/view.php?id=1416 */ diff --git a/library/modules/Gui.cpp b/library/modules/Gui.cpp index 96a7ab5472..e9d272df37 100644 --- a/library/modules/Gui.cpp +++ b/library/modules/Gui.cpp @@ -2041,7 +2041,9 @@ void Gui::showPopupAnnouncement(std::string message, int color, bool bright) df::popup_message *popup = new df::popup_message(); popup->text = message; popup->color = color; // Doesn't do anything anymore? Popups are always [C:7:0:0] gray text - popup->bright = bright; // See: https://dwarffortressbugtracker.com/view.php?id=12672 + // DF-MITIGATION: the bright flag has no effect due to a DF bug; keep it set + // in case DF starts honoring it: https://dwarffortressbugtracker.com/view.php?id=12672 + popup->bright = bright; auto &popups = world->status.popups; popups.push_back(popup); diff --git a/library/modules/MapCache.cpp b/library/modules/MapCache.cpp index a14d81288e..6507bc7f8f 100644 --- a/library/modules/MapCache.cpp +++ b/library/modules/MapCache.cpp @@ -531,7 +531,8 @@ void MapExtras::Block::ParseTiles(TileInfo *tiles) tt = con->original_tile; - // Ice under construction is buggy: + // DF-MITIGATION: DF bug 6330 makes ice under constructions + // behave incorrectly: // http://www.bay12games.com/dwarves/mantisbt/view.php?id=6330 // Therefore we just pretend it wasn't there (if it isn't too late), // and overwrite it if/when we write the base layer. diff --git a/library/modules/Screen.cpp b/library/modules/Screen.cpp index 133caa82b2..ec74e5bc3c 100644 --- a/library/modules/Screen.cpp +++ b/library/modules/Screen.cpp @@ -845,6 +845,7 @@ void dfhack_viewscreen::logic() // Various stuff works poorly unless always repainting Screen::invalidate(); + // DF-MITIGATION: DF can get stuck when a dismissed screen stays buried // if the DF screen immediately beneath the DFHack viewscreens is waiting to // be dismissed, raise it to the top so DF never gets stuck auto *p = parent; diff --git a/library/modules/World.cpp b/library/modules/World.cpp index 020e76127d..1e4c921216 100644 --- a/library/modules/World.cpp +++ b/library/modules/World.cpp @@ -218,8 +218,8 @@ int32_t World::GetCurrentSiteId() { if (!plotinfo) return -1; if (isFortressMode()) { - // on a reclaimed fortress, site_id isn't assigned until the first - // save; fortress_site is set at embark, so use it as a fallback + // DF-MITIGATION: reclaimed forts lack site_id until first save (#5716) + // fortress_site is set at embark, so use it as a fallback if (plotinfo->site_id >= 0) return plotinfo->site_id; if (auto site = plotinfo->main.fortress_site) diff --git a/plugins/lua/buildingplan/unlink_mechanisms.lua b/plugins/lua/buildingplan/unlink_mechanisms.lua index d6a676fdd1..8ba281ae73 100644 --- a/plugins/lua/buildingplan/unlink_mechanisms.lua +++ b/plugins/lua/buildingplan/unlink_mechanisms.lua @@ -224,7 +224,9 @@ function MechLinkOverlay:init() on_activate = self:callback("ask_unlink_all"), enabled = function() return next(self.links) end, }, - widgets.Scrollbar --Work around for https://dwarffortressbugtracker.com/view.php?id=12721 + -- DF-MITIGATION: extra scrollbar works around DF bug 12721 + -- https://dwarffortressbugtracker.com/view.php?id=12721 + widgets.Scrollbar { view_id = "scroll", frame = {t=0, r=0, h=24}, diff --git a/plugins/lua/sort/slab.lua b/plugins/lua/sort/slab.lua index 95274b36d6..57ee225a66 100644 --- a/plugins/lua/sort/slab.lua +++ b/plugins/lua/sort/slab.lua @@ -68,6 +68,7 @@ function SlabOverlay:onInput(keys) end function SlabOverlay:get_key() + -- DF-MITIGATION: building.category not reset when no units memorializable -- DF fails to set building.category back to NONE if there are no units that -- can be memorialized, so we have to manually check for a populated button vector if #building.button > 0 and diff --git a/plugins/reveal.cpp b/plugins/reveal.cpp index e5bd2479f5..516e2fc0e5 100644 --- a/plugins/reveal.cpp +++ b/plugins/reveal.cpp @@ -371,7 +371,8 @@ static void unhideFlood_internal(const df::coord &xy) { if(!des || !des->bits.hidden) continue; - // we don't want constructions or ice to restrict vision (to avoid bug #1871) + // DF-MITIGATION: we don't want constructions or ice to restrict vision + // (to avoid bug #1871) df::tiletype *tt = Maps::getTileType(current); if (!tt) continue; diff --git a/plugins/tweak/tweaks/reaction-gloves.h b/plugins/tweak/tweaks/reaction-gloves.h index e880c0bfef..f760e14b83 100644 --- a/plugins/tweak/tweaks/reaction-gloves.h +++ b/plugins/tweak/tweaks/reaction-gloves.h @@ -1,4 +1,4 @@ -// Workaround for DF bug #6273 - adjust all custom reactions to produce GLOVES items in sets with correct handedness +// DF-MITIGATION: workaround for DF bug #6273 - adjust all custom reactions to produce GLOVES items in sets with correct handedness // It also analyzes the body plan of the unit performing the reaction, so Antmen will get 4 gloves instead of 2 // If a reaction tries to produce either 1 glove or 2 gloves, it will produce a single set diff --git a/plugins/zone.cpp b/plugins/zone.cpp index 1e69b7e193..3f15e8714b 100644 --- a/plugins/zone.cpp +++ b/plugins/zone.cpp @@ -539,7 +539,9 @@ static command_result assignUnitToCage(color_ostream& out, df::unit* unit, df::b return CR_WRONG_USAGE; } - // don't assign owned pets to a cage. the owner will release them, resulting into infinite hauling (df bug) + // DF-MITIGATION: caging owned pets causes infinite hauling (df bug) + // don't assign owned pets to a cage. the owner will release them, resulting + // into infinite hauling if(unit->relationship_ids[df::unit_relationship_type::PetOwner] != -1) return CR_OK; From 97c4673ac4fe0b45b75c23d7d0aa254064c15c08 Mon Sep 17 00:00:00 2001 From: Alistair-Afton Date: Thu, 17 Sep 2026 20:05:00 +0200 Subject: [PATCH 2/3] fix doc references and mark script executable --- ci/list-df-mitigations.py | 0 docs/dev/Contributing.rst | 2 +- docs/dev/release-process.rst | 2 +- 3 files changed, 2 insertions(+), 2 deletions(-) mode change 100644 => 100755 ci/list-df-mitigations.py diff --git a/ci/list-df-mitigations.py b/ci/list-df-mitigations.py old mode 100644 new mode 100755 diff --git a/docs/dev/Contributing.rst b/docs/dev/Contributing.rst index 646a0b6a95..e7247061ae 100644 --- a/docs/dev/Contributing.rst +++ b/docs/dev/Contributing.rst @@ -91,7 +91,7 @@ them with a ``DF-MITIGATION:`` comment where they live in the code, e.g.:: Briefly describe the defect being worked around and include a reference to the DF bug or DFHack issue/PR when one exists. The markers are collected by :source:`ci/list-df-mitigations.py` into a checklist that is reviewed on every -new DF release; see `release-process-df-mitigations`_. The ``fix/*`` scripts are +new DF release; see `release-process-df-mitigations`. The ``fix/*`` scripts are all DF bug mitigations by definition and are listed automatically, so they do not need markers. diff --git a/docs/dev/release-process.rst b/docs/dev/release-process.rst index df33bd36a0..f5b6bc06fe 100644 --- a/docs/dev/release-process.rst +++ b/docs/dev/release-process.rst @@ -14,7 +14,7 @@ New DF releases When Bay 12 releases a new version of DF, the mitigations we maintain for defects in DF itself may need to be adjusted or removed. These mitigations are marked with ``DF-MITIGATION:`` comments in the code (see -`contributing`_ for the convention) and the ``fix/*`` scripts in +`contributing` for the convention) and the ``fix/*`` scripts in the scripts repo are all mitigations by definition. To review them: From 6b401ab35c2cb4a5b035641b9422e1c253f0ad02 Mon Sep 17 00:00:00 2001 From: Alistair-Afton Date: Thu, 17 Sep 2026 20:17:40 +0200 Subject: [PATCH 3/3] clarify popup color/bright marker scope --- docs/dev/Contributing.rst | 6 ++++-- library/modules/Gui.cpp | 8 +++++--- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/docs/dev/Contributing.rst b/docs/dev/Contributing.rst index e7247061ae..28eb592599 100644 --- a/docs/dev/Contributing.rst +++ b/docs/dev/Contributing.rst @@ -88,8 +88,10 @@ them with a ``DF-MITIGATION:`` comment where they live in the code, e.g.:: // DF-MITIGATION: site_id is not assigned on reclaim until the first save -Briefly describe the defect being worked around and include a reference to the -DF bug or DFHack issue/PR when one exists. The markers are collected by +The marker is also appropriate for code whose behavior depends on a DF defect +without working around it, e.g. comments explaining why a field we set has no +effect. Briefly describe the defect and include a reference to the DF bug or +DFHack issue/PR when one exists. The markers are collected by :source:`ci/list-df-mitigations.py` into a checklist that is reviewed on every new DF release; see `release-process-df-mitigations`. The ``fix/*`` scripts are all DF bug mitigations by definition and are listed automatically, so they do diff --git a/library/modules/Gui.cpp b/library/modules/Gui.cpp index e9d272df37..f1cbc4e927 100644 --- a/library/modules/Gui.cpp +++ b/library/modules/Gui.cpp @@ -2040,9 +2040,11 @@ void Gui::showPopupAnnouncement(std::string message, int color, bool bright) { df::popup_message *popup = new df::popup_message(); popup->text = message; - popup->color = color; // Doesn't do anything anymore? Popups are always [C:7:0:0] gray text - // DF-MITIGATION: the bright flag has no effect due to a DF bug; keep it set - // in case DF starts honoring it: https://dwarffortressbugtracker.com/view.php?id=12672 + // DF-MITIGATION: DF ignores popup color/bright fields (bug 12672) + // Popups always render as [C:7:0:0] gray text; keep setting the fields so + // they take effect again if DF is fixed: + // https://dwarffortressbugtracker.com/view.php?id=12672 + popup->color = color; popup->bright = bright; auto &popups = world->status.popups;