Headless bot fleet: automation client, movement resilience, and shared client fixes#15
Closed
paleophyte wants to merge 153 commits into
Closed
Headless bot fleet: automation client, movement resilience, and shared client fixes#15paleophyte wants to merge 153 commits into
paleophyte wants to merge 153 commits into
Conversation
…-client # Conflicts: # .gitignore # src/game/chat_handler.cpp # src/game/packet_parsers_tbc.cpp
…ram boarding wowee_headless is compiled with WOWEE_HEADLESS_DEFAULT=1, which unconditionally skips SMSG_UPDATE_OBJECT/SMSG_COMPRESSED_UPDATE_OBJECT/etc, so the client never learns about any nearby entity - other players, NPCs, or GameObjects like the Deeprun Tram. Let WOWEE_HEADLESS=0 override that per-leader (fleet.settings.json fullWorldSimulation) so a leader that needs spatial awareness can opt back in, while the rest of the fleet keeps the lightweight default. Keep a few cosmetic/animation packets (SMSG_EMOTE, spell visuals, obsolete aura packets) always skipped - their handlers assume a fully initialized rendering pipeline the headless harness doesn't have, and enabling them crashed with SIGSEGV. Also adds a board-tram command that walks a leader onto a live Deeprun Tram car and rides it, using /world/entities instead of the old offline DBC-cycle prediction now that entities are actually trackable.
The headless harness never called TransportCallbackHandler (Application- only) or Application::update()'s per-frame M2 boarding check, so it could never register a transport in TransportManager or attach a player to one, regardless of anything else being correct. Now that both pieces are extracted into shared, renderer-independent methods (TransportManager::resolveAndRegisterSpawn, GameHandler:: updateM2TransportBoarding), wire them into HeadlessSession: - setGameObjectSpawnCallback/setTransportMoveCallback register or update a transport in TransportManager on spawn/move, using a small hardcoded isM2 check (Deeprun Tram displayId 3831 / entry 176080-176085, Thunder Bluff lifts) since headless has no renderer to resolve a model asset and decide WMO vs M2 the way EntitySpawner does for the GUI client. - update() calls updateM2TransportBoarding() once per tick using the player's live canonical position. - configureProtocol() loads TransportAnimation.dbc/TaxiPathNode.dbc into the TransportManager, which nothing in headless previously did, so transports registered without this fell back to a stationary path. Verified end to end: Codexvale now registers, animates, and boards a live Deeprun Tram car under wowee_headless (previously impossible).
Boarding correctly set onTransport=true, but nothing kept movementInfo.x/y/z following the tram afterward - the GUI client's per-frame render loop does this itself (locking the player to the deck each frame), which headless has no equivalent of. Without it, /world/self kept reporting the pre-boarding position while the tram moved out from under it, and the disembark distance check would eventually (incorrectly) kick the player off from apparent drift rather than real movement. Feed updateM2TransportBoarding() the composed (transport + fixed offset) position and write it back via setPosition() each tick, unless a movement task is actively driving a walk (e.g. stepping off at the destination), so that explicit goto isn't immediately overwritten back onto the tram. Verified end to end: Codexvale's reported position now tracks smoothly through the whole tunnel while riding.
…tilt, and grab-box fixes
…eadless regression testing
Headless bot fleet: automation client, movement resilience, and shared client fixes
Application::update() tracked "was this player just taxiing" (used to arm the post-landing floor-snap clamp) from the same broad onTaxi flag used for input-freezing, which includes isTaxiActivationPending(). That flag goes true the instant a taxi request is sent, before the server replies - so a request that gets rejected a frame or two later still briefly flips the tracker true, and the moment the rejection clears the pending flag, the clamp arms as if a real flight had just landed. It then snaps the player to whichever floor candidate is closest to their current, never-actually-moved position - at a multi-level WMO (e.g. Booty Bay's upper platform over a lower level), that can easily pick the wrong one and drop the player through the building. Live-reproduced: 0 money, activate a taxi flight, get a correct "Cannot take that flight path" rejection, and still end up teleported down a level with zero actual movement in between. This is the same "activation race" bug class PR Kelsidavis#96's core fix already addressed for flight-motion/mount state in MovementHandler - just a second, separate instance of it in the landing-clamp arm trigger here in application.cpp, which wasn't touched by that fix. Fixed by tracking "actually flying" (isOnTaxiFlight() || isTaxiMountActive(), deliberately excluding isTaxiActivationPending()) separately from the broader onTaxi used for input-freezing, and arming the clamp only off the narrower flag.
…ount processPendingUnloads() drained a fixed maxTileUnloadsPerFrame_=8 tiles per frame call - a count, not a time budget. That cap was itself an earlier fix for a worse bug (an unbounded synchronous unload of ~100 tiles causing a single 1.9s stall), but a fixed count doesn't scale with actual frame cost: streamTiles() discovers newly-out-of-range tiles on a 33ms wall-clock timer, so during a long/fast taxi flight it keeps finding tiles at a roughly constant real-world rate regardless of frame rate, while a count-based drain processes fewer tiles per *second* whenever frame rate drops for any reason. The growing backlog (loadedTiles keeps growing, and streamTiles()'s own per-call scan is O(loadedTiles) with no early exit) then makes frames slower still - a feedback loop, not a leak. Live-reproduced via a long cross-continent taxi flight: terrain-update frame cost climbed from ~50ms early in the flight to 600ms+ later in the same flight. Converted processPendingUnloads() to a time budget (8ms normal, 16ms during taxi streaming) matching the existing taxi-aware budget already used by the mirror-image loading side (processReadyTiles()/ advanceFinalization()), instead of assuming frame rate stays high.
Live-captured the "authoritative early completion" SMSG_DISMOUNT branch for the first time this whole engagement (UNIT_FLAG_TAXI_FLIGHT already cleared server-side, genuinely authoritative) - the PR Kelsidavis#96 dispatch logic fired correctly, but honoring it left the player ~90 yards short of the real destination on a long (27-waypoint) flight, over open water with no WMO/M2 floor to catch it. Root cause, confirmed by reading AzerothCore's actual server source: its flight spline (Movement::MoveSplineInit, SetFly()) is built from the same TaxiPathNode.dbc waypoints at an identical PLAYER_FLIGHT_SPEED 32.0f - matching our own taxiClientSpeed_ exactly, so not a speed mismatch. The mismatch was pacing: our updateClientTaxi() paced by the straight-line chord distance between consecutive waypoints (std::sqrt(segLenSq)) while separately *rendering* a smooth Catmull-Rom curve through those same points for visual quality - two different, inconsistent notions of "how long is this path" in the same code path. A smooth curve through a series of waypoints is essentially always shorter than the straight-line polyline connecting the same points (it cuts corners at turns), more so with more waypoints. At identical speed, our longer straight-line-paced flight necessarily took longer than the server's shorter-curve-paced one - unnoticeable on a short 2-3 waypoint hop, ~90 yards over a long multi-waypoint one. Fixed: startClientTaxiPath() now precomputes each segment's real curve arc length (taxiClientSegmentArcLengths_, numerically sampled - no closed-form solution exists for a Catmull-Rom arc length) instead of relying on the chord. updateClientTaxi()'s pacing loop uses that arc length instead of the chord distance. Also extracted the previously duplicated inline Catmull-Rom position formula (was written out twice) into a shared evalTaxiCatmullRom() helper, used both for the new arc-length sampling and the existing per-frame position evaluation.
Live retest confirmed the arc-length pacing fix and the 300-yard authoritative-completion safety net both worked correctly, but the character still landed ~139 yards short of the real Stormwind flight point (in the lake again). The safety net was snapping to taxiClientPath_.back() - the last entry of the TaxiPathNode.dbc waypoint trace for the path - but that's a different table from TaxiNodes.dbc (which defines each node's own registered position, the one every GM teleport and .gps cross-check validated as accurate all session). A taxi path's waypoint trace apparently doesn't reliably terminate exactly on the destination node's actual coordinate - live-confirmed a 139-yard gap between the two tables for this path. This almost certainly affected every prior "natural completion" landing too, just too small to notice on the short hops used in all earlier testing. Added taxiDestNodeId_ (set in activateTaxi(), cleared on landing and on rejected activation) so finishClientTaxiFlight() can resolve taxiNodes_[taxiDestNodeId_]'s own position as the landing target for both the natural-completion snap and the authoritative-completion safety net, falling back to taxiClientPath_.back() only if the destination node can't be resolved.
Debug retest showed the TaxiNodes.dbc landing fix was actually working correctly - "Taxi landing: snapped to final waypoint (489.7, -8840.56, 109.61)", the precise, .gps-validated Stormwind coordinate - but the character still visually landed in the lake at the same wrong spot as every prior attempt. Root cause: finishClientTaxiFlight() corrects movementInfo and the entity-manager position, but Application::update()'s own per-frame render-position sync only pulls game state into renderer->getCharacterPosition() while onTaxi is true. finishClientTaxiFlight() clears onTaxiFlight_/taxiMountActive_ before returning, so by the time that sync runs later in the same frame, onTaxi has already gone false and the sync block is skipped - the renderer's position becomes physics/input-authoritative from that point on and never re-pulls from game state. The landing clamp reads renderer->getCharacterPosition() directly, so it kept computing from the stale, never-corrected render position no matter how correct movementInfo now was. The fix was real, it just never reached the one place that mattered for the actual visible landing spot. Added GameHandler::TaxiLandingPositionCallback, mirroring the existing TaxiOrientationCallback pattern already used for mount rotation during flight. Registered in TransportCallbackHandler's constructor (which already has direct renderer access, unlike MovementHandler) to write straight to renderer->getCharacterPosition() plus the camera follow target, matching what the normal onTaxi sync branch does. finishClientTaxiFlight() now invokes it alongside the existing movementInfo/entity updates whenever it snaps to a landing position.
This machine has no use for running wowee - the user pulls the repo to their Mac or Windows laptop to actually test the GUI client, and wowee's compile time is significant. Default to building only wowee_headless locally to verify shared-code changes compile; rely on CI (which already builds wowee for every PR) or the user's own Mac/Windows build for wowee-side verification. Ask for confirmation first if something seems to specifically require a local wowee build.
…ch base origin/master now permanently carries the headless-bot-fleet content (PR #4 merged it in directly), so it can no longer be treated as a clean upstream mirror. Shared-code fix branches must now be cut from upstream/master instead. Also brings over the wowee_headless-only local build guidance.
Application::update() tracked "was this player just taxiing" (used to arm the post-landing floor-snap clamp) from the same broad onTaxi flag used for input-freezing, which includes isTaxiActivationPending(). That flag goes true the instant a taxi request is sent, before the server replies - so a request that gets rejected a frame or two later still briefly flips the tracker true, and the moment the rejection clears the pending flag, the clamp arms as if a real flight had just landed. It then snaps the player to whichever floor candidate is closest to their current, never-actually-moved position - at a multi-level WMO (e.g. Booty Bay's upper platform over a lower level), that can easily pick the wrong one and drop the player through the building. Live-reproduced: 0 money, activate a taxi flight, get a correct "Cannot take that flight path" rejection, and still end up teleported down a level with zero actual movement in between. This is the same "activation race" bug class PR Kelsidavis#96's core fix already addressed for flight-motion/mount state in MovementHandler - just a second, separate instance of it in the landing-clamp arm trigger here in application.cpp, which wasn't touched by that fix. Fixed by tracking "actually flying" (isOnTaxiFlight() || isTaxiMountActive(), deliberately excluding isTaxiActivationPending()) separately from the broader onTaxi used for input-freezing, and arming the clamp only off the narrower flag.
…nup waits Landing a taxi flight snaps the streaming radius down immediately (setLoadRadius/setUnloadRadius), which could queue ~100 distant tiles for unload all at once. unloadTile() ran synchronously and unbounded in that same frame - live-confirmed via a real flight-landing log showing "Unloaded 103 distant tiles" immediately followed by "SLOW terrainManager->update: 1943.71ms". Queue unloads instead and drain a bounded batch per frame (processPendingUnloads()), mirroring the existing processReadyTiles() pattern used for loads. Also time the vkDeviceWaitIdle() call in WMORenderer/M2Renderer's periodic (every 5s) cleanupUnusedModels() - a full GPU pipeline stall that's a suspected but not yet confirmed contributor to the remaining multi-second freeze observed after that same landing. The next repro with this logging in place will confirm or rule it out.
…ount processPendingUnloads() drained a fixed maxTileUnloadsPerFrame_=8 tiles per frame call - a count, not a time budget. That cap was itself an earlier fix for a worse bug (an unbounded synchronous unload of ~100 tiles causing a single 1.9s stall), but a fixed count doesn't scale with actual frame cost: streamTiles() discovers newly-out-of-range tiles on a 33ms wall-clock timer, so during a long/fast taxi flight it keeps finding tiles at a roughly constant real-world rate regardless of frame rate, while a count-based drain processes fewer tiles per *second* whenever frame rate drops for any reason. The growing backlog (loadedTiles keeps growing, and streamTiles()'s own per-call scan is O(loadedTiles) with no early exit) then makes frames slower still - a feedback loop, not a leak. Live-reproduced via a long cross-continent taxi flight: terrain-update frame cost climbed from ~50ms early in the flight to 600ms+ later in the same flight. Converted processPendingUnloads() to a time budget (8ms normal, 16ms during taxi streaming) matching the existing taxi-aware budget already used by the mirror-image loading side (processReadyTiles()/ advanceFinalization()), instead of assuming frame rate stays high.
Live-captured the "authoritative early completion" SMSG_DISMOUNT branch for the first time this whole engagement (UNIT_FLAG_TAXI_FLIGHT already cleared server-side, genuinely authoritative) - the PR Kelsidavis#96 dispatch logic fired correctly, but honoring it left the player ~90 yards short of the real destination on a long (27-waypoint) flight, over open water with no WMO/M2 floor to catch it. Root cause, confirmed by reading AzerothCore's actual server source: its flight spline (Movement::MoveSplineInit, SetFly()) is built from the same TaxiPathNode.dbc waypoints at an identical PLAYER_FLIGHT_SPEED 32.0f - matching our own taxiClientSpeed_ exactly, so not a speed mismatch. The mismatch was pacing: our updateClientTaxi() paced by the straight-line chord distance between consecutive waypoints (std::sqrt(segLenSq)) while separately *rendering* a smooth Catmull-Rom curve through those same points for visual quality - two different, inconsistent notions of "how long is this path" in the same code path. A smooth curve through a series of waypoints is essentially always shorter than the straight-line polyline connecting the same points (it cuts corners at turns), more so with more waypoints. At identical speed, our longer straight-line-paced flight necessarily took longer than the server's shorter-curve-paced one - unnoticeable on a short 2-3 waypoint hop, ~90 yards over a long multi-waypoint one. Fixed: startClientTaxiPath() now precomputes each segment's real curve arc length (taxiClientSegmentArcLengths_, numerically sampled - no closed-form solution exists for a Catmull-Rom arc length) instead of relying on the chord. updateClientTaxi()'s pacing loop uses that arc length instead of the chord distance. Also extracted the previously duplicated inline Catmull-Rom position formula (was written out twice) into a shared evalTaxiCatmullRom() helper, used both for the new arc-length sampling and the existing per-frame position evaluation.
Live retest confirmed the arc-length pacing fix and the 300-yard authoritative-completion safety net both worked correctly, but the character still landed ~139 yards short of the real Stormwind flight point (in the lake again). The safety net was snapping to taxiClientPath_.back() - the last entry of the TaxiPathNode.dbc waypoint trace for the path - but that's a different table from TaxiNodes.dbc (which defines each node's own registered position, the one every GM teleport and .gps cross-check validated as accurate all session). A taxi path's waypoint trace apparently doesn't reliably terminate exactly on the destination node's actual coordinate - live-confirmed a 139-yard gap between the two tables for this path. This almost certainly affected every prior "natural completion" landing too, just too small to notice on the short hops used in all earlier testing. Added taxiDestNodeId_ (set in activateTaxi(), cleared on landing and on rejected activation) so finishClientTaxiFlight() can resolve taxiNodes_[taxiDestNodeId_]'s own position as the landing target for both the natural-completion snap and the authoritative-completion safety net, falling back to taxiClientPath_.back() only if the destination node can't be resolved.
Debug retest showed the TaxiNodes.dbc landing fix was actually working correctly - "Taxi landing: snapped to final waypoint (489.7, -8840.56, 109.61)", the precise, .gps-validated Stormwind coordinate - but the character still visually landed in the lake at the same wrong spot as every prior attempt. Root cause: finishClientTaxiFlight() corrects movementInfo and the entity-manager position, but Application::update()'s own per-frame render-position sync only pulls game state into renderer->getCharacterPosition() while onTaxi is true. finishClientTaxiFlight() clears onTaxiFlight_/taxiMountActive_ before returning, so by the time that sync runs later in the same frame, onTaxi has already gone false and the sync block is skipped - the renderer's position becomes physics/input-authoritative from that point on and never re-pulls from game state. The landing clamp reads renderer->getCharacterPosition() directly, so it kept computing from the stale, never-corrected render position no matter how correct movementInfo now was. The fix was real, it just never reached the one place that mattered for the actual visible landing spot. Added GameHandler::TaxiLandingPositionCallback, mirroring the existing TaxiOrientationCallback pattern already used for mount rotation during flight. Registered in TransportCallbackHandler's constructor (which already has direct renderer access, unlike MovementHandler) to write straight to renderer->getCharacterPosition() plus the camera follow target, matching what the normal onTaxi sync branch does. finishClientTaxiFlight() now invokes it alongside the existing movementInfo/entity updates whenever it snaps to a landing position.
protocolVersion 3 was rejected during auth against a VMangos classic server - the working live-tested value is 8, matching tbc/turtle's existing protocol selection.
protocolVersion 3 was rejected during auth against a VMangos classic server - the working live-tested value is 8, matching tbc/turtle's existing protocol selection.
upstream picked up its own taxi dismount-timing fixes (a8a650e, 066eae1) and render-performance work (436d073, bb6e445, dd5bb5e) that touch the same files this branch's taxi/terrain fixes do. The taxi fixes are complementary, not competing: upstream's isNearTaxiDestination()/deferServerTaxiCompletion() decide *when* to trust an early completion signal (gated by proximity to the path's own last waypoint); this branch's finishClientTaxiFlight() still decides *where* to land (the accurate TaxiNodes.dbc position, not the imprecise path waypoint) regardless of which caller triggers it. Verified both authoritative-completion call sites and the deferred consumption path all still funnel through the DBC-accurate snap. Only one textual conflict (two independent cleanup lines added at the same point in finishClientTaxiFlight()) - kept both. Terrain/render files auto-merged cleanly.
Owner
Author
|
Closing this PR now that CI validated the isolated headless/bot branch. Per our branch policy, this work stays in the fork branch and is not merged into master or proposed upstream. |
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.
Summary
Reopens the headless bot fleet work after restoring
masterto upstream parity.Validation
Not rerun during master-recovery bookkeeping; branch content is unchanged from the previously merged PR #4 except for the existing branch history that had synced
masterafterward.