splitscreen: 6 of 13 findings fixed and verified on hardware; root causes and remaining work documented - #3
Draft
wh1ter0se69 wants to merge 42 commits into
Conversation
W3DInGameUI::draw() runs once per frame rather than once per view, and it tested m_seatContexts[0].m_isDragSelecting alone - so a pad seat's lasso was tracked correctly and then never painted. drawSelectionRegion() likewise read seat 0's region unconditionally. It now takes a seat and draw() loops every seat, skipping any seat>0 that has lost its viewport mid-drag so it cannot paint a frozen box. The region is already in absolute screen pixels, so no per-seat transform is needed. Drawing those seats exposes a latent bug that was invisible while nothing painted them: m_dragSelecting/m_dragSeat in SelectionTranslator are ONE state machine shared by all seats, so a second seat pressing steals the drag and leaves the first seat's m_isDragSelecting stuck TRUE - permanently, because that seat's own button-up then takes the else branch and never calls endAreaSelectHint. RAW_MOUSE_LEFT_BUTTON_DOWN now hands the previous owner's lasso back first, via a new InGameUI::endAreaSelectHintForSeat() (the seat being ended is not the seat being translated, so m_activeSeat cannot name it - same reason c2a26a4 needed messageForSeat). reset() also clears every seat's drag flag, which nothing did before, so a lasso live at match end cannot survive into the next match. Single-view is byte-identical: seats 1-7 never set the flag, the pre-emption branch cannot fire with one seat, and endAreaSelectHint still resolves m_activeSeat for all five existing callers.
findAndSelectCommandCenter resolved the seat with getSeatIndexForPlayer(), which answers 'whose viewport shows this player', not 'who is playing it'. An observer seat watching a live AI resolves to a real seat index exactly like a human seat, so the watched army's command centre was selected into the spectator's own selection context - and since round 3k made every ControlBar correctly read its own seat's selection, the now-correct per-seat highlighting faithfully displayed it for the whole match. Rather than change getSeatIndexForPlayer(), which several callers rightly want the watching semantic from, this adds getCommandingSeatIndexForPlayer() as a thin filter over it: same answer, except an observer seat returns -1. The defeat broadcast in VictoryConditions.cpp:213 deliberately keeps the watching form - an observer viewport SHOULD show 'Player N has been defeated' for the AI it is watching - which is the call site that would have silently regressed had the exclusion gone inside the shared function. Corrected that function's doc comment, which claimed 'commands' while the body has always meant 'watches'. GameLogic::selectObject takes the commanding form too, so no logic-driven selection of any kind can land in a spectator's context. Single-view is unchanged on every path: seat 0 can never be an observer (reset clears m_observer, bindFakeSeats only marks seats >= 1, takeOverSeat clears it), so the guard is never taken, and under !RTS_SDL3_ENABLE the new function is a pure passthrough.
ControlBar::onDrawableDeselected called the legacy 2-arg placeBuildAvailable, which forwards to a literal seat 0 (InGameUI.cpp:3458) rather than to m_activeSeat like the selection family does. Dispatch into the function was already correct - ControlBarInstances::get(seat) routes seat 1's deselect to seat 1's bar - so seat 1's own deselect ran and then cleared SEAT 0's armed placement as a side effect. m_seatIndex was already in scope 11 lines above. Two more instances of the same call, both missed by the handoff and both fixed here because they are clear-only and cannot desynchronise anything: ControlBar::processCommandUI runs its clear BEFORE the command switch, so any seat pressing any control-bar button cancelled player 1's placement; and CommandXlat's right-click cancel had cmdSeat resolved and in use on the lines immediately above it. DELIBERATELY NOT FIXED HERE - the ARM sites (ControlBarCommandProcessing.cpp GUI_COMMAND_DOZER_CONSTRUCT and the two special-power variants). Arm and consume are both pinned to seat 0 today, which is why a pad seat's build currently completes at all, wrongly, through seat 0's context. Passing m_seatIndex to the arm calls alone would write the pending placement into seat N's context while PlaceEventTranslator still reads seat 0's - so seat N would arm a build it could never place, which is worse than the present bug. That pair has to move together, and PlaceEventTranslator also projects through TheTacticalView, so seat N's pixels would go through seat 0's camera. Logged as its own finding rather than half-landed here. Single-view is unchanged: every seat resolves to 0.
Every seat's ControlBar points at TheControlBar's single ControlBarSchemeManager (initAsSeatInstance), and the paint callbacks drew through that manager's m_currentScheme - which only ever holds whatever scheme was applied LAST, by any bar. So when player 1 was defeated and killPlayer set the blank FactionObserver skin, all eight viewports went blank; and in general eight players on different factions all showed one faction's artwork. Each bar now records the scheme it was given, at the multiplier it was given with, and draws with its own: ControlBar::setBarScheme, called from a new ControlBarSchemeManager::applyCurrentSchemeToTargetBar that replaces the five identical 'm_currentScheme->init( takeApplyToBar() )' statements. The multiplier is captured verbatim rather than recomputed, because setControlBarScheme uses integer division while the two by-player paths cast to Real - recomputing would have silently changed the by-name/shell path. ControlBarScheme::drawForeground/drawBackground are pure reads of m_layer[], so several bars drawing one scheme object at different offsets is safe. This also removes the setDrawScale() writes from inside the two draw callbacks. Those were mutating shared manager state from a paint path to work around the same root cause; the scale is now simply passed in. Second, orthogonal bug fixed in the same area: because killPlayer's reskin is behind isLocalPlayer(), which is never true for a seat>0 player, a defeated SEAT player's bar never became an observer bar at all. applySchemeForBarPlayer latched on player template alone, and a defeated player keeps their template. It now also latches on isPlayerActive(). Read client-side from the player, so no sim code is asked about seats - Player.cpp is deliberately untouched, per the conventions rule against sim code reading seat state. Single-view: with one bar every apply path already ends in init(TheControlBar), so the recorded pair is by construction the pair the manager held; dock scale is 1; both m_currentScheme and m_barScheme start null and both draws no-op. The applySchemeForBarPlayer change is reached only from syncToSeats over seats 1..7 and is unreachable with one seat.
Not the structural cause the handoff proposed. It claimed recreateControlBar rebuilds three window roots while HideControlBar can only reach one, so the radar and RightHUD were structurally unreachable. Extracting the shipped ControlBar.wnd from WindowZH.big shows exactly ONE column-0 WINDOW block, ControlBarParent; LeftHUD (the radar's DRAWCALLBACK) and RightHUD are its CHILDren. Hiding the parent hides the radar - drawWindow and isHidden both stop at a hidden ancestor. The real cause is a stale-instance resolution. recreateControlBar calls createControlBar(), whose HideControlBar() runs BEFORE 'delete TheControlBar' - so actingControlBar() still resolves the OLD bar, and ControlBar::findBarWindowById scopes strictly to that instance's roots and returns the OLD ControlBarParent. The seat-0 global fallback never runs because the scoped search succeeded. Net: the outgoing, already-hidden root is hidden again and the incoming one - authored ENABLED - stays visible. This is handoff2 5.2's bug class 1 arriving from the opposite direction: not a global lookup returning an arbitrary instance, but a scoped lookup pinned to a dead one. Before splitscreen the global winGetWindowFromId happened to land on the newest root and this worked by accident. Fixed at the end of recreateControlBar, after init() has given the new bar its roots, so both call sites (OptionsMenu and MainMenu) and any future one are covered. Deliberately did NOT take the handoff's first proposal of guarding those two call sites: window geometry is baked from the live display size at parse time and the ControlBar layout is never rebuilt at match start, so skipping the rebuild would leave the bar sized for the old resolution for the rest of the process - a quieter, worse bug. Guard text copied verbatim from the idiom already in OptionsMenu.cpp.
…tion The handoff's diagnosis for this one does not hold. It says seat 0's mouse resting on its own HUD force-resets every other seat's cursor via the TheMouse->getMouseStatus() reads in createCommandHint/createMouseoverHint. getWindowUnderCursor is seat-aware - all three of its window loops skip any window winSeatOwnsWindow() rejects - so for seat N the lookup is essentially always null and underWindow is stuck FALSE. The claimed symptom is the inverse of what that code can produce. The dominant defect is Object::isLocallyControlled(), which compares against ThePlayerList->getLocalPlayer() - seat 0's player - and is read four times across the two functions. The decisive one gates the MOUSEMODE_DEFAULT early return: getSelectCount()/getAllSelectedDrawables() are correctly per-seat, so a seat with exactly ONE of its own units selected yields a non-null srcObj that fails isLocallyControlled(), takes setMouseCursor(ARROW) and returns. With two or more selected, srcObj is null and control falls through to the message-type switch - which predicts a distinctive signature to check against the live repro: the cursor is stuck with one unit selected but changes shape with two. All four now ask isControlledByPlayer(getCommandActingPlayer()), the pattern already used in SelectionXlat and CommandXlat. Exact identity in single view: getCommandActingPlayer() returns the local player whenever the seat override is unset, which is what isLocallyControlled() compares against. The getMouseStatus reads are still wrong and are fixed too, via a getSeatHoverPixel() helper - seat 0 reads TheMouse verbatim, a pad seat reads its own virtual cursor, which is already display-space and clamped to its viewport. Shroud gating: createCommandHint's getObservedOrLocalPlayer read is swapped unconditionally (it early-returns on playback, so the observer case is unreachable); createMouseoverHint's is seat-gated instead, because that function has NO playback guard and an unconditional swap would change replay-observer tooltips to the local player's shroud. The three OS-mouse tooltip writes are now seat-0-only - the tooltip belongs to the pointer, and a pad seat's hover was clearing and rewriting player 1's.
…es landed Records what the pre-implementation verification sweep found, because the refutations are worth more than the fixes and will be re-derived otherwise. The headline: handoff3's 12 open findings had accurate file:line citations and inaccurate reasoning about them. TheSuperHackers#8's prescribed fix is already in the tree and its stated mechanism is architecturally impossible; TheSuperHackers#2/TheSuperHackers#3's would have written only entry [0] of a MAX_SEATS array; TheSuperHackers#9's would have left a pad seat unable to place anything at all; TheSuperHackers#10's and TheSuperHackers#13's named causes cannot produce the reported symptoms; TheSuperHackers#11 is partly dead code behind a static that is never assigned. Also records the build recipe that actually works (VS-bundled cmake, not whatever is on PATH), three SSH/PowerShell failure modes that exit 0 while doing nothing, six new findings nobody had written down, and the probe TheSuperHackers#8 needs before it can be diagnosed at all - the existing input log is structurally blind to click messages.
…raps A fresh build of this branch does not start on the test box, and the BASELINE commit f72603e crashes identically with none of this round's fixes in it - which is what clears them. Both stdout and stderr are 0 bytes and no DXVK log appears, so it dies before the graphics device or any engine logging; the pre-existing GeneralsX binary emits ~156KB and a DXVK log on the same box with the same env and data. Records two measurement traps that each produced a confidently wrong answer before being caught. Get-Process is not a liveness test here - a crashed instance stays alive holding the Technical Difficulties modal, which scored crashes as successes and manufactured a non-monotonic seat-count 'boundary' and a bogus conclusion that -splitscreendev was to blame. And launching over plain SSH always dies 0xC0000005 whatever the binary, so an SSH-vs-task comparison is not an A/B at all. Ground truth is ReleaseCrashInfo.txt's mtime against the run start. Also root-causes the STATUS_DLL_NOT_FOUND seen when relocating the exe: the build links against its own binkw32/mss32 under _deps, which differ by hash from the ones in the existing run dir.
Two stacked defects. Object::attemptDamage gated the radar event on isLocallyControlled(), which compares against ThePlayerList's local player, so the event never fired at all for a seat>0 unit. And inside Radar::tryUnderAttackEvent every consumer resolved seat 0: the message feed, the radar frame glow, and the EVA branch. The gate now asks getSeatIndexForPlayer(...) >= 0 - does ANY local seat command this player - kept last in the && chain so the seat scan is only reached after m_radarData != nullptr has culled most objects. Deliberately NOT done by changing Object::isLocallyControlled() (28 callers) nor by adding an isLocallyControlledByAnySeat() to Object's public API, where sim code could reach for a seat concept. Inside tryUnderAttackEvent the concerned player is resolved once, and all four message() calls route through messageForSeat() - the handoff names only three; the fourth is the generic 'RADAR:UnderAttack' else branch, and without it the default case still lands in seat 0's viewport. triggerRadarAttackGlow now flashes the concerned seat's own bar (ControlBarInstances::get returns nullptr for an unregistered seat, unlike fromWindow, so the fallback to TheControlBar is mandatory) - that site is not in the handoff either, and without it the wrong radar keeps blinking. The EVA test, which was a third instance of isLocalPlayer() and is dead today because the old gate guaranteed it true, becomes live and correct. DELIBERATELY NOT FIXED, logged in handoff4 instead: widening the gate means every seat can now raise a radar event, and tryEvent dedups map-wide for 10s under PRESERVE_RADAR_WARNING_SUPPRESSION - so seat 0 being attacked can swallow another seat's warning. And W3DRadar::drawEvents has no owner filter, so blips leak across viewports. Both want an owner on RadarEvent, which sits in Radar's xfer chain; the conventions say stop and ask before touching that.
…ntation, not a fix) handoff3's stated cause for TheSuperHackers#8 is refuted - getWindowUnderCursor IS seat-aware and its doc comment names that exact hypothesis as the bug it was added to kill - and its stated verification cannot be performed, because the translator trace filters to msg type >= MSG_BEGIN_META_MESSAGES (177) while MSG_MOUSE_LEFT_CLICK is 163. splitscreen_input.log has never been able to see a click. Adds the two readings that discriminate the surviving hypotheses, both gated on GX_CLICKPROBE so a normal build pays nothing: [GXPICK] in W3DView::pickDrawable - the acting seat, whether getWindowUnderCursor returned a window at that pixel, its id, and whether an opaque window is what refused the pick. [GXCLICK] in SelectionXlat at the empty-list break - the message's seat tag, getCommandActingSeat(), isPoint, the pixel region, and how many drawables the region actually yielded. Reading them: window non-null and owned by the acting seat's own bar => the narrowed window theory; window non-null via getWindowUnderCursor's m_grabWindow/m_mouseCaptor early return (which sit BEFORE any seat filter, and are swapped per seat by winBeginSeatInput) => a stale per-seat grab, which is the best surviving explanation; both null => the ray-cast itself missed; isPoint FALSE => the click never entered the pick path and the finding is misfiled. No fix is attempted - handoff3's own instruction was to get a repro before committing to a shape, and that is still right.
…7 never got one The handoff's fix shape for this would have shipped a no-op. It said to make ScriptActions::m_messageWindow a [MAX_SEATS] array and resolve getSeatIndexForPlayer 'for the player the script action concerns'. There is no such player at those call sites, and only entry [0] would ever be written: the MP victory/defeat SCRIPTS are appended to side 0 alone (GameLogic.cpp:1607) and their conditions resolve through m_localSlotNum, so doVictory/doDefeat/ doLocalDefeat fire at most once per match, for seat 0's player. Seats 1..7 never reach ScriptActions at all. TheScriptEngine->getCurrentPlayer() is live there but is side 0's player, so threading it in would have looked correct while mislabelling every splash. So this needs two halves, and does both. (a) Positioning. The splash pointer moves out of the file-scope static in ScriptActions and into SeatUIContext (Pattern B, as c2a26a4 did for the message feed), behind InGameUI::showOutcomeSplashForSeat(). A seat whose view is smaller than the display gets the layout scaled and centred into its own viewport, using the same root-takes-the-transform mapping as ControlBar::dockToRect. winCreateFromScript returns only the first root, so the WindowLayoutInfo out-param is used to reach them all. (b) The missing trigger. VictoryConditions::update is the one place that already detects defeat per player and victory per alliance, and c2a26a4 already wired it to getSeatIndexForPlayer. Seats > 0 now get LocalDefeat.wnd on elimination and Victorious/Defeat.wnd when the match resolves. The seat>0 path deliberately runs NOTHING else from doLocalDefeat - doDisableInput, closeWindows, startCloseWindowTimer, SetVictorious and markMPLocalDefeatWindowShown are all machine-global. Seat 3 losing must not freeze seats 0-2 or end the match, and leaving MPLocalDefeatWindowShown alone keeps seat 0's later victory as Victorious.wnd rather than ObserverQuit.wnd. Single-view is byte-identical: the transform is gated on splitscreen being enabled AND the seat's view being strictly smaller than the display, which is never true for seat 0; the new triggers are guarded > 0.
…erHackers#11 were held Adds the per-finding test recipe mirelle asked for, leading with the two falsifiable predictions: TheSuperHackers#10's cursor should be stuck with exactly ONE unit selected and fine with two or more, and TheSuperHackers#7's pad lasso should DISAPPEAR rather than freeze when seat 0 pre-empts the drag. If either fails the diagnosis is wrong and should be re-opened rather than patched around. Records why #1, TheSuperHackers#11 and TheSuperHackers#9's arm/consume pair were deliberately not attempted: each is large, none is runtime-verifiable from a Mac, and each fails in a way a one-match smoke test misses - #1 crashes only on the SECOND match via the omitted forgetBarLayout, on a path single-view also takes; TheSuperHackers#11 leaves a seat's popup permanently on screen if the update-func half is missed; TheSuperHackers#9's arm side alone leaves a pad seat unable to place anything. Full plans are in section 6.
Diplomacy.cpp had no seat concept at all - grep for 'seat' returned zero hits in the whole file. One layout, one window, one AnimateWindowManager and one set of per-slot widget pointers, so the popup opened at Diplomacy.wnd's authored full-display position whoever pressed the button, and a second seat opening it stomped the first seat's pointers. ControlBarCallback called ToggleDiplomacy(FALSE) with no seat, unlike the GBM_MOUSE_ENTERING/LEAVING handlers directly above it and the generals button in the same switch, both of which already resolve the instance via ControlBarInstances::fromWindow. All per-INSTANCE state is now indexed by seat. The NameKeyTypes are NOT - they are derived from layout-name strings and are identical for every instance; turning them into arrays would have been pure noise, which is exactly what a scripted rename over that block would have produced. The bodies that walk slots alias the arrays back to their original local names so they stay untouched. Positioning is not enough on its own, and this is the part the handoff missed: winSeatOwnsWindow's own comment keeps 'the quit menu, diplomacy, message boxes, the whole shell' with seat 0, so a seat>0 could have seen a correctly-placed popup and been unable to press a single button in it. The layout is therefore adopted by the seat's own ControlBar, which is the only mechanism that exists for this - the generals screen and the special-power shortcut bar are the precedents. That buys position, per-frame re-dock, paint clipping AND click ownership together. Added ControlBar::adoptPopupLayout() as the public entry point (redockAfterRootsChanged is protected) and made it report the MAX_BAR_LAYOUT_WINDOWS overflow instead of silently half-docking, since a silent drop looks identical to the fix doing nothing. ResetDiplomacy calls forgetBarLayout BEFORE destroyWindows for every seat. That is mandatory, not hygiene: it runs on every match teardown including single-view, and without it dockToRect writes through freed GameWindows every frame - a crash that surfaces later inside winSetFont with an unrelated stack and only reproduces on the SECOND match. The five global winGetWindowFromId lookups are now winFindChildById scoped to the seat's own tree (bug class 1). Default arguments keep every existing caller compiling unchanged; seat < 0 means seat 0 for show/toggle and every seat for hide/reset/populate, so the GameLogic-side callers stay seat-free.
…t 0's anchor commandButtonTooltip called the GLOBAL TheControlBar->showBuildTooltipLayout, so hovering any seat's button raised seat 0's tooltip, anchored off seat 0's marker. It now resolves the firing bar via ControlBarInstances::fromWindow - the mechanism WP8 built for exactly this - which falls back to TheControlBar, so single view is the same object and the same call. The routing fix alone is not safe, and this is the part the handoff does not mention: ControlBarPopupDescriptionUpdateFunc is installed on EVERY instance's layout and run per instance, but drove the global TheControlBar. Dormant only because no seat>0 layout was ever shown - and made live by the routing change itself. Without the companion fix, seat N's popup is evaluated against seat 0's m_showBuildToolTipLayout and never hides, while seat 0's layout is deleted instead. ControlBar::update now passes 'this' through runUpdate's existing userData parameter. Four hover/delay statics (one file static, two function statics, one more for the offset) become per-bar members - same defect and same fix as m_lastMoneyShown, whose comment already records that function statics made one bar's value suppress another's. Six global window lookups scoped: three ControlBar.wnd ids through findBarWindowById, three ControlBarPopupDescription.wnd ids through a new findTooltipWindowById that searches only this bar's own tooltip roots. The first three only ever resolved via winGetWindowFromId's sibling walk, so with N bars they matched an arbitrary bar's copy and the == test dropped into the DEBUG_CRASH. The BackgroundMarker anchor had a second defect independent of the lookup: getBackgroundMarkerPos returns an AUTHORED coordinate captured once at init, mixed against a DOCKED winGetScreenPosition - so the anchor is wrong for any docked bar even on its own. Scaled by the bar's dock scale, the correction W3DControlBar already carries. Exactly 1 for an undocked bar. BEHAVIOURAL DELTA, deliberate, not byte-identical: the tooltip now prices against getCurrentlyViewedPlayer() rather than ThePlayerList->getLocalPlayer(). Identical unless observer mode is on, where it now prices against the observed player - which matches what line 570 of the same function already did. NOT DONE, deliberately - the SIZE half. Registering the tooltip layout into the bar's dockToRect pass is what would make it shrink with the viewport, but populateBuildTooltipLayout grows the description box at runtime with raw winSetSize/winSetPosition, and dockToRect re-applies authored geometry every frame - in single view too. Registering without first converting those to placeBarWindow/resizeBarWindow would collapse the popup back to its authored 102px height on the very next frame, for everyone. That pair has to land together; logged in handoff4. Also dropped from the finding: theAnimateWindowManager. The file-scope flag 'useAnimation' is never assigned anywhere, so the manager is permanently nullptr and there is no slide-in to bound - a fix there would ship nothing while still forcing a redeploy.
…mera Completes finding TheSuperHackers#9. The earlier commit landed only the CLEAR-only sites, because moving the arm side alone would have been a regression: arm and consume were both pinned to a literal seat 0 by the legacy accessor family, and that symmetry is the only reason a pad seat's build completed at all - wrongly, through seat 0's pending placement and seat 0's cursor. Passing m_seatIndex to the arm calls while PlaceEventTranslator still read seat 0 would have made getPendingPlaceType() return nullptr for that seat, so seat N could arm a build and then never place anything. So the pair moves together here: arm - the three ControlBarCommandProcessing sites (DOZER_CONSTRUCT and the two special-power variants) now pass m_seatIndex, which is already in scope and already used elsewhere in the same function. consume - PlaceEventTranslator, the only consumer, resolves the acting seat once from the message and routes every placement read and write through it: getPendingPlaceType, isPlacementAnchored, getPendingPlaceSourceObjectID, getPlacementAngle, getPlacementPoints and the four placeBuildAvailable clears. Every seat-taking overload already existed. Third defect in the same path, not in the handoff: all three screenToTerrain calls projected through TheTacticalView - seat 0's camera. A pad seat's pixels were being unprojected through the wrong view, so even once the placement is armed in the right context the building would land in the wrong world position. They now go through the acting seat's view, falling back to TheTacticalView. Single view is unchanged: every seat resolves to 0 and getCommandActingView() returns TheTacticalView.
…with a debugger Two separate environment problems, neither in any fix from this round and both reproducing on the untouched baseline commit. The init crash is INI::loadFileDirectory throwing INI_CANT_OPEN_FILE for Data\INI\Weather (GameEngine.cpp:488), caught by the bare catch(...) at :788. The file exists in retail INIZH.big and the three sibling loads succeed - the difference is the five mod archives in the test install, whose ! prefix sorts them first and shadows the directory listing. Excluding them produces no C++ throw at all. Same archives the TheSuperHackers#13 analysis flagged as able to override ControlBar.wnd, biting somewhere else first. Underneath that sits a second, still-open failure: a first-chance access violation in W3DDisplay::getDisplayModeCount during display enumeration. An AV is not a C++ throw so catch(...) never sees it and the process dies with no crash record and zero stderr - which is precisely why this presented as 'crashes at init with no information' until a debugger was attached. Also records that CNC_GENERALS_ZH_PATH does not exist on this branch at all (an unbounded grep returns zero hits; it is a GeneralsX-fork addition), so this build finds data via the working directory only.
The AV in W3DDisplay::getDisplayModeCount is fully explained: dx8wrapper does
LoadLibrary("D3D8.DLL") and the process loads Windows' SysWOW64 stub rather
than the DXVK copy sitting next to the exe, so zero render devices are
enumerated and Get_Render_Device_Desc(0) returns a garbage reference.
Records what has been ruled out by measurement so it is not re-derived: the
DXVK dll is present and byte-identical to the one the working binary uses,
its dependencies resolve, vulkan-1 is present, the GPU is capable, dxvk.conf
makes no difference, mirroring the entire working run directory still
reproduces it, and SDL3 does not harden the process-wide DLL search path
(it scopes LOAD_LIBRARY_SEARCH_SYSTEM32 to combase.dll only).
That leaves it as a difference in this binary's loader behaviour versus the
GeneralsX fork's, with identical DLLs in identical directories - which is a
question for whoever runs this branch successfully today.
…rent path
StdLocalFileSystem::getFileListInDirectory passed only the LEAF name of each
subdirectory into its recursive call:
AsciiString tempsearchstr(filenameStr.c_str());
getFileListInDirectory(tempsearchstr, originalDirectory, ...);
so every level rebuilt the path as originalDirectory + leaf, losing the parent
entirely. A bare leaf re-resolved against originalDirectory can land back on a
directory already being walked, and the function then recurses until the stack
is gone. Caught with a debugger on a real launch: 498 stacked frames of this
one function, ending in c00000fd STACK OVERFLOW inside
std::filesystem::directory_iterator. It kills the process about ten seconds in,
AFTER DXVK has initialised and the swapchain exists, with no crash record and
no stderr - so it presents as 'the game just vanishes'.
The Win32 implementation of the identical function has always done this
correctly (Win32LocalFileSystem.cpp): it concatenates currentDirectory + name +
separator, keeping the full relative path as it descends. This makes the Std
version behave the same, with the separator matching the platform.
Found while trying to get a runtime verification pass for the splitscreen
fixes; unrelated to them, and it blocks any run whose data directory has
subdirectories.
std::filesystem::path::string() throws std::system_error when the name will not convert to the narrow code page. StdLocalFileSystem::getFileListInDirectory called it unguarded on every entry it walked, so a single map or file with a non-ASCII name anywhere under the search root threw all the way out to the catch(...) in GameEngine::init and became 'Uncaught Exception during initialization'. Caught with a debugger on a real launch: std::_Throw_system_error_from_std_win_error std::_Convert_wide_to_narrow std::filesystem::path::string StdLocalFileSystem::getFileListInDirectory @ 242 StdLocalFileSystem::getFileListInDirectory @ 290 FileSystem::getFileListInDirectory MapCache::loadMapsFromDisk @ MapUtil.cpp:525 so it is the MAP SCAN that trips it, which means any user with an oddly named map cannot start the game at all - and the error names neither the file nor the directory. A file this engine cannot name is a file it cannot open either, so both loops now skip such an entry and keep walking, logging the directory. This only became reachable once the previous commit made the recursion actually descend into subdirectories.
…s a flag
The dispatcher is 'arg += func(&argv[0]+arg, argc-arg)': num counts the flag
plus every token after it, and the return value is how many tokens to consume.
A parser that reads args[1] must therefore return 2.
parseMapName got both halves wrong:
* 'if (num == 2)' meant the map name was only read when '-map <name>' happened
to be the LAST two tokens on the line. Put any flag after it and -map did
nothing whatsoever, silently.
* 'return 1' consumed only the flag, leaving the map name itself to be matched
against the flag table on the next iteration. Harmless while no map name
collides with an option name, but it is matching user data against the
option table, which is not a property to rely on.
parseFullVersion had the same return-count bug (guard was already correct).
Audited every parser in the table: these two were the only ones that read
args[1] and could return 1. The parsers that return either 1 or 2
(-splitscreendev, -xres, -yres, -replay, -jobs, -jumptoframe) are correct -
they take an OPTIONAL value and return the count they actually consumed.
Fixed in both trees since the defect is identical and is not splitscreen work.
NOTE: the Generals/ target does not build on this branch (documented in
handoff2 5.4), so that copy is unverified by compilation - the change is the
same two lines as the GeneralsMD one, which is built and verified.
Excluding the five mod archives made the INI_CANT_OPEN_FILE throw disappear, and this document blamed them for it. That was a coincidence of load order and the conclusion was wrong. The actual cause was the StdLocalFileSystem recursion bug fixed in 782c609: loadFileDirectory finds Data\INI\Weather through the recursive directory walk, and the walk was resolving bare leaf names against the wrong parent, so it read zero files and threw. With the recursion fixed the game loads with all five mod archives present - verified in-game on the operator's own modded install, 8-seat splitscreen running. Recording this because the wrong version told the next person to strip a user's mods, which would have been bad advice for a symptom that no longer exists.
… seat User-confirmed TheSuperHackers#10 is NOT fixed and my diagnosis was wrong: the pad seat's cursor stays a plain arrow with ONE unit selected AND with two or more. The isLocallyControlled defect predicted an asymmetry (broken at 1, fine at 2+); there is none, so something upstream is killing the hint entirely. Prime suspect is the early return: m_isScrolling, m_isSelecting and m_mouseMode are single-instance InGameUI members, not SeatUIContext fields, so seat 0's state suppresses hint generation for every other seat. That was identified during the verification sweep and deliberately logged as a separate work item rather than fixed - which now looks like the wrong call. GX_CURSORPROBE logs, per hint message: the acting seat, the message type, m_isScrolling/m_isSelecting/m_mouseMode, whether the early return fired, and inside MOUSEMODE_DEFAULT whether underWindow or the srcObj ownership test sent it to ARROW. That distinguishes 'never runs' from 'runs and picks ARROW', which is the fork the fix depends on.
…TheSuperHackers#8 and TheSuperHackers#10 are one bug Real pad on a real seat, Release build, operator-driven. TheSuperHackers#7 VERIFIED. Both halves confirmed by the operator: the pad seat's lasso draws and it vanishes rather than freezing when seat 0 pre-empts the drag. TheSuperHackers#10 NOT FIXED, and the diagnosis recorded here was wrong. It predicted an asymmetry - broken with one unit selected, fine with two or more. There is no asymmetry; the cursor is a plain arrow either way. The probe shows the single-instance scrolling/selecting early return is NOT responsible (scrolling=0 selecting=0), the window gate is not blocking (underWindow=0), and the isControlledByPlayer fix genuinely works (srcOwned=1) - but mousedOver=0 on every sample, so there is never a hovered drawable to decide a shape from. TheSuperHackers#8 REPRODUCED with the probe, and handoff3's stated cause is refuted by measurement: isPoint=1 and the acting seat resolves correctly, willSelect=0 means the region yielded zero drawables, and no [GXPICK] line shows a window blocking the pad seat - the only refusals belong to seat 0. Both are the same defect: SelectionXlat builds the mouseover hint from pickDrawable, so an empty pick gives no hover (no cursor shape) and no selection. Leading hypothesis is a coordinate-space mismatch between the display-absolute click region and the view-relative pick ray; that is the first thing to establish, not to assume.
Records where the branch stands, what runs, and the one unanswered question that decides whether TheSuperHackers#10 is fixed or still broken. Nothing is pushed anywhere - no remote, no fork, no PR. The Windows box is kept in sync by a git bundle over scp.
…e hint said TheSuperHackers#10 again, and the previous diagnosis was wrong in an instructive way. It kept asking which gate in the hint chain killed the cursor. None of them did. The shape decision was always arriving correctly and the RENDERER could not draw it. cursorTextureFileName picked the filename off info->numFrames: <=1 meant the unnumbered form, otherwise <name>%04d.tga. numFrames is ALWAYS 1, because retail Mouse.ini declares no frame count for any cursor - an unbounded grep for `Frames` over the shipped Data\INI\Mouse.ini returns zero hits, the only anim key present being `Directions = 8` on Scroll. So this always asked for the unnumbered name. The shipped art does not agree. Scanning every .big in a retail ZH install: Art\Textures\sccmove0000.dds .. sccmove0020.dds 21 frames, NO plain file Art\Textures\sccscroll0000.dds .. 4 frames, NO plain file Art\Textures\sccpointer.dds plain, exists Art\Textures\sccattack.dds plain AND numbered so the lookup for SCCMove missed, WW3D returned its 128x128 missing-texture placeholder, the size guard correctly refused it as not cursor-shaped, and drawSeatCursor substituted ARROW. Move and Scroll could not render for any seat, ever, regardless of what createCommandHint decided. That is also why the only shapes ever seen on a pad seat were the arrow and the attack cursor: SCCPointer and SCCAttack are the two that ship unnumbered. The "cursor works on enemies but not on my own units" report was never about picking at all - Select has no texture art whatsoever, only a .ani. Fix: ask the art rather than the INI. Measure the unnumbered file, take it if it measures cursor-shaped, else use the numbered one; resolve once per cursor state and cache. Left UNRESOLVED rather than latching a guess when neither measures, because cursor textures load on demand and the first frames a seat cursor is drawn can measure nothing - and re-point the cached Image when it does resolve, or it keeps drawing from the name we just established was wrong. Player 1 is untouched: it runs RM_WINDOWS and draws the OS .ani cursors from Data\Cursors, never these textures. Still open, logged not fixed: 27 of 37 cursor states have no texture art at all (AttackMove, Select, Enter, Waypoint, ...) and exist only as .ani and .W3D. A pad seat still falls back to the arrow for those. That needs a different art source and is its own change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… visibility, not the acting seat's
Instrumentation, not a fix. Default OFF; set GX_PICKALL=1.
"Click does not select, drag does" has exactly two candidate gates, because both
go through iterateDrawablesInRegion from the same call site with the same struct
and differ only in branch. The rect branch walks TheGameClient->firstDrawable()
and projects each. The point branch delegates to pickDrawable, which adds (a) the
window gate - already cleared by measurement, underWindow=0 - and (b)
castRay(raytest, testAll=false, pickType).
(b) filters on Is_Really_Visible(). That flag is pure RENDER RESIDUE:
W3DScene.cpp:534-655 Visibility_Check rewrites it for every render object
from THIS view's camera frustum and THIS view's
player's vision (seatOwnerFilterHidesObject)
W3DScene.cpp:1466 Visibility_Checked is cleared straight after, so the
pass genuinely re-runs per view
Display.cpp:163 drawViews walks the view list head to tail
Display.cpp:109 attachView PREPENDS
so seat 0's view - attached first at InGameUI.cpp:1442, the only other
attachView being the seat>0 one - sits at the tail and is drawn LAST, and seat
0's visibility set is the one standing when the message stream is translated
(GameClient.cpp:820 draws, GameEngine.cpp:929-930 then propagates).
A pad seat's point pick is therefore answered against what SEAT 0 can see. Its
own units, framed by its own camera somewhere seat 0 is not looking, are
frustum-culled or shrouded away and the ray never tests them. The drag path
never reads the flag. That is the reported asymmetry exactly, and it predicts
seat 0 keeps working - which it does.
Not yet proven, hence a probe rather than a fix. GX_PICKALL=1 bypasses the
filter: if a pad seat's click-select starts working with it set, the diagnosis
is confirmed; if it does not, the cause is elsewhere and this should be reverted
rather than built on. Bypassing is NOT the fix either way - it would let a seat
pick units hidden in its own fog. The real fix is to evaluate visibility for the
PICKING seat instead of reusing the last render pass's answer.
Also retracts a load-bearing piece of handoff4's evidence: "mousedOver=0 on every
sample" is not diagnostic. InGameUI.cpp:3064 sets m_mousedOverDrawableID to
INVALID in the terrain branch, so 0 is the CORRECT value whenever the cursor is
over ground - which is what was being sampled.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Follow-up to the seat cursor fix. cursorUsesNumberedArt left s_naming UNRESOLVED whenever neither the unnumbered nor the numbered texture measured, so the 27 retail cursor states that have no texture art in either convention - they exist only as Data\Cursors\*.ani and Art\W3D\*.W3D - re-measured two absent textures on every seat on every frame, forever. Adds an ABSENT state so a conclusive miss is latched. The wrinkle is that Get_Texture returns its placeholder both for "file does not exist" and for "archives not mounted yet", and those are indistinguishable at this level, so ABSENT is only latched after a bounded number of attempts - one second of frames, far more than a mounted archive needs. Behaviour is unchanged: ABSENT reports not-numbered, the size guard rejects the placeholder and drawSeatCursor falls back to the arrow, exactly as before. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… not its own Fixes TheSuperHackers#8, and TheSuperHackers#10's remaining half with it. Confirmed by A/B on a real pad, not argued. W3DView::pickDrawable casts through RTS3DScene::castRay with testAll=false, which considers only render objects flagged Is_Really_Visible(). That flag is RENDER RESIDUE. Visibility_Check rewrites it for every object in the scene once per VIEW per frame, from that view's camera frustum and that view's player's vision. Display::drawViews walks the view list head to tail and Display::attachView PREPENDS, so seat 0's view - attached first at InGameUI.cpp:1442 - sits at the tail and is drawn LAST. Its answer is the one still standing when the message stream is translated next frame. So every seat's point-pick was answered with SEAT 0's vision. A pad seat's own units, framed by its own camera somewhere seat 0 was not looking, were frustum-culled or shrouded away and the ray never tested them. Drag-select kept working because iterateDrawablesInRegion's rect branch walks TheGameClient->firstDrawable() and never reads the flag - which is exactly why the bug presented as "drag selects, click does not", for seats > 0 only. It explains the cursor too, so TheSuperHackers#8 and TheSuperHackers#10 really were one defect - handoff4 was right about that and wrong about the mechanism. createCommandHint takes `draw` from the pick, so an empty pick leaves drawSelectable FALSE and the MSG_DO_MOVETO_HINT arm falls through to MOVETO. That is the move cursor appearing over your own units instead of SELECTING. Fix: evaluate the predicate live for the picking view instead of reading the leftover bit. objectVisibleToView is Visibility_Check's own per-object decision - force-visible, hidden, frustum cull, seat owner filter, effectively-hidden/shroud-obscured - in the same order, so the two cannot drift. castRay takes an optional view camera and player; passing them switches it from residue to live evaluation. Bypassing the filter (testAll=TRUE) is NOT the fix even though it made the symptom go away in the probe run: it would let a seat pick units hidden in its own fog. Single view is byte-identical: the call site only supplies a camera when getBoundSeatCount() > 1, so a one-seat game takes the original branch unchanged. The Generals tree gets the same signature so the shared Core W3DView has one spelling to call. It has no splitscreen and no owner filter, so a supplied camera there only swaps the residue for a live frustum test, and viewPlayerIndex is unused. Supersedes the GX_PICKALL probe from 0a17bb6, which has served its purpose and is removed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… other seat saw a ghost Reported live: on a pad seat, click a structure in the control bar, click again to place, nothing happens - and the building preview never appears at all. The arm side was already correct. ControlBarCommandProcessing passes m_seatIndex, so placeBuildAvailable sets ctx.m_pendingPlaceType for the right seat, creates the preview drawable and tags it to that seat's player. Nothing was wrong there, which is why this survived the round that fixed TheSuperHackers#9's arm/consume pair. What was missing is the PER-FRAME UPDATE. handleBuildPlacements resolves everything through m_activeSeat, and it is called from InGameUI::update - not from message translation. m_activeSeat is only non-zero while a seat's message is being translated; InGameUI.h says so directly: "Render/HUD code always runs with m_activeSeat == 0". So the update serviced seat 0 and nothing else. A pad seat's ghost was created and then never moved to that seat's cursor, never legality-checked, never tinted - so it sat wherever it was born and the placement could not be completed. Two more seat-0 assumptions inside the same function would have kept it broken even once the right context was reached: * the cursor position came from TheMouse->getMouseStatus(), the OS pointer. A pad seat has none, so every seat's ghost would have tracked seat 0's mouse. * all five screenToTerrain calls went through TheTacticalView, seat 0's camera, so a seat's pixels projected to the wrong world position - a building placed somewhere other than where it was aimed. Fix: loop the seats that actually have a placement pending and scope m_activeSeat around the body, which is the same mechanism MessageStream already uses. That is what keeps the change small - every legacy accessor inside (isPlacementAnchored, getPlacementPoints, getPendingPlaceSourceObjectID, the m_seatContexts lookups) then answers for the right seat with no further edit. The cursor now comes from getSeatHoverPixel, which returns TheMouse for seat 0 unchanged, and the projections go through that seat's own View. Single view is unchanged: the loop finds seat 0 alone, setActiveSeat(0) is what it already was, getSeatHoverPixel(0) is TheMouse, and viewForSeat(0) is TheTacticalView. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…greed Two places decide whether a fogged building's stand-in belongs in a viewport, and they said different things about the same object. seatOwnerFilterHidesObject - the one Visibility_Check consults - is deliberately lenient (W3DScene.cpp:524-532). The scene holds ONE snapshot per object, whichever seat fogged it last, but every seat that has fogged it recorded its own and they all depict the same building in the same place. So a viewport whose player also remembers the object is shown the stand-in; only a player with no memory of it at all is shown nothing. Its comment says exactly that. renderSingleDrawable then tested only "does the scene copy belong to me" and returned early otherwise, which puts back the hole that rule exists to avoid: a seat that had scouted a civilian building saw nothing there as soon as another local seat was the more recent one to fog it. The disagreement was pre-existing - Visibility_Check has always used the lenient form - but it was one-sided and mostly invisible. It became reachable from the other side in 66c286d, which made the pick answer per-seat: a seat could then CLICK a ghost building its own viewport was refusing to draw. Fix: the draw gate now asks the same question as the filter. Single view is unaffected - with one seat there is only ever one snapshot owner, so the added term cannot change the answer. This is a coherence fix, not a claim to have closed TheSuperHackers#12. TheSuperHackers#12 has a second, independent sub-mechanism - snapShot displaces the real render object out of RenderList under an ownsScene guard that restoreParentObject has no counterpart for, so a seat meeting a displaced building for the first time has neither a snapshot nor a FOGGED->clear edge to trigger a restore, and no visibility predicate can reach an object that is not in the list at all. That one still needs a probe run to confirm it is the live case. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…print Regression from 478b07d, reported live: with a placement armed on the pad seat, seat 0's placement square stopped drawing - "there is no more square that draws; I had to build or move to make it appear again". removeAllBibs() is GLOBAL - it clears every seat's footprint decal at once - and 478b07d left it inside the body that now runs once per seat. So on each odd frame seat 0's pass added its bib and seat 1's pass immediately removed it again, permanently, for as long as any other seat had a placement armed. Only the last seat in the loop kept a footprint. Hoisted the clear into the caller, on the same odd-frame cadence the per-seat legality check uses so the two stay in step. Each seat then adds its own bib after the single global clear. Single view is unchanged: with one seat the clear happens once per odd frame and one seat adds its bib, exactly as before. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…d seat could never build Three reported symptoms, one cause. Operator, on a pad seat: the building ghost is red everywhere so it can never be placed; the same press that should place it also orders the dozer to walk there; and the ghost only appears while the button is held. Every no-arg overload in the placement accessor family forwards to a LITERAL 0 (InGameUI.cpp:3848, :3862, :3879, :3906, :3924, :3941, :3961) - not m_activeSeat, not the acting seat. Both the message translator and the per-frame ghost updater are seat-scoped and call them. RED EVERYWHERE. handleBuildPlacementsForActiveSeat read getPendingPlaceSourceObjectID() no-arg, so seat 1 got seat 0's builder - INVALID_ID whenever seat 0 has nothing armed. BuildAssistant.cpp:936 then leaves playerIndex at -1, and PartitionManager.cpp:3258 returns CELLSHROUD_SHROUDED for a negative player before it looks at a cell at all, so isLocationLegalToBuild returns LBC_SHROUD at every position on the map. InGameUI.cpp:1885 is the engine's only IllegalBuildColor tint, so the ghost is red by construction, everywhere, forever. The DEBUG_ASSERTCRASH guarding that case is compiled to (void)0 in this build. CLICK ALSO MOVES. PlaceEventTranslator reads through placeSeat everywhere (:90, :91, :103, :177, :190, :195, :338, :348) but four WRITES had no seat: :115, :154, :317, :356. So a pad press armed seat 0's anchor, seat 1's own isPlacementAnchored(placeSeat) at :177 stayed FALSE, the commit block was skipped, and disp stayed at its KEEP_MESSAGE initialiser - the click fell through to the command translator, which issued the move. Translator order is Place 30 before Command 70 (GameClient.cpp:294-303), so "placement runs too late" is refuted: it saw the click first and declined it. GHOST ONLY WHILE HELD. Same latch from the other side: once a pad press sets seat 0's anchor, isPlacementAnchored() reads TRUE for every seat, so the ghost position comes from seat 0's stale anchor pixel instead of the correct getSeatHoverPixel(m_activeSeat) branch. That is also why the stick did not move the ghost but the button did. Fixed by routing to the seat the code is already scoped to: six reads in handleBuildPlacementsForActiveSeat take m_activeSeat, four writes in PlaceEventTranslator take placeSeat. Three further no-arg calls in seat-scoped translators went with them, one of which is a real cross-seat bug in its own right: SelectionXlat.cpp:1044 tested seat 0's placement and :1046 called the 2-arg placeBuildAvailable, so a PAD seat's right-click cancelled PLAYER 1's building placement. Also SelectionXlat.cpp:983 and WindowXlat.cpp:262. Left alone deliberately: W3DInGameUI.cpp:687 reads isPlacementAnchored() from RENDER code, where m_activeSeat is always 0. It drives m_buildingPlacementAnchor/m_buildingPlacementArrow, which are single shared render objects - making the line-build drag arrow per-seat is a separate change, not a one-line seat argument. Single view is unchanged throughout: with one seat every argument added here is 0, which is exactly what the no-arg overloads were passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The on-screen SEATCURSOR line latched to the first seat drawn each frame, which is always seat 0, so it could not answer the only question now outstanding about cursors: what shape is a PAD seat asking for, and what art does that resolve to. The latch was itself a fix for the opposite failure - the LAST seat used to overwrite the line, so a perfectly healthy "seat7 SCCPointer.tga 32x32" stood in for seat 0's broken cursor. One line per seat answers both without either hiding the other, and mirrors the control bar report that already works this way. This matters before any work on the missing cursor art. 27 of the 37 cursor states ship no texture at all - only Data\Cursors\*.ani and Art\W3D\*.W3D - and making them renderable is roughly a day. That is only worth spending if the hint logic actually selects those states for a pad seat. If a pad seat's cursorType never leaves ARROW, the art is not the bug and decoding it buys nothing. This probe is what tells the two apart, and it costs nothing to run. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ip no texture The operator, comparing against stock Generals: "there is no proper icons over buildings, it should be this, and over units too waypoint, and over supplies/garrison the three green animated arrow should show ... you can not tell if something works unless you see it works." Retail ships texture art for only 8 of the 37 cursor states - SCCPointer, SCCAttack and SCCRepair unnumbered, plus SCCMove and SCCScroll as numbered frames. The other 27 (Select, EnterFriendly, Waypoint, Dock, SetRallyPoint, ResumeConstruction, CaptureBuilding, ...) exist ONLY as Data\Cursors\*.ani and Art\W3D\*.W3D. Seat 0 is unaffected: it runs RM_WINDOWS and hands the .ani straight to the window manager. A seat cursor is drawn by us through TheDisplay->drawImage, had nothing to draw, and silently fell back to the arrow - so a pad seat showed the same shape for garrison, waypoint, dock, select and invalid alike. That is also why it was hard to confirm any other fix: the cursor is the feedback channel. This is not a new decoder. SDL3CursorManager::initResources already loads every one of those .ani files at startup and IMG_LoadAnimation_IO already decodes them to RGBA; loadANI then threw the surfaces away at IMG_FreeAnimation the moment SDL had built an opaque SDL_Cursor from them. AnimatedCursor now retains a tightly-packed ARGB8888 copy plus the hotspot, and the seat renderer uploads one frame to a TextureClass on first use, wrapping it as an IMAGE_STATUS_RAW_TEXTURE Image the same way W3DRadar builds its per-player radar images. Nothing is read from or written to disk beyond what the engine already loads - the art is the player's own installed game data, decoded in memory - so no retail art is redistributed and the repo stays publishable. Order matters and is deliberate: mapped image, then texture, then .ani, then the arrow. Preferring .ani would take MOVE and SCROLL off their 32x32 DXT art and onto 4bpp indexed frames, undoing 6afadc1. Animation is driven by the .ani's OWN frame count, not CursorInfo::numFrames. Retail Mouse.ini declares no frame count for any cursor - unbounded grep, zero hits - so numFrames is always 1 and using it here would freeze every animated cursor on frame 0, including the three green arrows the operator specifically asked for. Clamped to MAX_2D_CURSOR_ANIM_FRAMES. Single view is unaffected: seat 0 draws its cursor through the OS, not through this renderer. Not yet runtime-verified. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… ever Finding TheSuperHackers#12, deferred since 2026-08-01 and confirmed still live today: a bunker visible in one seat's viewport and absent from the other's, standing right next to it. Same with oil derricks. The scene displacement is one-sided. W3DGhostObject::snapShot takes the REAL render object out of the shared scene - robj->Remove() at :463 - under an ownsScene guard meaning "I am the last local seat to lose sight of this". That part is right: the scene is shared, so one seat fogging a building must not blank it while another seat is looking at it. But every path that puts it back runs through freeSnapShot, and freeSnapShot needs two things that a seat meeting the object for the first time does not have. Its whole body is inside `if (m_parentSnapshots[playerIndex])`, and both callers in PartitionManager gate on `m_shroudednessPrevious[playerIndex] == OBJECTSHROUD_FOGGED`. A seat that has never seen the building has no snapshot, and goes straight from SHROUDED to CLEAR without ever being FOGGED. So neither gate opens, restoreParentObject never runs, and the real object stays out of RenderList - invisible to that seat permanently, at any range. Neutral IMMOBILE structures are the visible case because they are exactly what gets ghosted: PartitionData::getShroudedStatus forces anything neutral and mobile down to SHROUDED instead. Adds GhostObject::restoreIfDisplacedFor(playerIndex) - a no-op on the base, overridden in W3DGhostObject - called from the CLEAR and PARTIAL_CLEAR branches when the FOGGED precondition does NOT hold. It restores only when something is actually displacing the object (m_sceneSnapshotPlayer >= 0) and only for a local seat, then mirrors the restore already inside freeSnapShot: clear every seat's snapshot out of the scene, forget the owner, put the real object back. This is the second and last of the two mechanisms behind TheSuperHackers#12. The first was the draw gate disagreeing with the visibility filter, fixed in 67801c8; that one could not reach this case, because no visibility predicate can help an object that is not in the render list at all. Sim-safe: this only moves render objects in and out of the shared W3D scene. It reads shroud state that PartitionManager has already computed and writes none of it, and GhostObject's xfer chain is untouched. Single view is unchanged: with one seat, ownsScene is true exactly when that seat fogs the object and freeSnapShot's own FOGGED->clear edge always fires on the way back, so the new branch finds m_sceneSnapshotPlayer < 0 and returns immediately. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…rHackers#10 and TheSuperHackers#12 fixed; four verified on a pad Eleven commits. The previous dropoff's single 'decisive' question turned out to be decisive about the cursor RENDERER rather than the hint logic, and three pieces of handoff4's evidence are retracted here: the enemies-vs-own-units report, mousedOver=0, and the coordinate-space hypothesis. Records what is runtime-verified (click-select, placement, cursors) versus merely compiled (six of mirelle's thirteen have still never been run), why TheSuperHackers#11's size half is a considered NO-GO rather than an oversight, and the pad-vs-pad test plan the second controller unlocks. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Reported live, and visible on seat 0 as well as a pad seat: the control bar shows "Building:\n%.0f%%" verbatim while the world-space text over the structure shows the correct "Building: 51%". updateConstructionTextDisplay does format the string correctly. The defect is which window it writes to: winGetWindowFromId(nullptr, descID) is a GLOBAL name lookup, and with more than one ControlBar instance it reaches whichever copy the name resolves to - the newest head-inserted one. Every other bar therefore never had its description text written and kept the placeholder authored in ControlBar.wnd, which is literally the format string. That is why the symptom is an UNFORMATTED string rather than a stale or wrong number: nothing wrote to that window at all. It is also why seat 0 shows it too - seat 0's bar is not the newest instance either. Routed through findBarWindowById, which scopes strictly to this instance. No-op in single view, where the global lookup and the scoped one resolve to the same window. Note for whoever picks this up: this is bug class 1 and it is NOT isolated. An unbounded grep finds 22 remaining winGetWindowFromId(nullptr, ...) lookups under GameClient/GUI/ControlBar/. Each is a latent instance of exactly this, and any of them that writes rather than reads will present the same way - a control put in one bar and never in the others. They should be swept deliberately, with a build between, rather than in one blind pass. Also records that the TheSuperHackers#12 "still invisible" observation from the same session is INCONCLUSIVE: both pads were dropping in and out at the time, so nothing seen on screen can be attributed to the shroud code. Re-test on wired pads before concluding anything. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…artwork pass, two new findings VERIFIED: TheSuperHackers#13 (main-menu resolution change leaves the shell map clean), TheSuperHackers#4's artwork half, and the build-progress text fix. TheSuperHackers#10 comprehensively confirmed - one frame showed three seats drawing three DIFFERENT cursor states at once, covering the plain texture, the .ani decode and the numbered-frame art simultaneously. NEW: TheSuperHackers#11's position half is not fixed despite 7c91c33 - seat 0's tooltip anchors to seat 2's bar. The lookups are correctly scoped; the anchor math applies a RUNNING DELTA against m_tooltipLastOffset instead of an absolute, which cannot be right across bars at different dock offsets. NEW: seat 2 renders terrain lit where its own radar shows it unexplored. The per-view render player is set correctly, so it is the shared destination shroud texture and the draw ordering it depends on - seat 2 is drawn first, so it is exactly the view that would show a later upload. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ckers#3 half passes; splash misposition narrowed to two candidates TheSuperHackers#4 VERIFIED in full: three seats showed three faction schemes, and on seat 0's defeat only seat 0's bar went observer while seats 1 and 2 kept theirs. The empty-looking bar in the first screenshot was simply nothing selected. TheSuperHackers#2/TheSuperHackers#3 half passes. The trigger fires and there is no global input freeze - seats 1 and 2 played on, which was the sharper claim. But the splash is centred on the whole display instead of the seat's viewport, and the operator reports money and team overlays doing the same, so it is broader than the finding. Three hypotheses were tried and all three are refuted in the doc, with the two survivors named: isSplitscreenEnabled() false at creation time, or info.windows empty so the transform loop iterates nothing. Both fall out of one probe. Stopping there rather than guessing a fourth time. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…command another army The generals promotion screen is positioned CORRECTLY per seat while the defeat splash, score panel and pause menu all go full-display. That makes this one job rather than four bugs: the generals screen and the special-power shortcut bar register their layout with the seat's own ControlBar, which is the established mechanism. Port the others onto it instead of hand-rolling scale/offset maths per overlay - showOutcomeSplashForSeat already tried that and is the one that does not work. Likely covers #1 as well. Also records that after seat 0 was defeated its keyboard began commanding player 3's units while seat 2 was still playing that army. Mechanism located: the observer cycle-player paths call rts::changeLocalPlayer(), which reassigns the local player outright, and getCommandActingPlayer() falls back to exactly that. Whether it is pre-existing retail observer behaviour or a splitscreen regression is NOT established - A/B the baseline before fixing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…orm instead of guessing at it again The end-of-match splash is centred on the whole display instead of the seat's viewport. FIVE static hypotheses have now been refuted, all by reading: * seat 0 is not on a legacy path - ScriptActions.cpp:217/220/241/244/263 all call showOutcomeSplashForSeat(0, ...), the same entry seats 1..7 use. * seat 0 has a view - InGameUI.cpp sets s0->m_view = TheTacticalView. * the full-display size guard cannot bail - seat 0's viewport is 960x540 of 1920x1080. * isSplitscreenEnabled() is true - CommandLine.cpp:797 sets m_splitscreenEnabled = TRUE in parseSplitscreenDev, and GameEngine.cpp:608 forwards it to the seat manager. * info.windows is populated - winCreateFromScript ends with `if(info) *info = scriptInfo;` after pushing every parsed root. Every gate that could skip the transform is open, and the transform itself looks correct. That is the point to stop reasoning and measure, so this logs every gate, the view/display geometry, and for each root the before, the values written, and a READ BACK of what the window manager actually stored. If the readback matches what was set, the transform worked and something re-applies authored geometry afterwards - which makes it an ordering problem, not a maths one, and points somewhere entirely different. Instrumentation only, default OFF, GX_SPLASHPROBE=1. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
1 participant
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.
Splitscreen findings from
PatchNotes/— 42 commits on top off72603e6c.Read the verification column before relying on anything here. Six of the thirteen findings have been exercised on real hardware with real gamepads. Several have not been run at all. That distinction is the most important thing in this description, and it is the reason this is opened for review rather than presented as finished.
Status against the thirteen
The root causes, since they generalise
The point pick was answered against seat 0's vision.
RTS3DScene::castRay(testAll=false)testsIs_Really_Visible(), whichVisibility_Checkrewrites for every render object once per view per frame from that view's camera and player.Display::drawViewswalks the view list head to tail whileattachViewprepends, so seat 0's view is drawn last and its answer is the one standing when input is translated. Drag-select was unaffected because the rect branch ofiterateDrawablesInRegioniterates all drawables and never reads the flag — which is exactly why it presented as "drag works, click does not". Fixed by evaluating the predicate live for the picking view; single view takes the original path unchanged.Every no-arg placement accessor forwards to a literal 0. Seven of them. The per-frame ghost updater read
getPendingPlaceSourceObjectID()no-arg, so a pad seat got seat 0's builder —INVALID_ID—BuildAssistantleftplayerIndex = -1, andgetShroudStatusForPlayerreturnsCELLSHROUD_SHROUDEDfor a negative player before it examines a cell, so every position on the map returnedLBC_SHROUDand the placement ghost was red everywhere by construction. SeparatelyPlaceEventTranslatorread through the acting seat but four writes did not, so a pad press armed seat 0's anchor and the click fell through to the command translator as a move order.Per-frame UI work ran only for seat 0.
handleBuildPlacementsresolves throughm_activeSeat, which is only non-zero during message translation — the header says so directly: "Render/HUD code always runs withm_activeSeat == 0". So the placement ghost was never positioned or legality-checked for any other seat.Ghost displacement is one-sided.
snapShotremoves the real render object from the shared scene when the last local seat loses sight of it, but every restore path runs throughfreeSnapShot, which requires that seat to have a snapshot and to have been previously FOGGED. A seat meeting the building for the first time has neither — it goes SHROUDED straight to CLEAR — so nothing restored it and it stayed out of the render list, invisible at any range.Seat cursors had no art. The renderer chose its texture filename from
CursorInfo::numFrames, always 1 because retailMouse.inideclares no frame count for any cursor — butsccmoveandsccscrollship only as numbered frames. Further, 27 of the 37 cursor states ship no texture at all, onlyData\Cursors\*.aniandArt\W3D\*.W3D. Seat 0 is unaffected because it runsRM_WINDOWS. The engine already decodes every one of those.anifiles at startup and then discarded the pixels; they are now retained and uploaded to a texture on first use. No art is copied into the repository — it is decoded from the player's own installed data at runtime.Still open, with what is known
TheSuperHackers#11 size half — not implemented, deliberately. Registering the tooltip layout into the bar's
dockToRectpass would make it re-apply authored geometry every frame, in single view too.populateBuildTooltipLayoutwrites the parent size, then has an earlyreturnwhen the background marker is missing, then writes the position and description-box size. Under registration, a hover taking that early return persists a half-updated authored record permanently. The function needs restructuring to have no mid-way bail before it can safely be registered.TheSuperHackers#11 position half — still broken despite the commit claiming it. Observed live: seat 0's tooltip renders above seat 2's bar. The window lookups are correctly scoped; the anchor math mixes an authored marker coordinate with a docked screen position and applies the correction as a running delta against
m_tooltipLastOffsetrather than as an absolute, which cannot be correct across bars at different dock offsets.Overlay positioning — one job, not four bugs. The end-of-match splash, the money/score panel and the pause menu are all placed for the whole display rather than the seat's viewport. Crucially the generals promotion screen is correct, which means the mechanism already exists and works: it registers its layout with the seat's own
ControlBar(addBarLayoutWindows+redockAfterRootsChanged). The others should be ported onto that path rather than hand-rolling scale/offset maths per overlay —showOutcomeSplashForSeattried that and is the one that does not work. This likely covers #1 as well. A probe is included (GX_SPLASHPROBE=1) because five static hypotheses about the splash were each refuted by reading; the sixth needs measurement.A defeated seat can command another seat's army. After seat 0 was defeated its keyboard began selecting and commanding player 3's units while another seat was actively playing that army. The observer cycle-player paths call
rts::changeLocalPlayer(), which reassigns the local player outright, andgetCommandActingPlayer()falls back to exactly that. Not established whether this is pre-existing observer behaviour or introduced here — it wants an A/B against the baseline before anyone writes a fix.Seat 2 renders terrain lit where its own radar shows it unexplored. The per-view render player is set correctly, so the suspicion is the single shared destination shroud texture and the draw ordering
prepareShroudForViewdepends on. Its own comment admits that fragility.ControlBarglobal window lookups. One was found and fixed live — the build progress readout displayed its own format string, because a globalwinGetWindowFromId(nullptr, ...)reaches the newest bar instance and every other bar kept the placeholder authored inControlBar.wnd. An unbounded grep finds 22 more such lookups underGameClient/GUI/ControlBar/. Each that writes rather than reads is a latent instance of the same bug. They want a deliberate sweep with a build between, not one blind pass.Gamepad reliability over Bluetooth is the biggest practical obstacle to further testing, and is environmental rather than a defect here: a pad whose battery died froze the game for about a minute inside a blocking Windows HID call, and reconnects arrive with a new SDL joystick id so a seat that went
DEVICE_LOSTdoes not automatically recover its device. Wired pads are recommended for any test session.Notes
Generals/is not buildable on this branch and was not made so; only the Zero Hour target is built. Every change is intended to be a no-op with a single seat, and each commit message states why. Several commits also record diagnoses from earlier handoffs that turned out to be wrong — the coordinate-space hypothesis for the pick, the "enemies work but own units don't" report, andmousedOver=0as evidence — because those were confidently written with accurate file:line citations and cost real time before being refuted by measurement.