fix(client): the flag a spawn drew, and the ground it buried the body in (X-89, X-90) - #263
Merged
Merged
Conversation
Three repairs from one investigation, all on the path between "connected" and
"standing where the server says".
PredictionReconciler returned Stale on a repeated acknowledgement before it
tested distance. A repeated ack says nothing new about the ticks this client
sent -- and nothing about where the SERVER has put the body either. The server
moves a body for reasons no input drives: the spawn placement, a respawn,
EnforceWireVolume's teleport. None of those advance lastProcessedInputTick, and
input travels unreliable on ChannelId.InputSequenced, so re-acking the same tick
is what standing still LOOKS like. The body therefore kept the position
GameManager.StartGame instantiates the prefab at -- (0, 1000, 0), the spot X-17
names -- and free-fell from there into a corner of the map while the server's
copy stood correctly at the base. ResyncDistanceMetres was written for exactly
this ~975 m case; it was simply unreachable behind the early return. The
adoption is position-only and only past that radius (4 x RunSpeed, ~26 m):
inside it a stale ack really does mean nothing to do, and adopting authority
there would throw away legitimate unacknowledged motion. _lastAckedTick is left
alone, because this tick is NOT newer -- claiming it would make the next
genuinely new ack read as stale.
The first deploy could also be owed forever. The loadout screen closes by three
routes and only FpsActorController.DeployFromLoadout posts the deploy edge; the
"Loadout" axis toggling it shut and EnterDeployedView dismissing it do not, so a
player who closed it with the key had no remaining path to C_SPAWN_REQUEST at
all. ILocalPlayerRig.IsLoadoutOpen (LoadoutUi.IsOpen(), false where there is no
UI) plus a five-second grace lets the client send the request itself once it is
clear no button is coming, without stepping on a player still choosing.
AdoptAlreadyAliveBody retires the debt off the snapshot's IsAlive bit, which
replaces the announce path's message.Health > 0 test: Health is 0 on a parked
body, so that test could only ever answer for a life already in progress.
[predict] is the instrument the next commit's measurement is taken with: one
line per second from ClientPredictionStage carrying rig / state / server
positions, the error between them, the agreed / corrected / resync / stale
counters, the actor count and whether the controller is on.
IRONFRONT_LOG_PREDICT=0 silences it; it is on by default because every fall
investigated so far has needed it and no run has been cheap to repeat.
Verified: 2283 of 2285 dotnet tests across 8 projects, and 134/134 Unity
EditMode. The two failures are AssetWiringGateTests.ThrowReleaseDelay_* and
predate this diff -- the detector formats its numbers in the machine's vi-VN
culture ("0,6000000") while the assertions expect invariant-culture decimals
("0.6000000"). Neither the test nor the detector it exercises is touched here.
Closes X-89 and X-90. Both are one line of placement arithmetic apiece, and together they are the whole of the human report: map trong khong co gi ngoai dia hinh va player / spawn o goc, nhay ra ngoai la rot / killed by the world. ActorManagerSpawnPoints.IsEligible read point.owner < 0 as "any team may spawn here". On a CapturePoint -- and every scene spawn point IS one, CapturePoint : SpawnPoint being the only subclass -- owner == -1 means NEUTRAL, held by nobody. ActorManager.RandomSpawnPointForTeam has always required owner == team, which is why the AI wave spawns at bases and the two paths disagreed about the same array. Measured in the 2026-09-04 playtest: actor 33 (team 0) at point 4, actor 34 (team 1) at point 1, with the client reporting points 0, 1, 2 and 4 as owner -1. Both players were dropped on contested flags, neither at the base its own side's bots spawned at, and a player alone on a flag is outside InterestManager's 500 m CullRadius from every one of those bots -- so the bots are culled out of the snapshot and the map is terrain and one body. IsEligible is now owner == team, and a team that owns nothing draws among the neutral points instead of being left at the prefab park to fall from ~975 m. PinnedSpawnPointDirectory's remark and its refusal text are corrected rather than re-tuned: under the fixed rule EVERY shipping index starves somebody, and RefuseIfAnyTeamIsStarved catches all of them unchanged because it asks IsEligible instead of counting. X-90 is what X-89's own verification run then exposed, with the placement finally on the right flag. SpawnPoint.GetSpawnPosition ends in GroundSnap.TrySnap, which returns hit.point -- the SURFACE. The player capsule is authored height 1.8, center.y 0, so it runs from 0.9 m below the transform to 0.9 m above it, and MoveToSpawnPoint teleported the body to that surface point verbatim: half the capsule inside the ground. FpsActorController.SpawnAt has always added + Vector3.up * (characterController.height / 2f); this path is the netcode's stand-in for that call and was missing its one arithmetic step, so it is a port omission and not a design difference. AiActorController.SpawnAt never writes the transform, which is why no bot was ever affected and the report said so. Both symptoms of the burial are in artifacts/lane-b/predict-01. On thick ground the client's controller de-penetrated upward to ground + 0.9 + the 0.08 skin while the server held the surface, so err stood at 0.98 m across 250 consecutive ticks -- 197 corrections in ten seconds, each re-applied and re-undone. PositionToleranceMetres is 0.25, so no tolerance could ever have absorbed it. And a body whose CENTRE starts under the surface has nothing to resolve against: actor 34, on a snap that SUCCEEDED with no X-81 warning in the run, went from y = 101.27 to y = -104.71 with probe=MISS, ctrlEnabled=True, and the server's own authoritative Y fell with it -- so the client was faithfully rendering a falling server body, which is why every client-side fix attempted before this failed. StandingBodyPosition lifts the drawn ground point by MovementCore.HeightFor(crouching: false) * 0.5f before BOTH the teleport and the Session.State write, so authority and rig agree from the first tick, plus State.IsCrouching = false so the stance ApplyStanceHeight builds is the one the lift was measured against (a gameplay spawn calls ForceEndCrouch for the same reason). The height is read from MovementCore rather than the live controller so the two cannot drift and a test can check it with no scene loaded, and the lift is deliberately NOT pushed down into GroundSnap or SpawnPoint: those answer "where is the ground", a question SpawnAt also asks and already offsets itself. Verified end-to-end on artifacts/lane-b/lift-01, same seed and same two spawn points as predict-01 so the runs compare line for line. actor 33 on the same ground y = 8.82 was placed at 9.72, settled at 9.81 against a server holding 9.83: err 0,02 m, Agreed, 0 corrections where the old build logged 197. actor 34 -- the body that fell to -104 m -- was placed at 104.39 and was still standing at 105.2 at the end of the run. Zero [fall] lines in all four logs, no [spawn] warning, no wire-volume kill. Also 134/134 EditMode, three of those tests new and pinning the lift against the measured 9.81; 2283 of 2285 dotnet tests, the two failures being the pre-existing vi-VN culture assertions in AssetWiringGateTests. No mutation pass was run and the ledger says so: err = 0,98 m for 250 ticks and a -104 m fall are recorded measurements, so a RED run would only reproduce by construction what a real one already showed. X-82 stays OPEN -- actor 35 did not leave its ground in lift-01, which is a data point for the two being the same defect, but one clean smoke run is not the condition X-82 was filed against.
…variable build-player.ps1 threw unless UNITY_PATH was already set, which made the first run on any new machine a documentation lookup. It now reads m_EditorVersion out of Ironfront_Reborn/ProjectSettings/ProjectVersion.txt and looks for that exact version, so the version it builds with is the version the project declares rather than whatever a stale variable points at. The search covers the Hub's default Editor directory, a plain install, and the secondary install path the Hub records in %APPDATA%/UnityHub/secondaryInstallPath.json -- which is the case that matters here, because a machine with a small C: keeps its Editors elsewhere. An explicitly supplied -UnityPath still wins when it exists, but no longer prevents discovery when it does not, and the failure now lists every path it looked in. .gitignore excludes .claude/, the agent session directory: local scratch, nothing another checkout can use. Both changes were already in the working tree when this session started; they are committed here rather than left behind, unmodified.
Both findings the client-wiring gate raised against this branch are stale RULES, not live defects, and each is corrected in the way the rule itself asks for. G12 hardcoded OnSpawnActor as the handler for "your body is deployed". That is no longer a question S_SPAWN_ACTOR can answer: since a JOIN stopped placing the body, that message reaches every client on INTEREST alone -- "you now know this actor exists", not "you are deployed" -- and the Health > 0 test it used to carry fired on every join, because a parked slot is Instantiate'd from the prefab and keeps health's field initializer of 100. The alive/dead bit is not in S_SPAWN_ACTOR at all; SpawnFlags carries IsBot and IsLocalPlayer and nothing else. The rule now names AdoptAlreadyAliveBody, which reads the snapshot's own StateFlags bit -- the server's view of a body it has actually placed. The rule's own remark says "if the handler was renamed, move this rule with it rather than deleting it", and that is what this is: the pair is still a pair, first placement in one handler and dead->alive in OnRespawned, and losing either still leaves a player looking at the menu for a whole life. MUTATION-TESTED rather than assumed green. Deleting the EnterDeployedView() call from AdoptAlreadyAliveBody turns the gate RED naming that handler and that line; restoring it turns it green. A rule that was never seen failing is not a rule. G4 is the sixth exemption of a shape the list already carries five of. TrackDeployFallbackGrace reads NetClientBindings.LocalPlayer.IsLoadoutOpen to decide whether a Deploy button is still on screen before the client sends its own unattended first request. Verified rather than asserted: line 440, inside Update() at line 366, is the helper's ONLY caller -- the same Update() that already reaches FirePressed, ScriptedRespawnPressed, ApplyLocalTeam, RequestRespawn and LoadoutDeployPressed. There is no actor id in scope for an IsLocalActor guard to be about, and the loadout screen it asks about is this client's own. The entry carries the same instruction as its five neighbours: if a per-actor caller ever reaches this helper, delete the entry and guard the read rather than widening it. Verified: client-wiring gate clean across 485 files, and 29 of 29 ClientWiringGateTests including PerActorGuardExemptions_HasNoStaleEntries, which re-checks that the new entry suppresses a real touch rather than nothing.
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
2 participants
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.
Closes X-89 and X-90. Two lines of placement arithmetic, and together they are the
whole of the human report: the map is empty terrain and one body, and a player
spawned in a corner walks off the edge and is killed by the world.
What was wrong
X-89 — the flag a spawn drew.
ActorManagerSpawnPoints.IsEligiblereadpoint.owner < 0as "any team may spawn here". On aCapturePoint— and everyscene spawn point IS one —
owner == -1means NEUTRAL, held by nobody.ActorManager.RandomSpawnPointForTeamhas always requiredowner == team, whichis why the AI wave spawns at bases and the two paths disagreed about the same
array. A player alone on a contested flag sits outside
InterestManager's 500 mcull radius from every one of their own bots, so the bots are culled out of the
snapshot and the map looks empty.
X-90 — the ground it buried the body in.
SpawnPoint.GetSpawnPositionends inGroundSnap.TrySnap, which returnshit.point— the SURFACE. The player capsuleis authored height 1.8,
center.y0, so it runs from 0.9 m below the transform to0.9 m above it, and
MoveToSpawnPointteleported the body to that surface pointverbatim: half the capsule inside the ground.
FpsActorController.SpawnAthasalways added
+ Vector3.up * (height / 2f); this path is the netcode's stand-infor that call and was missing its one arithmetic step. It is a port omission, not
a design difference — and
AiActorController.SpawnAtnever writes the transform,which is why no bot was ever affected.
Measured, not asserted
Verified end-to-end on
artifacts/lane-b/lift-01againstpredict-01, same seedand same two spawn points so the runs compare line for line:
errwhile standing[fall]linesPositionToleranceMetresis 0.25, so no tolerance could ever have absorbed the0,98 m — the disagreement was structural, not transient.
This is server-side, and that matters for deployment
Both defects are in
Ironfront.Net.Unity.Server— the fix ships inIronfront_Data/Managed/Ironfront.Net.Unity.Server.dll, notAssembly-CSharp.dll. A host machine has to run a build cut after this merge;copying a single DLL is not enough. The client is uninvolved in both defects,
which is why every client-side repair attempted before this one failed.
Scope left open
X-82 stays OPEN. Actor 35 did not leave its ground in
lift-01, which is a datapoint for the two being the same defect, but one clean smoke run is not the
condition X-82 was filed against — a body that leaves the ground AFTER standing
still is still unexplained.
Verification
tests are new and pin the lift against the measured 9.81).
tools/check-commit-scope.ps1: all three subjects conform.