Repository navigation
Reimpl/aiv state - #233
Draft
gynt wants to merge 89 commits into
Draft
Reimpl/aiv state#233gynt wants to merge 89 commits into
gynt wants to merge 89 commits into
Conversation
gynt
force-pushed
the
reimpl/AIVState
branch
2 times, most recently
from
September 26, 2026 23:44
d5e0ae2 to
4edefa0
Compare
gynt
force-pushed
the
reimpl/AIVState
branch
from
September 26, 2026 23:52
4edefa0 to
aa42e07
Compare
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… aivID branch Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…eax,eax Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ers differ Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…rs differ Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… return Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ches Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…d-calls Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…differ Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ing with /GL The original keeps this in ecx across the call, which needs /GL on caller and callee and a direct call (resolver enabled). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
# Conflicts: # cmake/compiler-flags-gl.txt # src/OpenSHC/AI/AIVState/aiPlaceAIVBuilding.cpp # src/OpenSHC/AI/AIVState/aiPlaceAdditionalOxtether.cpp # src/OpenSHC/AI/AIVState/aiPlaceBrazier.cpp # src/OpenSHC/AI/AIVState/aiPlaceFarm.cpp # src/OpenSHC/AI/AIVState/aiPlaceIronMine.cpp # src/OpenSHC/AI/AIVState/aiPlacePitchrig.cpp # src/OpenSHC/AI/AIVState/aiPlaceQuarryAndOxtether.cpp # src/OpenSHC/AI/AIVState/findAppropriateGridLocationForResourceTypeBuilding.cpp # src/OpenSHC/AI/AIVState/findAppropriateWoodCutterGridLocation.cpp # src/OpenSHC/AI/AIVState/findSpotNearEngineersGuild.cpp # src/OpenSHC/AI/AIVState/findSuitableFarmLocationAndPlaceFarm.cpp # src/OpenSHC/AI/AIVState/findSuitableGridLocationFittingCriteria.cpp # src/OpenSHC/AI/AIVState/recomputeAIAvailableGridTiles.cpp # src/OpenSHC/AI/AIVState/setKeepOffsetAndOrientation.cpp
144 commits behind main and uncompilable against its regenerated headers, so this branch had never been measured. All 38 sources now build, link and score, with none excluded. The merge conflicted on 15 files, all resolved to main's side. Three definitions took a `PlayerID` parameter. That type has no typedef or enum anywhere in the tree, so those files could never have compiled; the header declares `int playerID`, and the definitions now match it. Parameter names are kept from the .cpp, so executeDefaultCastleAIV keeps `isPausing` where the header says `param_2`. One stale generated field reference was rewritten by the project's refactor_struct_fields.py, and aiPlaceAIVBuilding's field240_0x2c8 is field202_0x2c8 after the Building.hpp regeneration - same offset, same type. First measurement, reimplemented data and 36 of 38 resolvers enabled: BUILD_OK, avg 95.83 over 38, 12 functions below 100. No match work is attempted here - none was possible until it built. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013Fzi11pcBU1Y7Y4oanrZQL
…ding toggle 16e56af ("build executeDefaultCastleAIV and findAppropriateGridLocationForBuilding with /GL") also flipped this resolver to true, which its message does not mention. Resetting it to false. The flip is redundant: enable_local_functions.py sets the resolver of every address implemented by a file in cmake/openshc-sources.txt.local, so with the whole class in the list all 36 resolvable AIVState resolvers come out true regardless of what is committed here, and --disable puts them all back to false. A committed true therefore changes nothing for anyone following the documented workflow. What it does change is the failure case. An enabled resolver whose .cpp is not compiled is an unresolved external, so a narrowed build list over a fresh clone stops linking - the same break this session hit on AICState after a file left the list while its resolver stayed on. False is the safe default because the tooling turns it on when it is wanted. Verified with reimplemented data and all 36 resolvers enabled locally: BUILD_OK, avg 95.83 over 38, unchanged. findAppropriateGridLocationForBuilding stays at 88.2/85.6 and executeDefaultCastleAIV at 53.6/51.9, since the local toggle governs the measurement either way. The /GL entries in cmake/compiler-flags-gl.txt from that commit are left alone. clearTheHeatmaps at line 73 is deliberately committed as true by c63e0f5 ("enable clearTheHeatmaps resolver so wipeAIVsAndHeatMaps tail call matches") and is left as it is; wipeAIVsAndHeatMaps measures 100.0/100.0. It carries the same narrowed-list hazard, so it may be worth the same treatment, but that is a call for whoever made it rather than a stray to clean up. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013Fzi11pcBU1Y7Y4oanrZQL
c63e0f5 ("enable clearTheHeatmaps resolver so wipeAIVsAndHeatMaps tail call matches") committed this resolver as true. The reasoning was sound and is measurable: with clearTheHeatmaps resolved to the original address, wipeAIVsAndHeatMaps reads 95.0, and with it resolved to our own implementation it reads 100.0, which moves the suite from 95.70 to 95.83 over 38. Its normalized score is 100.0 either way, so the whole difference is the tail-call target. It is still not something to commit. enable_local_functions.py sets the resolver of every address implemented by a file in cmake/openshc-sources.txt.local, so the 100.0 is what anyone measuring this class already gets, whatever is in the header; --disable puts it back. What a committed true adds is the failure case, because an enabled resolver whose .cpp is not compiled is an unresolved external, so a narrowed build list over a fresh clone stops linking. With this the branch has no committed true resolvers left, which matches main and the other eleven open reimpl branches. Re-run tools/reimplementation-helper/batch/enable_local_functions.py after a checkout to get the measuring state back, as its own docstring says. Verified with reimplemented data and all 37 resolvers enabled locally: BUILD_OK, avg 95.83 over 38, wipeAIVsAndHeatMaps 100.0/100.0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013Fzi11pcBU1Y7Y4oanrZQL
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reimplements the 37 functions of
OpenSHC::AI::AIVState.Measured with
reccmp-reccmp --target STRONGHOLDCRUSADER --resolve-wrapped-calls, with the global data resolvers enabled locally (tools/reimplementation-control/Enable-Reimplemented-Data.ps1, not committed). 26 of the 37 functions are at 100%.cmake/compiler-flags-gl.txtgainsexecuteDefaultCastleAIV.cppandfindAppropriateGridLocationForBuilding.cpp: the original keepsthisinecxacross that call. For the same reason the resolver offindAppropriateGridLocationForBuildingis set to implemented, andclearTheHeatmapsis set to implemented for the tail call inwipeAIVsAndHeatMaps.Functions below 100%
computeLargestSeparateArea0x4edd00aiDecideOnNewBuildings0x4f15c0aiPlaceFlag0x4ed240rotateAIV0x4ed0b0aiPlaceWoodcuttershut0x4efab0updateBuildingsStateAndUpdateAIBuildingDecisions0x4f1860recomputeHeatmaps0x4f0e80findAppropriateGridLocationForBuilding0x4eee10hostChecksLobbyAIVAvailability0x4ecbc0applyAIV0x4ef0d0executeDefaultCastleAIV0x4f0060In the diffs,
-is the original and+is this branch;@@marks skipped identical instructions.computeLargestSeparateArea- 98.78%Code is identical. The only difference is the name reccmp gives the stack-probe helper:
_chkstkin our build, an unnamed offset in the original. Not reachable from source.reccmp diff (2 differing lines)
aiDecideOnNewBuildings- 98.51%The original stores the initial values of
isPausingandstepIncrementbefore the branch of theaivID <= 0check; ours emits them after it, next to theresourceRebuildDelayload. Declaring the two locals before the check, or nesting the body inif (aivID > 0), moves the stores but disturbs the rest of the function (70-82%).reccmp diff (6 differing lines)
aiPlaceFlag- 98.25%Same instruction and same address. The access is
aics[aiType - 1].flagType, and its constant base lies just before the start of theAICStateglobal, so reccmp names it after the nearest preceding symbol. That symbol differs between the original (EntityState+705348) and our DLL (cached+4). Not reachable from source.reccmp diff (2 differing lines)
rotateAIV- 97.20%Only the order in which four array base addresses are materialised (
lea) before the copy loops differs. Swapping the two statements in the loop body (62.6%), reading the sources into locals first (27.8%) and a flat index forsteps(tie) were measured.reccmp diff (6 differing lines)
aiPlaceWoodcuttershut- 91.38%After the call to
findAppropriateWoodCutterGridLocationthe original comparesbuildingApproriateGridXPositionwith -1 in memory and then loads it; ours loads it and compares the register. Casts, a named local, nested positive conditions and/GLon caller plus both callees all compile to identical code. The trailing+lines are a compare-window artefact.reccmp diff (10 differing lines)
updateBuildingsStateAndUpdateAIBuildingDecisions- 90.83%In the start-of-day loop the original loads
currentPlayerFullIDArray[player]intoeaxand, on the path where it is -1 and the AI slot is 0, executes a redundantcmp eax, eax; je. That looks like a third== -1test with the constant replaced by the register known to hold it. Six ways of writing a redundant third condition (re-read, local, named constant, negated form) are all folded away by our build. The trailing+lines follow from the resulting size difference.reccmp diff (22 differing lines)
recomputeHeatmaps- 88.89%Three things. (1) Constant pooling: the original keeps
0x100000inebx, ours keeps0x90inbl. (2) In both tile loops the original computes y, then x, then divides; ours interleaves the load and the division. Namingy/xlocals in that order compiles to identical code. (3)test eax, 0x10001580is shown as<OFFSET>on our side because the flag mask looks like an address inside the DLL image - a reccmp artefact.reccmp diff (79 differing lines)
findAppropriateGridLocationForBuilding- 88.21%Loop rotation. The original reloads
algAIndexat the loop head, jumps back withjneand rematerialises the constant 1 inedievery iteration; ours reuses the register from the bottom test and setsediin a separate block before jumping back.while,do-whilewith the result inside or after the loop, and afor(;;)with the test at the top score 83.2-88.2%; split entry conditions and hoisted declarations tie.reccmp diff (31 differing lines)
hostChecksLobbyAIVAvailability- 68.63%The per-player availability test is instruction-identical. What differs is the outer loop: the original runs three induction variables (AI index in
ebx, a pointer into the received table inesi, a pointer intoaivFileAvailabilityPerAIArrayinedi) and ends the loop on the pointer; ours derives the received-table pointer from the other one (sub ebx, ecx/lea edx, [ebx + esi]) and ends on the index. That permutes the register roles from the firstpushon. Fourteen source variants (!=bound,uintindex, flat table index, hoisted or removed locals, post-increment store, if/else instead of early return) compile to identical code.reccmp diff (85 differing lines, first 110 diff lines shown)
applyAIV- 67.19%Register assignment in the first grid loop. The original keeps
yon the stack and spendsebxonaivID * 0x922; ours keepsyinebxand spills the product, which changes every stack slot and operand after it. Giving the loops their ownx/y, or sharing them differently, scores 40-55%;/GLscores 57%.reccmp diff (378 differing lines, first 150 diff lines shown)
executeDefaultCastleAIV- 53.56%Register and stack-slot assignment across the whole body. The original keeps the stockpile counter in
ebxwith a home slot at[esp+0x58], the constant 1 inebpand theplayerIDoffset only on the stack; ours keeps the stockpile counter inebpwithout a home slot, 1 inecxand the player offset inebx. The sequence of compares, calls and branches otherwise equals the original, except for the woodsman check (jg+jmpin the original, onejlein ours). Declaration order and placement of the leading locals move the score by less than half a point.reccmp diff (888 differing lines, first 170 diff lines shown)
🤖 Generated with Claude Code