Skip to content

Fix placeBuildingAt future-worker lookup for flags - #153

Merged
genixpro merged 1 commit into
masterfrom
junior/fix-placebuilding-worker-lookup
Sep 6, 2026
Merged

Fix placeBuildingAt future-worker lookup for flags#153
genixpro merged 1 commit into
masterfrom
junior/fix-placebuilding-worker-lookup

Conversation

@Giszmo

@Giszmo Giszmo commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Bugfix ported from PR #129 (feat/ai-trainer-support), split out to shrink #129's diff against master per Leo's request.

placeBuildingAt computed the finished-building default-assign row as getDefaultAssignedUnits(typeNum+1). The +1 relies on the buildingsTypes table being laid out as consecutive [site, finished] pairs, which holds by accident for normal buildings but breaks for flags: flags have no site variant, so the placeable typeNum is already the finished entry and +1 walks into the next building's row (e.g. an exploration flag reads the war flag's default assign). Looks the finished type up by name instead, matching the other call sites in this file.

Original commit: 5993b37 by kylelutze — adapted here to master's getTypeNum(name, level, isBuildingSite) API since the branch's getFinishedTypeNum(name) convenience helper doesn't exist yet on master (patch applied but referenced a symbol that doesn't compile on master; verified with a full scons build before and after the fix).

placeBuildingAt computed the finished-building default-assign row as
getDefaultAssignedUnits(typeNum+1). The +1 relies on the buildingsTypes
table being laid out as consecutive [site, finished] pairs, which holds
by accident for normal buildings but breaks for flags: flags have no site
variant, so the placeable typeNum is already the finished entry and +1
walks into the next building's row (e.g. an exploration flag reads the
war flag's default assign). Look the finished type up by name instead,
matching the other call sites in this file. No-op for normal buildings;
corrects the flag case.

Ported from PR #129 (feat/ai-trainer-support), original commit
5993b37 by kylelutze, adapted to
master's BuildingsTypes API (getTypeNum(name, level, isBuildingSite)
instead of the branch's getFinishedTypeNum(name) helper, which doesn't
exist yet on master).

@genixpro genixpro left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The finished-type lookup correctly handles flags without relying on adjacent site/finished entries. A focused C++ harness executing the production lookup and assignment expressions against data/buildings.txt reproduced incorrect defaults for all three flags on base and passed for all 13 level-zero placeable types on this head, on macOS arm64 and Linux x86_64.

Validation: a clean integration checkout combining #153#158 built successfully with scons release=1 on macOS arm64 and Ubuntu 26.04 x86_64; both binaries passed --version startup checks, and the existing CppUnit suite passed all 12 tests on Linux. This PR also has passing Ubuntu 22.04/24.04 and Windows CI. Validation was source review and automated checks; no interactive gameplay test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants