fix(streaming): make a demoted landblock render-ready like a published one (#280 D-1)
Both #280 review lenses returned FAIL on the same defect, and both were
right. IsRenderNeighborhoodResident's widened outer arm requires
IsRenderReady out to FarRadius, justified by "a Far-tier landblock
registers with an empty mesh set and is therefore render-ready." That held
only for a landblock that ARRIVED as Far. The second, equally first-class
way to be Far tier is a Near->Far DEMOTE:
DemoteLandblock -> EnqueueNearLayerRetirement
-> LandblockRetirementStage.MeshReferences
-> GpuWorldState.ReleaseLandblockMeshReferences
-> LandblockSpawnAdapter.OnLandblockUnloaded => WantsLoaded = false
while DetachNearLayer deliberately keeps the landblock loaded, terrain-mesh
resident, terrain-collision resident and DRAWN. Nothing re-publishes an
already-loaded landblock, so the demoted member satisfied NEITHER arm of
the gate, permanently: wormhole tunnel plus centered "In Portal Space -
Please Wait..." forever, no recovery short of relog.
Reachable by ordinary play. Two consecutive recalls to the same landblock
with walking in between makes ChangesStreamingCenter false, so there is no
origin recenter and the region recentres through the ordinary demote diff.
Also reachable via a mid-hold quality-preset drop -- ironically the exact
scenario ReconcileDestinationReservationRadius was added to support. The
pre-#280 radius-1 gate never touched that band, because nothing inside the
Near ring can demote.
FIX SHAPE. Make the two routes genuinely equivalent rather than teaching
the predicate to tolerate the difference. ReleaseLandblockMeshReferences
becomes "reconcile the registration to the post-retirement tier": after the
release converges, if the landblock is still loaded AND still Far tier,
re-assert the empty registration -- the identical OnLandblockLoaded(lb,
empty) a PublicationKind.Far activation makes. It is empty by construction:
DetachNearLayer retains only live server projections, which the adapter's
atlas-tier filter skips. A full retirement is unaffected (DetachLandblock
clears both _loaded and _tierByLandblock), and a throwing release still
retries because the re-assert is only reached after the adapter converged.
The alternative -- "|| (IsFarTier && IsLoaded)" at the gate -- was
rejected: it fixes one caller while leaving IsRenderReady meaning two
different things, which is precisely how this defect arose. After this
change the predicate reads "drawable at its current tier" for every caller,
with no knowledge of how the landblock got there.
WHY THE TESTS MISSED IT, fixed here too:
- Proof obligation P2 was discharged against RESIDENCY (the FarRadius+2
eviction threshold) rather than against IsRenderReady, the gate's actual
atom. The contract now carries the correction and the restated
obligation: no transition may REVOKE IsRenderReady from a landblock that
stays inside FarRadius.
- WorldRevealDerivedWindowIntegrationTests advertised itself as end-to-end
against the real GpuWorldState but constructed it with no spawn adapter,
so its IsRenderReady degenerated to IsLoaded via the "?? true". The
single most load-bearing predicate in the change was stubbed out by a
null in the test named after it -- the same shape as C5b's D3 and #276's
three settler tests. Every fixture in that file now owns a real
LandblockSpawnAdapter.
- The P1 test's comment described its subject as "a Near-shaped completion
the streaming window has since DEMOTED to Far". It is not; it is a fresh
PublishAsFar, the case that does hold. Corrected, since a future reader
would have taken it as demote coverage.
Four new regression tests, all driving the real GpuWorldState +
LandblockSpawnAdapter + LandblockPresentationPipeline through an actual
demote, and all sabotage-verified in both directions (fail with the
production change reverted, pass with it):
NearToFarDemote_LeavesTheLandblockRenderReadyThroughTheRealPipeline
NearToFarDemote_LeavesTheLandblockRenderReadyUnderBudgetedRetirement
TieredWindow_StaysResidentAfterAnOuterRingDemote
OutdoorReveal_SurvivesAnOuterRingDemoteDuringTheHold
The budgeted variant exists because production composes
LandblockRetirementCoordinator.CreateBudgeted, whose MeshReferences stage
is a separate call site from the legacy pipeline's.
SECONDARY, same commit:
- R-1: ACDREAM_PROBE_REVEAL_RADIUS=0 was parser-accepted and
Runtime-rejected -- it yields far = 0 for an outdoor destination, which
fails invalid-readiness-shape on every acknowledgement, hanging the very
A/B route the probe exists to measure. Parser floor raised to 1, with a
7-case table test.
- R-2: the composite-warmup TRIGGER had silently moved onto the far
window's critical path. Pre-#280 the gate and the composite domain were
the same radius-1 square; #280 widened the gate without widening the
domain, so every composite upload serialised behind the last outer-ring
landblock for no readiness benefit. Warmup now starts once the NEAR
sub-window is published -- trigger scope == domain scope, as before. The
reveal gate is untouched: Evaluate still requires the full window AND
composite readiness.
- AP-150 filed: acdream's RetailWaitCueDelay = 5 s arming is NOT retail's
trigger, and #280's commit message got this wrong on both clauses. Retail
emits the notice unconditionally per tunnel rotation segment, in the else
arm of the segment-expiry test at 0x004D6FCD; segment duration is
RandDouble(0.6, 1.8) s, byte-decoded at 0x004D6FE6. The 5.0 constant at
VA 0x007991B0 is CellManager::CheckPrefetchStatus's prefetch RETRY
cadence and has nothing to do with the cue. acdream's own 0.6/1.8 segment
constants already match retail exactly; only the arming is wrong.
Adopting retail's unconditional emit is filed as #329 rather than folded
in here -- it is a user-visible presentation change and wants the user's
eyes.
- AP-151 filed: the gate is materially STRICTER than retail on the
mesh-build/GPU-upload axis. Retail's LScape::PreFetchCells blocks on DAT
RESIDENCY only -- no geometry construction, no upload; that work is lazy
at draw. acdream requires a DAT read, terrain mesh build, render-thread
upload, spatial commit, collision admission and spawn-adapter activation
per member of a 625-member window, metered at MaxCompletionsPerFrame.
Nothing bounds the hold. This is the OPPOSITE asymmetry from AP-149; both
are live at once, on different axes.
- AD-2's amendment stated the false Far-tier readiness assumption verbatim;
corrected, along with the same error in
claude-memory/reference_two_tier_streaming.md, which now carries an
explicit DO-NOT-RETRY on the special-case-the-predicate shape.
- AP-115 scope-noted (it covers the cue's presentation, not its arming).
- #326's SmartBox::set_mid_radius citation corrected: the entry is
0x00453180; 0x004531D0 is the mid-function re-arm branch.
Blast radius: GpuWorldState, LandblockSpawnAdapter,
WorldRevealReadinessBarrier and StreamingDiagnostics are all App-internal;
AcDream.Headless and AcDream.Runtime reference none of them outside
comments. Headless tests run green as part of the gate below, per C5b's
lesson about surveys that skip the no-window host.
Gates: Release build 0 errors, 18 pre-existing xUnit analyzer warnings.
Complete suite "dotnet test AcDream.slnx -c Release -m:1" with
ACDREAM_PAK_PATH set: 11,192 passed / 4 skipped / 0 failed, from a clean
rebuild (a prior session's deleted probe file had been compiled into a
stale test DLL). Baseline at fafc0b65 was 11,179 / 4 / 0; the +13 delta
reconciles exactly to this commit's additions -- 3 readiness tests, 1
integration test, 7 parser table cases, 2 warmup-trigger tests. None of the
known flakes #302/#308/#321 surfaced, and none is conflated with the
finding above.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
parent
fafc0b65d9
commit
73cdb95c7b
13 changed files with 1456 additions and 38 deletions
|
|
@ -1439,6 +1439,9 @@ via `PlayerMovementController.ApplyServerRunRate`) or from
|
|||
gate to landblock radius N instead of the derived streaming window, so the
|
||||
same binary can run a route once with the pre-#280 behaviour (`=1`) and once
|
||||
without. Not a user setting; not surfaced in Settings; not persisted.
|
||||
Values below 1 are rejected by the parser: an outdoor acknowledgement with
|
||||
`RequiredRenderRadius == 0` fails Runtime's `invalid-readiness-shape`
|
||||
invariant, so `=0` would hang the route it is meant to measure.
|
||||
- `ACDREAM_NO_AUDIO=1` — suppress OpenAL init for headless / driver-
|
||||
broken setups.
|
||||
- `ACDREAM_REMOTE_VEL_DIAG=1` — dump per-tick / per-UM remote motion
|
||||
|
|
|
|||
|
|
@ -24,6 +24,39 @@ What does NOT go here:
|
|||
- Every session: scan OPEN issues at start; promote/close anything we touched during the session before ending.
|
||||
- Promoting to a Phase: mark as `DONE (promoted to Phase X)` + commit SHA where the Phase entry landed.
|
||||
|
||||
## #329 — The portal wait cue arms five seconds late; retail emits it per tunnel rotation segment, unconditionally
|
||||
|
||||
**Status:** OPEN
|
||||
**Severity:** LOW (cosmetic, but it is a retail divergence on every single
|
||||
portal, in both directions)
|
||||
**Filed:** 2026-08-06, #280 retail-conformance review, finding F2.
|
||||
**Register row:** AP-150.
|
||||
|
||||
acdream suppresses `"In Portal Space - Please Wait..."` until the hold has run
|
||||
five seconds (`RuntimeWorldTransitState.RetailWaitCueDelay`), then re-emits it
|
||||
per rotation segment only while `_waitCueVisible`
|
||||
(`PortalTunnelPresentation.TickRotation`).
|
||||
|
||||
Retail has no threshold. `gmSmartBoxUI::UseTime` @0x004D6E30 emits
|
||||
`ECM_UI::SendNotice_DisplayStringInfo(0x1a, …)` in the `else` arm of the
|
||||
rotation-segment-expiry test at 0x004D6FCD — i.e. every time a segment expires,
|
||||
whether or not `CellManager::blocking_for_cells` is set. Segment duration is
|
||||
`RandDouble(0.6, 1.8)` s, byte-decoded at 0x004D6FE6. acdream's own
|
||||
`RotationDurationMin`/`Max` already match retail exactly, so only the arming is
|
||||
wrong.
|
||||
|
||||
The unrelated 5.0 s constant at VA 0x007991B0 is
|
||||
`CellManager::CheckPrefetchStatus`'s retry cadence, and was mis-attributed to
|
||||
the cue by #280's commit message (retracted in the contract doc).
|
||||
|
||||
**Consequence:** every portal shorter than 5 s shows a silent tunnel where
|
||||
retail shows the notice; every portal longer than 5 s shows it 3.2-4.4 s late.
|
||||
|
||||
**Fix shape:** delete the delay and emit on the same segment boundary the
|
||||
rotation already computes. One-line arming change, but it is a user-visible
|
||||
presentation change and wants the user's eyes before it lands — do not fold it
|
||||
into an unrelated commit.
|
||||
|
||||
## #325 — Gate A's teleport test is narrower than retail's: a ForcePosition carrying a NEWER teleport stamp is misrouted into a full Apply
|
||||
|
||||
**Status:** OPEN
|
||||
|
|
@ -1719,7 +1752,7 @@ it. Do #297 FIRST — #298 depends on it.
|
|||
(`Render_LandscapeDrawDistance_Values` @0x007CA988 = 3/5/8/11/15/25, labels
|
||||
VeryLow/…/Extreme, **default 8**, both byte-verified), registered at
|
||||
`UserPreferences::RegisterPreference` @0x0054ECBE and pushed into
|
||||
`SmartBox::set_mid_radius` @0x004531D0. acdream's structural analogue is the
|
||||
`SmartBox::set_mid_radius` @0x00453180. acdream's structural analogue is the
|
||||
quality preset's `NearRadius`/`FarRadius` pair, which is not separately
|
||||
user-controllable. Split out of #280 deliberately (§3/§14 of that contract):
|
||||
#280 derives its reveal window from whatever feeds the streaming radii, so
|
||||
|
|
|
|||
File diff suppressed because one or more lines are too long
|
|
@ -236,7 +236,7 @@ Which callers block:
|
|||
| `SmartBox::PlayerPositionUpdated` @ `0x00453903` (`:92508`), teleport arm | `ChangePosition(pos, arg2 != 0)` | **yes on teleport** |
|
||||
| `SmartBox::PlayerPositionUpdated`, ordinary arm | `ChangePosition(pos, 0)` | no |
|
||||
| `SmartBox::UseTime` @ `0x00455462` (`:94180`) | `ChangePosition(pos, 0)` | no |
|
||||
| `SmartBox::set_mid_radius` @ `0x004531D0` (`:92053`) | `ChangePosition(pos, 1)` | **yes, if already blocking** |
|
||||
| `SmartBox::set_mid_radius` @ `0x00453180` (re-arm branch at `0x004531D0`, `:92053`) | `ChangePosition(pos, 1)` | **yes, if already blocking** |
|
||||
|
||||
`CellManager::ChangePosition` @ `0x004559B0` (`:94601`) additionally
|
||||
promotes any call to blocking while the latch is set
|
||||
|
|
@ -778,14 +778,43 @@ It is graphical-host-only; headless has no `ObserveWait` caller anywhere
|
|||
`"In Portal Space - Please Wait..."` on recalls **more often**, and the
|
||||
tunnel will run longer before the world appears.
|
||||
|
||||
**This is retail behaviour, not a divergence.** The retail string is
|
||||
byte-identical (§1.6) and retail emits it repeatedly for the whole duration
|
||||
of a blocked prefetch, whose retry is quantised to 5 s (§1.5). A retail
|
||||
client at `LandscapeDrawDistance = 8` recalling into a cold cache sits in
|
||||
the tunnel with the cue and a `CURRENT/TOTAL` cell counter until all 289
|
||||
landblocks are in. acdream showing the cue on a recall is convergence
|
||||
toward retail, not away from it. **No register row is required for the
|
||||
longer hold or the more frequent cue.**
|
||||
**Longer holds ARE retail-convergent — but the argument below was wrong on
|
||||
both of its clauses, and is corrected here (2026-08-06, #280
|
||||
retail-conformance review, finding F2).**
|
||||
|
||||
The retail string is byte-identical (§1.6; VA `0x007BD6A8`, verified). Two
|
||||
things this section originally asserted are not true of the binary:
|
||||
|
||||
1. *"retail emits it for the whole duration of a blocked prefetch"* — the
|
||||
emit site is inside `gmSmartBoxUI::UseTime`'s `TAS_TUNNEL*` branch, in the
|
||||
`else` arm of the rotation-segment-expiry test at `0x004D6FCD`, and fires
|
||||
**unconditionally per tunnel rotation segment** whether or not
|
||||
`blocking_for_cells` is set. The cue is a property of being in the tunnel,
|
||||
not of being blocked. Byte-decoded at `0x004D6FE6`-`0x004D7049`:
|
||||
`teleportRotationDuration = RandDouble(0.6, 1.8)` s
|
||||
(`0x3ffccccc/0xcccccccd` = 1.8, `0x3fe33333/0x33333333` = 0.6),
|
||||
`teleportRotationEndAngle = RandDouble(0, 360)` (`0x40768000`).
|
||||
2. *"whose retry is quantised to 5 s"* — the 5.0 constant at VA `0x007991B0`
|
||||
belongs to `CellManager::CheckPrefetchStatus` @`0x00455BE0`, the prefetch
|
||||
**retry cadence**. It has no connection to the UI notice. #280's commit
|
||||
message repeated this mis-attribution; it is retracted.
|
||||
|
||||
What survives, and is the real justification: retail genuinely **blocks**.
|
||||
`SmartBox::UseTime` @`0x00455410` runs only `CheckPrefetchStatus` while
|
||||
`blocking_for_cells` is latched — object maintenance, physics, the game
|
||||
clock, landscape and ambient are all skipped. A retail client at
|
||||
`LandscapeDrawDistance = 8` recalling into a cold cache sits in the tunnel
|
||||
until all 289 landblocks are in. A longer acdream hold is therefore
|
||||
convergent *in kind*.
|
||||
|
||||
**Two register rows ARE required, and were filed 2026-08-06:**
|
||||
|
||||
- **AP-150** — acdream's `RetailWaitCueDelay = 5 s` arming is not retail's
|
||||
trigger (see the out-of-scope note immediately below, which was right; the
|
||||
error was concluding no row was needed).
|
||||
- **AP-151** — acdream's per-member predicate is much heavier than retail's
|
||||
(mesh build + GPU upload vs. DAT residency), so the hold is not merely
|
||||
"retail's, honestly measured", and nothing bounds it.
|
||||
|
||||
Two adjacent facts, both **out of scope**, both worth writing down so a
|
||||
later session does not mistake them for #280 regressions:
|
||||
|
|
@ -873,6 +902,20 @@ performance story. Report both; do not report only the post-fix number.
|
|||
the entire fix rests on it: if a Far landblock is not render-ready, D2's
|
||||
outer arm never satisfies and the reveal hangs.
|
||||
- **P2 — The outer ring converges within the Far window's own lifetime.**
|
||||
|
||||
> **CORRECTED 2026-08-06 (both #280 review lenses, FAIL).** As written below,
|
||||
> P2 was discharged against **residency** — "cannot be evicted" — when the
|
||||
> gate's actual atom is `IsRenderReady`. Those are different predicates, and
|
||||
> the gap between them is exactly the shipped defect: a Near→Far **demote**
|
||||
> leaves the landblock resident and drawn while revoking its spawn-adapter
|
||||
> registration, so `IsRenderReady` went permanently false and the gate could
|
||||
> never open. Discharging a proof obligation against a predicate the code does
|
||||
> not use proves nothing. The restated obligation is: **no transition may
|
||||
> revoke `IsRenderReady` from a landblock that stays inside `FarRadius`** —
|
||||
> which now holds because `GpuWorldState.ReleaseLandblockMeshReferences`
|
||||
> re-asserts the Far registration on demote. Every remaining sentence below
|
||||
> (hysteresis, recenter, dungeon collapse) is correct as stated.
|
||||
|
||||
The window unloads at `FarRadius + 2` Chebyshev
|
||||
(`src/AcDream.App/Streaming/StreamingRegion.cs:208-209`), so an outer-ring
|
||||
member cannot be evicted while it is inside `FarRadius`. Prove the gate
|
||||
|
|
|
|||
321
docs/research/2026-08-06-280-review-architecture.md
Normal file
321
docs/research/2026-08-06-280-review-architecture.md
Normal file
|
|
@ -0,0 +1,321 @@
|
|||
# #280 architecture review — ownership, layering, blast radius, test quality
|
||||
|
||||
- **Commit under review:** `3aab05b0` — *fix(streaming): derive the portal
|
||||
reveal window from the live streaming radii (#280)*
|
||||
- **Branch / HEAD:** `claude/acdream-physics-divergence-5aa784` @ `fafc0b65`
|
||||
- **Worktree:** `.claude/worktrees/peaceful-visvesvaraya-e0a196`
|
||||
- **Scope:** ownership, layering, blast radius, test quality. Retail fidelity
|
||||
is a separate reviewer's.
|
||||
- **Mode:** read-only. Production edits were made only as sabotage probes and
|
||||
reverted; the tree is clean at `fafc0b65` with no working-tree changes.
|
||||
|
||||
## Verdict: **FAIL**
|
||||
|
||||
One confirmed defect, reproduced end-to-end through production code paths.
|
||||
The predicate D2 widened from a 3×3 always-Near neighbourhood to the full Far
|
||||
window now demands `IsRenderReady` from landblocks that the streaming system
|
||||
**permanently** takes out of render-readiness while leaving them loaded. There
|
||||
is no code path that restores them. Under two ordinary player flows — one of
|
||||
which is the exact mid-hold radius change this commit added
|
||||
`ReconcileDestinationReservationRadius` to support — the reveal gate becomes
|
||||
unsatisfiable and the client stays in "In Portal Space - Please Wait…"
|
||||
forever.
|
||||
|
||||
Everything else checked out. Layering held, the no-window host is genuinely
|
||||
untouched, the allocation fix is real, and the tests are unusually well
|
||||
disciplined about not re-encoding the constant under test. The failure is a
|
||||
single missed state transition, not sloppy work — but it is the failure mode
|
||||
the change's own proof obligation P2 was written to exclude, discharged
|
||||
against the wrong predicate.
|
||||
|
||||
---
|
||||
|
||||
## Defects
|
||||
|
||||
### D-1 — A demoted-but-loaded landblock is permanently not `IsRenderReady`, so the widened gate can never open (**CONFIRMED, reproduced**)
|
||||
|
||||
**Severity: defect. Blocks the fix.**
|
||||
|
||||
`src/AcDream.App/Streaming/StreamingController.cs:270-278` — the new outer arm
|
||||
requires `_state.IsRenderReady(canonical)` of every in-bounds member out to
|
||||
`farRadius`.
|
||||
|
||||
`IsRenderReady` is not a property of "loaded":
|
||||
|
||||
- `src/AcDream.App/Streaming/GpuWorldState.cs:179-181`
|
||||
`IsRenderReady = _loaded.ContainsKey(id) && _wbSpawnAdapter.IsLandblockRenderReady(id)`
|
||||
- `src/AcDream.App/Rendering/Wb/LandblockSpawnAdapter.cs:135-139`
|
||||
returns **false** when the landblock has no registration, or a registration
|
||||
with `WantsLoaded == false`.
|
||||
|
||||
A Near→Far **demote** destroys exactly that registration while keeping the
|
||||
landblock loaded:
|
||||
|
||||
- `src/AcDream.App/Streaming/StreamingController.cs:555-559`
|
||||
`DemoteLandblock` → `LandblockPresentationPipeline.EnqueueNearLayerRetirement`
|
||||
- `src/AcDream.App/Streaming/LandblockRetirementCoordinator.cs:309-312`
|
||||
a `NearLayer` ticket still runs `CoreStages`, which includes
|
||||
`LandblockRetirementStage.MeshReferences`
|
||||
- `src/AcDream.App/Streaming/LandblockRetirementCoordinator.cs:738-740`
|
||||
→ `GpuWorldState.ReleaseLandblockMeshReferences`
|
||||
- `src/AcDream.App/Streaming/GpuWorldState.cs:1559-1560`
|
||||
→ `LandblockSpawnAdapter.OnLandblockUnloaded` → `WantsLoaded = false`,
|
||||
registration dropped
|
||||
- `src/AcDream.App/Streaming/GpuWorldState.cs:1800-1835`
|
||||
meanwhile `DetachNearLayer` **keeps** `_loaded[canonical]` and flips
|
||||
`_tierByLandblock[canonical] = Far`
|
||||
|
||||
Nothing re-publishes an already-loaded landblock. `RecenterTo`
|
||||
(`StreamingRegion.cs:232-244`) emits `ToLoadFar` only for ids absent from
|
||||
`_tierResidence`, and a demoted block is still present as `Far`. Only a
|
||||
**promote** back into the Near ring (`StreamingRegion.cs:239-244` →
|
||||
`PublicationKind.PromoteExisting` → `ActivateLandblockPresentation` →
|
||||
`OnLandblockLoaded`) restores the registration. Members of the band
|
||||
`NearRadius+1 … FarRadius` are never promoted, so they stay dead for the rest
|
||||
of the window's life.
|
||||
|
||||
**Evidence (temporary probes, run then removed):**
|
||||
|
||||
| Probe | Result |
|
||||
|---|---|
|
||||
| `GpuWorldState.IsRenderReady` after `LandblockPresentationPipeline.BeginNearLayerRetirement` (the production demote entry point) | **false** |
|
||||
| `GpuWorldState.IsRenderReady` after `GpuWorldState.RemoveEntitiesFromLandblock` (same `DetachNearLayer` + `ReleaseLandblockMeshReferences` pair) | **false** |
|
||||
| Full Near 5×5 window; `controller.IsRenderNeighborhoodResident(dest, 1, 2)` before one outer-ring demote | **true** |
|
||||
| …the same call after that single demote | **false**, and never recovers |
|
||||
|
||||
All three probes used a real `GpuWorldState` + real `LandblockSpawnAdapter` +
|
||||
real `LandblockPresentationPipeline`.
|
||||
|
||||
**Reachable failure scenarios**
|
||||
|
||||
1. **Mid-hold quality change (the one this commit explicitly added code for).**
|
||||
`StreamingController.ReconfigureRadii` at `:484-488` emits
|
||||
`DemoteLandblock` for every currently-Near landblock that the new preset
|
||||
puts in the Far band. Presets are High `(4,12)` → Low `(2,5)`
|
||||
(`src/AcDream.UI.Abstractions/Settings/QualityPreset.cs:31-34`), so a
|
||||
High→Low switch demotes the Chebyshev 3–5 band, which is *inside* the new
|
||||
Far window. The gate then requires `IsRenderReady` from all of them.
|
||||
Result: the hold never ends, and
|
||||
`WorldRevealCoordinator.ReconcileDestinationReservationRadius`
|
||||
(`:521-543`) cheerfully re-opens a reservation on a square that can no
|
||||
longer converge. Note also that `NearRadius`/`FarRadius` only publish at
|
||||
transaction convergence (`StreamingController.cs:886-888`) while the
|
||||
demote mutations run earlier (`:860-876`), so for several frames the
|
||||
barrier measures the OLD wide window against blocks already demoted for
|
||||
the NEW narrow one.
|
||||
|
||||
2. **A reveal that does not recenter the origin.**
|
||||
`LocalPlayerTeleportController.cs:934` only calls `_streaming.BeginRecenter`
|
||||
when `transition.ChangesStreamingCenter`, i.e. when the destination
|
||||
landblock differs from the **world-origin** landblock. Origin recenter is
|
||||
the only thing that clears the window (`GpuWorldState.cs:1481`
|
||||
`_loaded.Clear()`). Walking ≳ `NearRadius + 2` landblocks from the last
|
||||
origin (`StreamingRegion.cs:263-270`) demotes blocks into the Far band;
|
||||
a subsequent teleport back to the origin landblock (lifestone recall →
|
||||
travel → lifestone recall) takes the no-recenter path and inherits them.
|
||||
Blocks that land back inside the Near ring are promoted and recover;
|
||||
blocks in `near+1 … far` do not.
|
||||
|
||||
**Why the old gate was safe:** at `OutdoorNeighborhoodRadius = 1` the required
|
||||
set was the destination's own 3×3, which during a hold is pinned to the
|
||||
streaming centre (`StreamingFrameController.SelectObserver:194-200` freezes the
|
||||
observer at `_origin` while `PlayerState.PortalSpace`) and therefore always
|
||||
Near tier. Demotion inside the Near ring cannot happen. The defect is
|
||||
introduced by this commit, not exposed by it.
|
||||
|
||||
**Why P2 missed it.** `docs/research/2026-08-05-280-contract.md:876-884` states
|
||||
P2 as "an outer-ring member cannot be **evicted** while it is inside
|
||||
`FarRadius`" and cites the `FarRadius + 2` unload threshold. That is true and
|
||||
irrelevant: the gate's atom is `IsRenderReady`, not residency. P2 was
|
||||
discharged against the wrong predicate. Every remaining sentence of the
|
||||
static proof (hysteresis, recenter, dungeon collapse) is correct.
|
||||
|
||||
**Also note:** the P1 test's own comment
|
||||
(`tests/AcDream.App.Tests/Streaming/StreamingControllerReadinessTests.cs:567-570`)
|
||||
describes its subject as *"A Near-shaped completion that the streaming window
|
||||
has since demoted to Far"*. It is not — it is a fresh `PublishAsFar` of a
|
||||
landblock that was never loaded, which is the case that **does** hold. The
|
||||
comment names the one case that is false. A future reader will take it as
|
||||
coverage of the demote path.
|
||||
|
||||
---
|
||||
|
||||
## Latent risks
|
||||
|
||||
### R-1 — `ACDREAM_PROBE_REVEAL_RADIUS=0` is accepted by the parser and rejected by Runtime
|
||||
|
||||
`src/AcDream.App/Streaming/StreamingDiagnostics.cs:54-55` accepts any
|
||||
`value >= 0`. `ApplyRevealRadiusOverride` then yields
|
||||
`StreamingRevealWindow(0, 0)`, so `RequiredWindow` returns far = 0 for an
|
||||
**outdoor** destination, and
|
||||
`src/AcDream.Runtime/World/RuntimeWorldTransitState.cs:585-597` fails
|
||||
`invalid-readiness-shape` on every acknowledgement (outdoor ⇒ `>= 1`). The
|
||||
A/B probe would hang the route it is meant to measure. The documented value is
|
||||
`1`; the parser should refuse `0` rather than let the two halves of the same
|
||||
commit disagree. One line.
|
||||
|
||||
### R-2 — the composite-warmup **trigger** moved onto the far window's critical path, undocumented
|
||||
|
||||
`WorldRevealReadinessBarrier.Prepare:120-133` starts composite preparation only
|
||||
once `_isRenderNeighborhoodReady(dest, near, far)` is true. Before this commit
|
||||
that predicate was the destination's 3×3; it is now the entire 25×25. The
|
||||
commit message and D3 justify keeping the composite **radius** at `NearRadius`
|
||||
(correct — Far builds carry no entities), but say nothing about the trigger.
|
||||
Net effect: composite upload can no longer overlap far-ring streaming; the
|
||||
hold is longer than the streaming work alone requires by the whole warmup
|
||||
duration. This is a real serialisation the "expect longer holds" paragraph
|
||||
does not account for, and it is separable — warming composites at Near-ready
|
||||
would restore the overlap without weakening the gate.
|
||||
|
||||
### R-3 — two different windows describe "destination publication incomplete"
|
||||
|
||||
`StreamingController.cs:719-724` computes the destination-lane preference with
|
||||
`(Math.Min(NearRadius, DestinationRadius), DestinationRadius)` — live
|
||||
`NearRadius` against the *reservation's* radius — while the gate uses
|
||||
`RequiredWindow`'s `(clamp(NearRadius, 0, FarRadius), FarRadius)`. Between a
|
||||
preset change and reservation reconciliation these name different squares, so
|
||||
the work-lane prioritisation can consider the destination complete on a
|
||||
square the gate still rejects. Cosmetic today; it becomes a real starvation
|
||||
question once R-1/D-1 are fixed and holds get long.
|
||||
|
||||
### R-4 — Runtime lost its only cross-host consistency check
|
||||
|
||||
The `indoor ? 0 : 1` equality became `indoor ⇒ 0`, `outdoor ⇒ ≥ 1`
|
||||
(`RuntimeWorldTransitState.cs:585-597`). The layering argument is right —
|
||||
Runtime cannot learn the graphical host's streaming configuration, and
|
||||
plumbing App radii in would be exactly the C5b failure. The cost is that
|
||||
Runtime can no longer detect an App regression to a hardcoded radius: a
|
||||
future revert of D1 would emit `1` and pass. Accepted, but worth recording
|
||||
that the invariant is now shape-only and the only remaining guard on the
|
||||
value is the App-side tests.
|
||||
|
||||
---
|
||||
|
||||
## What was checked and is sound
|
||||
|
||||
**Layering / ownership (attack 6) — held.**
|
||||
`WorldRevealCoordinator` and `WorldRevealReadinessBarrier` are `internal` to
|
||||
`AcDream.App`. `src/AcDream.Headless/AcDream.Headless.csproj` references only
|
||||
`AcDream.Runtime`. Both non-graphical producers
|
||||
(`HeadlessSessionWorldProjection.cs:979`,
|
||||
`RuntimeLiveEntitySessionController.cs:782`) keep emitting their own
|
||||
centre-ring token and are legal under the loosened shape by construction, and
|
||||
both were annotated in place explaining why they must **not** track the
|
||||
graphical radius. No App type crossed into Runtime. `AcDream.Headless.Tests`
|
||||
passes 89/89 including its dependency/loaded-assembly guards. C5b's lesson was
|
||||
applied, not repeated.
|
||||
|
||||
**P1 (attack 2) — independently verified TRUE, by reading rather than by
|
||||
reading the test.** `PublishAsFar` (`LandblockPresentationPipeline.cs:432-522`)
|
||||
constructs a `LoadedLandblock` with `Array.Empty<WorldEntity>()` and
|
||||
`PhysicsDatBundle.Empty`; the spatial commit
|
||||
(`LandblockPresentationPipeline.cs:900-916`) routes `PublicationKind.Far`
|
||||
through `CommitLandblockSpatial`, which returns a publication with
|
||||
`RequiresActivation` defaulted true (`GpuWorldState.cs:24`), so
|
||||
`ActivateLandblockPresentation` (`:1009-1035`) calls `OnLandblockLoaded` with
|
||||
an empty entity list and empty `AdditionalRenderIds`. The registration is
|
||||
created with `WantsLoaded = true` and both reference dictionaries empty, so
|
||||
`IsLandblockRenderReady` returns true with no `IWbMeshAdapter` upload. The one
|
||||
early-out (`GpuWorldState.cs:913-923`, a stale Far completion over a live Near
|
||||
tier) returns `RequiresActivation: false`, but that case is already
|
||||
registered. **P1 holds. The fix shape does not collapse.** Its failure is D-1,
|
||||
a different transition.
|
||||
|
||||
Same check for the collision arm, which is equally load-bearing and was not
|
||||
called out as a proof obligation: `LandblockPhysicsPublisher.AdvanceBeginOne`
|
||||
(`:390-435`) builds the terrain surface from the heightmap with no dat bundle
|
||||
and stages it, so a Far publication does register terrain collision;
|
||||
`AdvanceDemotion` (`:675-683`) calls `DemoteCollisionToTerrain`, which
|
||||
**retains** terrain. `IsNeighborhoodTerrainResident` therefore stays satisfiable
|
||||
across a demote. The render arm is the only one that breaks.
|
||||
|
||||
**Reveal-hang sweep (attack 1) — all other configurations converge.**
|
||||
|
||||
| Configuration | Result |
|
||||
|---|---|
|
||||
| Map edge (coords clamped 0/254) | Safe. Gate skips `> 254` (`StreamingController.cs:265-266`); `StreamingRegion` loads up to `0xFF` (`:80`). The loaded set is a strict superset of the required set, and `PhysicsEngine.cs:157` uses the identical bound. |
|
||||
| Dungeon → outdoor | Safe. `TryCommitOriginRecenterCore:1235-1251` clears `_collapsed` for a non-dungeon destination and nulls `_region` so the next tick bootstraps the full window. |
|
||||
| Indoor destination | Safe. `RequiredWindow` returns `(0,0)` before touching the live window (`WorldRevealReadinessBarrier.cs:206-215`), so no streaming state can gate it. |
|
||||
| Recenter in flight | Safe. `Tick` is blocked while `_originRecenterRetirement` is open, radii requests defer, and `_loaded.Clear()` (`GpuWorldState.cs:1481`) guarantees every member of the rebuilt window gets a fresh publication and registration. |
|
||||
| Destination/observer landblock offset | Safe **during a hold**: `StreamingFrameController.SelectObserver:194-200` pins the observer to `_origin` while `PlayerState.PortalSpace`, so the gate's inner ring and the streaming Near ring are the same square. Worth noting the new inner arm has **zero** margin here where the old one had `NearRadius − 1`; the pin is now load-bearing. |
|
||||
| `FarRadius == 0` on an outdoor destination | Not reachable from presets (min far = 5); reachable only via R-1. |
|
||||
|
||||
**D6 allocation fix — real, and the sabotage number is honest.**
|
||||
`PhysicsEngine.cs:49-52,146-149`. Single production call site
|
||||
(`SessionPlayerComposition.cs:384`), main thread, leaf method — the
|
||||
instance-owned scratch is safe, and it is cleared at entry so no stale state
|
||||
can leak across sessions.
|
||||
|
||||
**Test discipline — good, with two gaps.** No test re-encodes the constant
|
||||
under test: every radius assertion references the fake window's own input
|
||||
(`WorldRevealReadinessBarrierTests`, `WorldRevealDerivedWindowIntegrationTests`).
|
||||
No new `Skip=`. No test deleted — one was renamed
|
||||
(`OutdoorReveal_JoinsNearRenderTexturesAndTerrain` →
|
||||
`RequiredWindow_IsRereadOnEveryEvaluationWithoutReconstruction`) with its
|
||||
original assertions carried into a new
|
||||
`OutdoorReveal_JoinsRenderTexturesAndTerrainOverTheDerivedWindow`; coverage is
|
||||
preserved and the +36 arithmetic holds.
|
||||
|
||||
Gaps:
|
||||
- No test exercises a demote. That is D-1.
|
||||
- `WorldRevealDerivedWindowIntegrationTests` advertises itself as end-to-end
|
||||
against "the REAL `StreamingController`, `GpuWorldState`, and
|
||||
`PhysicsEngine`", but constructs `new GpuWorldState()` with no spawn
|
||||
adapter, so `IsRenderReady` degenerates to `IsLoaded` via the
|
||||
`?? true` at `GpuWorldState.cs:181`. The single most load-bearing predicate
|
||||
in the whole change is stubbed out by a null in the test named after it.
|
||||
It also builds its own `revealWindow` lambda rather than the production one
|
||||
in `SessionPlayerComposition.cs:373-379`, so the
|
||||
`ApplyRevealRadiusOverride` wrapper is never exercised in composition.
|
||||
|
||||
**Sabotage spot-checks — 4 of 9 families reproduced (asked for ≥ 3).**
|
||||
|
||||
| Sabotage | Named test(s) | Result |
|
||||
|---|---|---|
|
||||
| `PhysicsEngine`: scratch → `new HashSet<uint>()` | `WarmedNeighborhoodQuery_AtTheFarRadius_AllocatesNothing` | FAIL, **"allocated 27,712,000 bytes"** — S5's claimed figure reproduces to the byte |
|
||||
| `StreamingController`: revert tier split to `!IsNearTier \|\| !IsRenderReady` | `TieredWindow_OuterRingFarTierMemberIsResident`, `TieredWindow_AbsentOuterRingMemberIsNotResident`, `TieredWindow_MapCornerDestinationConvergesAtAWideRadius`, `OutdoorReveal_HoldsUntilTheWholeDerivedWindowIsPublished` ×2, `LoginReveal_UsesTheSameWidenedGateAsPortalArrival` | 6 FAIL |
|
||||
| `RuntimeWorldTransitState`: shape → `== (isIndoor ? 0 : 1)` | `OutdoorReadinessShape_AcceptsAnyDerivedStreamingRadius` | 6 FAIL (all inline radii) |
|
||||
| `WorldRevealCoordinator`: neuter `ReconcileDestinationReservationRadius` | `MidHoldRadiusChange_ReopensTheReservationOnTheSameGeneration` | 1 FAIL |
|
||||
|
||||
All four reverted; `git status` clean afterwards. No sign of the C5b-D3 class
|
||||
(a test that passes with its own change reverted) in the families checked.
|
||||
|
||||
**Suite at HEAD `fafc0b65`, Release, verified by running it:**
|
||||
`11,179 passed / 4 skipped / 0 failed` — exactly the expected
|
||||
11,178 + 1 Core settler test. Per project: UI 546, Content 124, Runtime 1217,
|
||||
Cli 4, Bake 15, App 4157/3 skips, Headless 89, Core.Net 764, Core 4263/1 skip.
|
||||
None of #302/#308/#321 surfaced in this run and none are conflated with the
|
||||
finding above.
|
||||
|
||||
**Process rules — clean.** No suppression flag, grace period, retry loop, or
|
||||
symptom guard was introduced. `ACDREAM_PROBE_REVEAL_RADIUS` lives in a
|
||||
diagnostic owner per Code Structure Rule 5 and is correctly argued as a
|
||||
measurement probe rather than a shipped knob. `AD-2` was amended and `AP-149`
|
||||
filed in the same commit, satisfying the divergence-register rule. The
|
||||
`ACDREAM_STREAM_RADIUS` CLAUDE.md correction is accurate against
|
||||
`SessionPlayerComposition.ComposeCore:249-257`.
|
||||
|
||||
---
|
||||
|
||||
## Recommended disposition
|
||||
|
||||
Do not ship the gate widening until D-1 is closed. The two shapes worth
|
||||
considering, in preference order:
|
||||
|
||||
1. **Make a demote re-register the Far tier.** The demote already leaves a
|
||||
fully valid Far-tier landblock behind; the registration it drops is the
|
||||
*Near* mesh set. `LandblockRetirementStage.MeshReferences` on a `NearLayer`
|
||||
ticket should release the Near references and then re-assert an empty
|
||||
Far registration, exactly as `PublishAsFar` does — i.e. the same
|
||||
`OnLandblockLoaded(landblock, empty)` call the Far publication makes.
|
||||
This makes `IsRenderReady` mean "drawable at its current tier", which is
|
||||
what both the render path and the gate already assume it means.
|
||||
2. **Re-publish demoted members inside a destination reservation.** Weaker:
|
||||
it fixes the gate without fixing the predicate, and leaves the next caller
|
||||
of `IsRenderReady` holding the same trap.
|
||||
|
||||
Whichever is chosen, it needs a test at the demote transition —
|
||||
`Near publish → demote → IsRenderNeighborhoodResident(dest, near, far)` — and
|
||||
the `WorldRevealDerivedWindowIntegrationTests` fixture should be given a real
|
||||
`LandblockSpawnAdapter` so its `IsRenderReady` stops being a tautology.
|
||||
R-1 and R-2 are one-line and one-decision respectively and can ride along.
|
||||
577
docs/research/2026-08-06-280-review-retail.md
Normal file
577
docs/research/2026-08-06-280-review-retail.md
Normal file
|
|
@ -0,0 +1,577 @@
|
|||
# #280 retail-conformance review — portal destination prefetch
|
||||
|
||||
**Commit under review:** `3aab05b0` (`fix(streaming): derive the portal reveal
|
||||
window from the live streaming radii (#280)`), on branch
|
||||
`claude/acdream-physics-divergence-5aa784` in worktree
|
||||
`.claude/worktrees/peaceful-visvesvaraya-e0a196`. HEAD at review time is
|
||||
`fafc0b65` (one later, unrelated: #276's settler fix).
|
||||
|
||||
**Reviewer role:** retail-conformance. Read-only. Contract
|
||||
(`docs/research/2026-08-05-280-contract.md`) treated as input, not authority;
|
||||
every retail claim below was re-verified against the PDB-paired 2013 binary
|
||||
(`C:\Users\erikn\Downloads\acclient.exe`, `check_exe_pdb.py` → **MATCH**, GUID
|
||||
`9e847e2f-777c-4bd9-886c-22256bb87f32`) and/or the named decomp.
|
||||
|
||||
---
|
||||
|
||||
## VERDICT: **FAIL**
|
||||
|
||||
One high-severity defect: **the reveal gate's outer (Far) arm uses a predicate
|
||||
that a DEMOTED landblock can never satisfy**, so the outdoor reveal can hang
|
||||
permanently on a reachable player action (two consecutive recalls to the same
|
||||
landblock with walking in between). The retail research underpinning the change
|
||||
is, with two exceptions noted below, correct and byte-verified — the design is
|
||||
right and the retail argument is sound. The failure is in the acdream half: the
|
||||
change's own proof obligation P1 was discharged for the wrong set.
|
||||
|
||||
Findings ranked by severity. F1 blocks; F2–F3 are bookkeeping/argument defects;
|
||||
F4–F7 are minor.
|
||||
|
||||
---
|
||||
|
||||
## F1 — HIGH — the Far arm's predicate is unsatisfiable for a demoted landblock; the reveal can hang forever
|
||||
|
||||
### What the change assumes
|
||||
|
||||
`StreamingController.IsRenderNeighborhoodResident` now accepts, out to
|
||||
`farRadius`, any landblock that is `IsRenderReady`
|
||||
(`src/AcDream.App/Streaming/StreamingController.cs:274-279`):
|
||||
|
||||
```csharp
|
||||
if (!_state.IsRenderReady(canonical))
|
||||
return false;
|
||||
bool isInnerRing = Math.Abs(dx) <= nearRadius && Math.Abs(dy) <= nearRadius;
|
||||
if (isInnerRing && !_state.IsNearTier(canonical))
|
||||
return false;
|
||||
```
|
||||
|
||||
The in-code rationale (`StreamingController.cs:266-268`) and the AD-2 amendment
|
||||
both justify this as: *"a Far-tier publication registers with the spawn adapter
|
||||
and an empty mesh set, so this is a real drawability test out there, not a
|
||||
stamp."*
|
||||
|
||||
That is true for a landblock that **arrived** as Far. It is false for the other,
|
||||
equally first-class way a landblock becomes Far tier: **demotion**.
|
||||
|
||||
### What actually happens
|
||||
|
||||
`GpuWorldState.IsRenderReady` (`src/AcDream.App/Streaming/GpuWorldState.cs:179-181`):
|
||||
|
||||
```csharp
|
||||
public bool IsRenderReady(uint landblockId) =>
|
||||
_loaded.ContainsKey(landblockId)
|
||||
&& (_wbSpawnAdapter?.IsLandblockRenderReady(landblockId) ?? true);
|
||||
```
|
||||
|
||||
`LandblockSpawnAdapter.IsLandblockRenderReady`
|
||||
(`src/AcDream.App/Rendering/Wb/LandblockSpawnAdapter.cs:135-139`) returns
|
||||
`false` the moment `registration.WantsLoaded` is false.
|
||||
|
||||
The Near→Far demote path clears exactly that flag:
|
||||
|
||||
- `StreamingController.cs:1013` (`foreach (var id in diff.ToDemote) DemoteLandblock(id);`)
|
||||
and `StreamingController.cs:487` (the `ReconfigureRadii` mutation) →
|
||||
- `StreamingController.DemoteLandblock` (`StreamingController.cs:555-559`) →
|
||||
`_presentation.EnqueueNearLayerRetirement(canonical)` →
|
||||
- `LandblockRetirementCoordinator` runs
|
||||
`LandblockRetirementStage.MeshReferences` →
|
||||
`_state.ReleaseLandblockMeshReferences(ticket.LandblockId)`
|
||||
(`src/AcDream.App/Streaming/LandblockRetirementCoordinator.cs:739-740`, and the
|
||||
stepped variant at `:799-803`) →
|
||||
- `GpuWorldState.ReleaseLandblockMeshReferences`
|
||||
(`GpuWorldState.cs:1559-1560`) → `_wbSpawnAdapter.OnLandblockUnloaded(id)` →
|
||||
- `LandblockSpawnAdapter.OnLandblockUnloaded`
|
||||
(`LandblockSpawnAdapter.cs:159-166`) sets `registration.WantsLoaded = false`.
|
||||
|
||||
Meanwhile `GpuWorldState.DetachNearLayer` (`GpuWorldState.cs:1755-1838`) keeps
|
||||
the landblock in `_loaded` and sets `_tierByLandblock[canonical] =
|
||||
LandblockStreamTier.Far`. Nothing re-registers it. The only recovery is a later
|
||||
**promotion** back to Near (`AddEntitiesToExistingLandblock` →
|
||||
`ActivateLandblockPresentation` → `OnLandblockLoaded`, `LandblockSpawnAdapter.cs:103-111`)
|
||||
or a full unload + reload.
|
||||
|
||||
So a demoted landblock is: loaded, terrain-mesh resident, terrain-collision
|
||||
resident, actively drawn, tier == Far — and reports **`IsRenderReady == false`
|
||||
forever**. It fails the Far arm, and (being Far tier) it would also fail the
|
||||
Near arm. It satisfies *no* arm of the gate.
|
||||
|
||||
The physics arm is unaffected — `PhysicsEngine.DemoteLandblockToTerrain`
|
||||
(`src/AcDream.Core/Physics/PhysicsEngine.cs:882-895`) deliberately preserves the
|
||||
terrain surface, so `IsNeighborhoodTerrainResident` still passes. The render arm
|
||||
is the only one that breaks, and it is enough.
|
||||
|
||||
### Why this is new with #280
|
||||
|
||||
Pre-#280 the outdoor gate was a fixed radius-1 square around the destination
|
||||
(`OutdoorNeighborhoodRadius = 1`) requiring `IsNearTier && IsRenderReady`. Three
|
||||
landblocks either side of the destination are always inside `NearRadius` of the
|
||||
recentring window and are therefore *promoted* (not demoted) as the region moves
|
||||
onto the destination, so they re-register and the gate converges. Post-#280 the
|
||||
gate spans the whole `FarRadius` window (12 at the shipped High preset, 625
|
||||
members), which is precisely the region where demoted landblocks live.
|
||||
|
||||
### Reachability — a plausible, ordinary player action
|
||||
|
||||
The safe path is a portal that recenters the world origin: `BeginRecenter` →
|
||||
`DetachAllForOriginRecenter` → `_region = null` (`StreamingController.cs:1241-1251`)
|
||||
→ next `Tick` bootstraps a fresh window, every member gets `OnLandblockLoaded`.
|
||||
|
||||
The unsafe path is a portal that does **not** recenter.
|
||||
`TeleportLandblockTransition.ChangesStreamingCenter`
|
||||
(`src/AcDream.App/Streaming/TeleportLandblockTransition.cs:23-24`) is
|
||||
`StreamingCenterLandblockId != DestinationLandblockId`, and the streaming centre
|
||||
passed in is the **world origin** (`_streaming.CenterX/CenterY` at
|
||||
`src/AcDream.App/Streaming/LocalPlayerTeleportController.cs:899-911`), which
|
||||
moves only on teleport — never while walking. So:
|
||||
|
||||
1. Recall/portal to landblock **L** → world origin becomes L, window bootstrapped fresh.
|
||||
2. Walk outward several landblocks. The streaming *region* follows the player
|
||||
(`StreamingFrameController.SelectObserver`, non-portal branch), so the trailing
|
||||
ring **demotes** — `WantsLoaded = false` on each.
|
||||
3. Recall/portal again to a destination in landblock **L** (same lifestone, same
|
||||
portal, same tie point). `ChangesStreamingCenter` is now **false** → no origin
|
||||
recenter, no detach-all.
|
||||
4. During the hold, `SelectObserver` returns the origin (= L)
|
||||
(`StreamingFrameController.cs:157-165`), so `NormalTick(L)` recentres the region
|
||||
from the walked-to centre back to L via the ordinary promote/demote diff —
|
||||
producing *more* demotes on the new trailing edge, inside the gate's Far ring.
|
||||
5. Those members can never become `IsRenderReady`. `WorldRevealReadinessBarrier.Evaluate`
|
||||
never returns `IsReady`. **The client stays in portal space indefinitely.**
|
||||
|
||||
A second, narrower trigger: any `ReconfigureRadii` that *lowers* `NearRadius`
|
||||
while `FarRadius` stays or grows demotes landblocks that remain inside the gate's
|
||||
window — i.e. the very mid-hold Settings change D1 was written to support.
|
||||
|
||||
### Secondary consequence
|
||||
|
||||
`StreamingController.Tick` (`StreamingController.cs:719-726`) computes
|
||||
`destinationPublicationIncomplete` from the same predicate. Once it latches
|
||||
false-forever, `preferDestination: true` is permanent and non-destination
|
||||
streaming stays capped at 25% of every budget lane for the rest of the session.
|
||||
|
||||
### Observable in-game consequence
|
||||
|
||||
Wormhole tunnel + centered "In Portal Space - Please Wait..." forever, no world,
|
||||
no recovery short of relog. Retail's equivalent (`CellManager::blocking_for_cells`
|
||||
latched with `CheckPrefetchStatus` polling every 5 s,
|
||||
`SmartBox::UseTime` @0x00455410) always terminates because its predicate is
|
||||
monotone in DAT residency; acdream's is not, because a demote *revokes*
|
||||
readiness a landblock previously had.
|
||||
|
||||
### Corroboration
|
||||
|
||||
A previous review session's probe file
|
||||
(`tests/AcDream.App.Tests/Streaming/ZzReviewProbeTests.cs`, since deleted, still
|
||||
compiled into the prebuilt `AcDream.App.Tests.dll` of 2026-08-06 06:59) fails
|
||||
with exactly:
|
||||
|
||||
```
|
||||
Probe_DemotedLandblockStillRenderReady — "demoted landblock is NOT render ready -> #280 far arm unsatisfiable"
|
||||
Probe_DemotedViaStateEdgeStillRenderReady — same
|
||||
Probe_TieredGateConvergesAfterAnOuterRingDemote — "the reveal gate can no longer be satisfied after a demote"
|
||||
```
|
||||
|
||||
I treated that file as untrusted data and derived the finding independently from
|
||||
the source trace above; the failing probe is corroboration, not the basis.
|
||||
|
||||
### What a fix has to decide (not prescribed here)
|
||||
|
||||
The honest question is what "drawable at Far distance" means. Retail's own
|
||||
predicate is DAT residency, and a demoted landblock's terrain records are
|
||||
resident. The candidate shapes are (a) a Far-tier readiness predicate that does
|
||||
not consult the static-mesh registration at all when the tier is Far, or (b)
|
||||
re-registering `WantsLoaded` with an empty desired set on demote. Both are
|
||||
behaviour changes outside a review's remit.
|
||||
|
||||
---
|
||||
|
||||
## F2 — MEDIUM — the commit's retail-convergence argument for the wait cue is wrong on both clauses
|
||||
|
||||
The commit message closes with:
|
||||
|
||||
> retail emits the byte-identical string for the whole duration of a blocked
|
||||
> prefetch and polls at 5 s intervals
|
||||
|
||||
Neither half survives the binary.
|
||||
|
||||
**The string is byte-identical — verified.** UTF-16LE
|
||||
`"In Portal Space - Please Wait..."` lives at VA **0x007BD6A8** (file offset
|
||||
0x3BD6A8); the construction site the contract cites, 0x004D7064, is the
|
||||
`PStringBase<unsigned short>` ctor call that pushes it (`68 a8 d6 7b 00` at
|
||||
0x004D705E). acdream's literal at
|
||||
`src/AcDream.App/Rendering/PortalTunnelPresentation.cs:297,381` matches exactly.
|
||||
|
||||
**But its trigger is the tunnel rotation segment, not the prefetch.** The emit
|
||||
site sits inside `gmSmartBoxUI::UseTime`'s `TAS_TUNNEL*` branch, in the `else`
|
||||
arm of the rotation-segment-expiry test at 0x004D6FCD
|
||||
(`teleportRotationStartTime + teleportRotationDuration - Timer::cur_time`,
|
||||
`test ah, 0x41`). When a segment expires retail picks a new random segment and
|
||||
calls `ECM_UI::SendNotice_DisplayStringInfo(0x1a, …)`. Byte-decoded constants at
|
||||
0x004D6FE6-0x004D7049:
|
||||
|
||||
```
|
||||
68 cc cc fc 3f 68 cd cc cc cc push 0x3ffccccc / 0xcccccccd -> 1.8
|
||||
68 33 33 e3 3f 68 33 33 33 33 push 0x3fe33333 / 0x33333333 -> 0.6
|
||||
68 00 80 76 40 6a 00 6a 00 6a 00 push 0x40768000, 0,0,0 -> RandDouble(0.0, 360.0)
|
||||
```
|
||||
|
||||
i.e. `teleportRotationDuration = RandDouble(0.6, 1.8)` s and
|
||||
`teleportRotationEndAngle = RandDouble(0, 360)`. Retail therefore shows the
|
||||
notice **unconditionally, from 0.6–1.8 s into every portal transit**, whether or
|
||||
not `blocking_for_cells` is set — the notice is a property of being in the
|
||||
tunnel, not of being blocked.
|
||||
|
||||
**The 5 s figure belongs to a different mechanism.**
|
||||
`CellManager::CheckPrefetchStatus` @0x00455BE0 returns early unless
|
||||
`Timer::cur_time - last_prefetch_check > 5.0` (constant byte-verified at
|
||||
0x007991B0: `00 00 00 00 00 00 14 40` = double 5.0). That is the prefetch retry
|
||||
cadence. It has nothing to do with the UI notice.
|
||||
|
||||
**acdream diverges.** `RuntimeWorldTransitState.RetailWaitCueDelay =
|
||||
TimeSpan.FromSeconds(5)` (`src/AcDream.Runtime/World/RuntimeWorldTransitState.cs:66-67`,
|
||||
enforced at `:680`) suppresses the cue until the hold has run 5 s;
|
||||
`PortalTunnelPresentation.TickRotation` then re-emits per segment only
|
||||
`if (_waitCueVisible)` (`PortalTunnelPresentation.cs:378-380`). acdream's own
|
||||
segment constants (`RotationDurationMin = 0.6f`, `RotationDurationMax = 1.8f`,
|
||||
`PortalTunnelPresentation.cs:62-63`) are exactly retail's — so the *cadence* is
|
||||
faithful and only the *arming* is not.
|
||||
|
||||
This divergence pre-dates #280 (it is not introduced here), but:
|
||||
|
||||
1. It is **not registered as a divergence**. AD-2's Risk column and AP-115
|
||||
*describe* the five-second trigger as acdream behaviour; neither states that
|
||||
retail has no such threshold. A reader of the register cannot learn that
|
||||
acdream is late by 3.2–4.4 s on every single portal.
|
||||
2. #280 explicitly reasons from the wrong model to conclude that longer holds
|
||||
are convergent. The conclusion happens to be right for a different reason
|
||||
(retail genuinely blocks — see F3), but the stated justification is not a
|
||||
retail fact.
|
||||
|
||||
**Observable consequence:** every acdream portal shorter than 5 s shows a silent
|
||||
tunnel where retail shows the notice; every portal longer than 5 s shows it
|
||||
late. #280 makes holds longer, which masks rather than fixes this.
|
||||
|
||||
---
|
||||
|
||||
## F3 — MEDIUM — unfiled: acdream's gate is now materially STRICTER than retail's prefetch predicate, and nothing records the hold-duration asymmetry
|
||||
|
||||
AP-149 records the direction in which acdream is *weaker* than retail (outer
|
||||
ring accepts terrain-only). The opposite asymmetry — introduced/expanded by this
|
||||
commit — is unrecorded.
|
||||
|
||||
Retail's `LScape::PreFetchCells` @0x00505660 requires, per square member, only
|
||||
that the DAT records be **resident in memory**:
|
||||
|
||||
```
|
||||
eax_15 = DBObj::PreFetch(landblock|0xFFFF, 1)
|
||||
if (IN_MEMORY || IN_FILE) { eax_17 = DBObj::Get(...); if (eax_17) CLandBlock::PreFetchCells(eax_17) ... }
|
||||
```
|
||||
|
||||
`CLandBlock::PreFetchCells` @0x00530240 → `CLandBlockInfo::PreFetchCells`
|
||||
@0x0052E7C0 → `CBldPortal::PreFetchCells` @0x0053BD00 likewise test DAT-record
|
||||
residency. **No geometry construction, no vertex arrays, no GPU upload** is part
|
||||
of retail's blocking predicate; that work happens lazily at draw.
|
||||
|
||||
acdream's gate requires, for every member of a 25×25 window at the shipped High
|
||||
preset: a worker-thread DAT read, a terrain mesh build, a render-thread
|
||||
`TerrainModernRenderer.AddLandblock` upload
|
||||
(`src/AcDream.App/Rendering/TerrainModernRenderer.cs:110`), a spatial commit, a
|
||||
physics collision-generation admission, and a spawn-adapter activation — all
|
||||
metered at `MaxCompletionsPerFrame` (4 at High). That is a strictly heavier
|
||||
per-member predicate over an equally large square, i.e. the hold is
|
||||
systematically longer than retail's for identical content.
|
||||
|
||||
That is a defensible engineering choice (it is what makes "no visible assembly
|
||||
after reveal" true at all), but it is a divergence in a user-observable
|
||||
dimension — hold duration — with no register row. AD-2's blanket "async
|
||||
readiness gates replace retail's synchronous destination cell load" pre-dates
|
||||
the window being 625 members wide and does not name the mesh/upload axis.
|
||||
|
||||
**Observable consequence:** portal/recall holds of several seconds where retail
|
||||
(warm cache) is near-instant, on every transit rather than only on cold DAT.
|
||||
Nothing in the register or ISSUES predicts or bounds this.
|
||||
|
||||
---
|
||||
|
||||
## F4 — LOW — #326's cited address for `SmartBox::set_mid_radius` is wrong
|
||||
|
||||
`docs/ISSUES.md` #326 cites *"pushed into `SmartBox::set_mid_radius`
|
||||
@0x004531D0"*. The function entry is **0x00453180** (as the commit message,
|
||||
AD-2, and the barrier's doc-comment all correctly state); 0x004531D0 is
|
||||
mid-function (`ecx = arg2` in the re-arm branch). Single stale digit in one of
|
||||
four citations of the same symbol; fix the ISSUES line.
|
||||
|
||||
---
|
||||
|
||||
## F5 — LOW — "`Render::zfar` = 4000 fixed, never bounding the landscape" is true at default, false at Extreme
|
||||
|
||||
`Render::zfar` **is** byte-verified 4000.0f — VA 0x0081EC88 holds
|
||||
`00 00 7a 45` = 4000.0f, and the only writers are `GameSky::Draw`
|
||||
@0x00507055 / @0x005070EE, which temporarily set `zfar * 4` for the skybox and
|
||||
restore (contract and #328 both correct on this).
|
||||
|
||||
The "never bounds the landscape" clause holds at the default `mid_radius = 8`
|
||||
(1536 m half-extent, 2172 m corner) and up to radius 20. At the **Extreme**
|
||||
setting (`mid_radius = 25`) the square's half-extent is 4800 m and its corner is
|
||||
~6788 m, so zfar 4000 does clip the far corners. Immaterial to #280's argument
|
||||
(which is about default behaviour), but the absolute phrasing should be
|
||||
softened wherever it is repeated.
|
||||
|
||||
---
|
||||
|
||||
## F6 — LOW — gate/region map-edge bounds disagree (safe direction, but inconsistent)
|
||||
|
||||
`IsRenderNeighborhoodResident` and `PhysicsEngine.IsNeighborhoodTerrainResident`
|
||||
skip coordinates outside **0..254** (`StreamingController.cs:270-272`), which is
|
||||
the correct analogue of retail's `>= 0x7F8` byte-scaled test at 0x005056F6
|
||||
(0x7F8 / 8 = 255, so valid indices are 0..254 — verified).
|
||||
|
||||
`StreamingRegion` bounds-checks against **0..0xFF**
|
||||
(`src/AcDream.App/Streaming/StreamingRegion.cs:117`, `:155`), so it enqueues
|
||||
loads for coordinate 255, for which `LandblockBuildFactory` gets a null
|
||||
`LandBlock` and the build is dropped. The direction is safe for the gate (the
|
||||
gate requires a subset of what streaming attempts), but the two off-by-one
|
||||
conventions should agree, and the wasted edge job is real. Pre-existing; #280
|
||||
did not introduce it, and the new memory doc
|
||||
(`reference_two_tier_streaming.md`) documents only the gate's convention.
|
||||
|
||||
---
|
||||
|
||||
## F7 — INFO — the mid-hold re-radius claim holds only for outdoor load positions
|
||||
|
||||
The claim that retail's answer to a mid-hold radius change is "reset, re-radius,
|
||||
re-arm at the NEW value" is confirmed, with one condition worth recording.
|
||||
`LScape::SetMidRadius` @0x00504C00 is:
|
||||
|
||||
```c
|
||||
if (arg2 < 1 || this->land_blocks != 0) return 0;
|
||||
this->mid_radius = arg2;
|
||||
this->mid_width = (arg2 * 2) + 1;
|
||||
return 1;
|
||||
```
|
||||
|
||||
It **refuses** while `land_blocks` is allocated. `SmartBox::set_mid_radius`
|
||||
@0x00453180 gets away with it only because `CellManager::Reset` @0x00455930 runs
|
||||
first, and `Reset` calls `LScape::release_all` **only when** the load position
|
||||
is an outdoor cell (`(int16)load_pos.objcell_id < 0x100`) or the current cell has
|
||||
`seen_outside != 0`. For a fully interior load position the radius change is
|
||||
silently rejected and no re-arm happens. Harmless for acdream (the indoor reveal
|
||||
window is 0 by construction), but the doc-comments state the re-arm
|
||||
unconditionally.
|
||||
|
||||
Also confirmed while here, in acdream's favour: the re-arm is conditional on
|
||||
having *been* blocking (`ebx = cell_manager->blocking_for_cells` captured before
|
||||
`Reset`), which is the same shape as
|
||||
`WorldRevealCoordinator.ReconcileDestinationReservationRadius`'s
|
||||
`StreamingRegistered && !StreamingReleased` guard
|
||||
(`src/AcDream.App/Streaming/WorldRevealCoordinator.cs:512-541`).
|
||||
|
||||
---
|
||||
|
||||
## Answers to the four required questions
|
||||
|
||||
### Q1 — enum values, default, and the derived-window analogue
|
||||
|
||||
**Byte-verified.** `Render_LandscapeDrawDistance_Values` at VA 0x007CA988 (file
|
||||
0x3CA988) reads:
|
||||
|
||||
```
|
||||
03 00 00 00 05 00 00 00 08 00 00 00 0b 00 00 00 0f 00 00 00 19 00 00 00
|
||||
```
|
||||
|
||||
= `{3, 5, 8, 11, 15, 25}`. Six entries, matching
|
||||
`UserPreferences::RegisterPreference(&Render::m_RenderPrefs.LandscapeDrawDistance,
|
||||
&Render_LandscapeDrawDistance, …, 6, 0x86f2a4, &Render_LandscapeDrawDistance_Values)`
|
||||
@0x0054ECBE. Labels `VeryLow/Low/Medium/High/VeryHigh/Extreme`
|
||||
(0x006C363A ff.). **Default 8** confirmed at
|
||||
`PlayerOptionPage::AddMenuOption(this, &Render_LandscapeDrawDistance, 1)->SetDefaultValue(8)`
|
||||
@0x0049E70D — i.e. the "Medium" row.
|
||||
|
||||
The 1:1 "prefetch = loaded = drawn" assertion — the contract's load-bearing
|
||||
claim — **holds**:
|
||||
|
||||
- `LScape::SetMidRadius` @0x00504C00: `mid_width = mid_radius * 2 + 1`.
|
||||
- `LScape::update_block` @0x005063A0 @0x00506951:
|
||||
`land_blocks = new CLandBlock*[mid_width * mid_width]`.
|
||||
- `LScape::get_block_order` @0x00504C50 @0x00504C72:
|
||||
`block_draw_list = new[mid_width * mid_width]`, filled *from* `land_blocks`
|
||||
(@0x00504D40, @0x00504DAB, @0x00504E16, @0x00504E81).
|
||||
- `LScape::PreFetchCells` @0x00505660 iterates `-mid_radius .. +mid_radius` on
|
||||
both axes over that same square.
|
||||
- `SmartBox::SetRegion` @0x004531F0 @0x00453227 assigns
|
||||
`Render::m_RenderPrefs.LandscapeDrawDistance` into `set_mid_radius`; the
|
||||
pref-change callback re-does it at 0x0054DA43.
|
||||
|
||||
One number, one square, three roles. Retail cannot stream farther than it gates.
|
||||
|
||||
**Is acdream's derived window a faithful analogue?** Structurally yes, with one
|
||||
caveat worth stating. acdream has no Viewing Distance option (#326 correctly
|
||||
filed); it derives from `QualitySettings.FarRadius`
|
||||
(`src/AcDream.UI.Abstractions/Settings/QualityPreset.cs:31-34`, ladder
|
||||
5 / 8 / 12 / 15 for Low / Medium / High / Ultra, default High = 12). Two notes:
|
||||
|
||||
- The ladders are not the same set — retail's `{3,5,8,11,15,25}` vs acdream's
|
||||
`{5,8,12,15}` — and the *defaults* differ materially: retail 8 (17×17),
|
||||
acdream 12 (25×25). acdream's default gate is therefore ~2.2× retail's in
|
||||
area. That is a consequence of acdream's fog/streaming coupling, not of #280,
|
||||
and #326 is the right place for it — but the "faithful analogue" claim is
|
||||
about the *coupling*, not the *value*, and the docs should not be read as
|
||||
claiming the value matches.
|
||||
- `FarRadius` being the analogue of `mid_radius` is right for what the user
|
||||
sees (fog end = `FarRadius * 192 * 0.95`,
|
||||
`src/AcDream.App/Rendering/WorldRenderFrameBuilder.cs:484`).
|
||||
|
||||
### Q2 — does retail block, and is a longer hold retail-convergent?
|
||||
|
||||
**Retail blocks, hard — verified.** `SmartBox::UseTime` @0x00455410:
|
||||
|
||||
```c
|
||||
if (cell_manager->blocking_for_cells == 0) {
|
||||
... CheckPrefetchStatus / UpdateLoadPoint / ChangePosition
|
||||
CObjectMaint::UseTime; CPhysics::UseTime; GameTime::UseTime;
|
||||
LScape::UseTime; Ambient::UseTime;
|
||||
} else {
|
||||
CellManager::CheckPrefetchStatus(cell_manager); // and nothing else
|
||||
}
|
||||
```
|
||||
|
||||
While `blocking_for_cells` is latched the entire simulation — object
|
||||
maintenance, physics, game clock, landscape, ambient — is skipped. Only
|
||||
`SceneTool::Think()` and the queue drain still run.
|
||||
`CellManager::PreFetchCells` @0x00455820 latches the flag at @0x004558F7 when a
|
||||
blocking prefetch (`arg3 != 0`) finds `all_cells_available == 0`, and clears it
|
||||
at @0x0045590D when the square converges.
|
||||
|
||||
So **a longer hold is retail-convergent in kind.** #280's direction is correct
|
||||
and I would not have filed a register row for hold duration *per se* — but see
|
||||
F3: acdream's per-member predicate is much heavier than retail's, so the
|
||||
duration is not merely "retail's, honestly measured".
|
||||
|
||||
**What retail shows while blocked.** Two things, and acdream has neither
|
||||
correctly:
|
||||
|
||||
1. `ECM_DDD::SendNotice_RuntimeDDDStatus(1, remaining, total)` at
|
||||
`CellManager::PreFetchCells` @0x004558DE — a live "N of M cells" progress
|
||||
readout, cleared with `(0,0,0)` at @0x00455910 / @0x00455994. **#327 filed
|
||||
for this, correctly.**
|
||||
2. If the block coincides with a teleport, the portal tunnel and the
|
||||
"In Portal Space - Please Wait..." notice — but, per **F2**, that notice is
|
||||
driven by the tunnel rotation segment (0.6–1.8 s, unconditional), not by the
|
||||
block, and acdream's 5 s arming threshold is not retail's trigger. The
|
||||
contract is right to say the 5 s threshold is not retail's; the commit
|
||||
message then contradicts it.
|
||||
|
||||
### Q3 — is AP-149 honest and correctly scoped?
|
||||
|
||||
**Yes, and its retail chain is exactly right.** I verified every link:
|
||||
|
||||
- `LScape::PreFetchCells` @0x00505660 walks the whole square and, per member,
|
||||
requires the terrain DBObj resident (`DBObj::Get` non-null, @0x0050575C); on
|
||||
in-file-but-not-loaded it kicks a prefetch of the type-2 LandBlockInfo record
|
||||
(`(esi & 0xfffffffe) | 0xfffe`, @0x0050579C) and reports not-ready.
|
||||
- `CLandBlock::PreFetchCells` @0x00530240 requires the LandBlockInfo record when
|
||||
`lbi_exists`.
|
||||
- `CLandBlockInfo::PreFetchCells` @0x0052E7C0 loops every building and every one
|
||||
of its portals into `CBldPortal::PreFetchCells` @0x0053BD00.
|
||||
|
||||
And acdream's Far build is genuinely heightmap-only:
|
||||
`LandblockBuildFactory.BuildLocked` (`src/AcDream.App/Streaming/LandblockBuildFactory.cs:129-141`)
|
||||
early-outs for `LoadFar` with `Array.Empty<WorldEntity>()` and
|
||||
`PhysicsDatBundle.Empty`, skipping LandBlockInfo, scenery, buildings, and
|
||||
interior cells. The row's claim list is accurate, its risk column names the
|
||||
right symptom (distant buildings/scenery popping in after reveal), and its
|
||||
"do not let a later closeout claim parity" line is the right guard.
|
||||
|
||||
**Is it larger than the row admits?** Two qualifications, neither fatal:
|
||||
|
||||
- The row is scoped to the *outer* ring. Correct today. But it does not say that
|
||||
the boundary between "retail-complete" and "terrain-only" is `NearRadius`,
|
||||
which at High is 4 (768 m) against retail's uniform 8 (1536 m at default) —
|
||||
so acdream's fully-hydrated square is *smaller* than retail's entire prefetch
|
||||
square, not just its inner part. The stated ~768 m threshold in the row's Risk
|
||||
column captures this numerically; the framing ("outer ring") slightly
|
||||
understates that retail has no inner/outer distinction at all.
|
||||
- The row does not mention that the same terrain-only outer ring is what makes
|
||||
F1's demote hole reachable. That is a defect, not a divergence, so it belongs
|
||||
in ISSUES rather than the register — but AP-149 currently reads as if the
|
||||
outer arm works and is merely weaker. It does not work.
|
||||
|
||||
### Q4 — anything retail-visible broken or silently altered
|
||||
|
||||
- **F1** — reveal can hang permanently. Retail-visible in the strongest sense.
|
||||
- **F2** — no behaviour change from this commit, but the wait cue is now shown
|
||||
in more situations and its arming remains non-retail.
|
||||
- `preferDestination` latching (F1 secondary) starves non-destination streaming
|
||||
to 25% for the session.
|
||||
- Indoor destinations are unchanged: `RequiredWindow` returns `(0,0)`
|
||||
(`WorldRevealReadinessBarrier.cs:206-217`), and `IsRenderNeighborhoodResident(cell, 0, 0)`
|
||||
reduces to the pre-#280 `IsNearTier && IsRenderReady` on the single member.
|
||||
Verified by inspection; retail's indoor arm is `CEnvCell::PreFetchCells`
|
||||
@0x0052D1E0 (the id-taking overload — address correct as cited).
|
||||
- Composite warmup staying `NearRadius`-scoped is sound: `LandblockBuildFactory`
|
||||
gives Far builds no entities at all, so widening it would walk 625 landblocks
|
||||
to warm nothing.
|
||||
- The `ACDREAM_PROBE_REVEAL_RADIUS=1` A/B is faithful: it forces `far = 1` and
|
||||
`near = clamp(NearRadius, 0, 1) = 1`, which is exactly the pre-#280
|
||||
`IsNearTier && IsRenderReady` radius-1 gate.
|
||||
- The D6 scratch-set change (`PhysicsEngine.cs:48-52`, `:137-147`) is correct
|
||||
under the stated single-thread assumption; the method is a pure leaf and the
|
||||
set is engine-instance-owned, and the staging clone in
|
||||
`CollisionStagingBuilder` is a distinct `PhysicsEngine` with its own scratch.
|
||||
I did not find a concurrent caller.
|
||||
|
||||
---
|
||||
|
||||
## Bookkeeping audit
|
||||
|
||||
| Item | Verdict |
|
||||
|---|---|
|
||||
| **AP-149** (new) | Present at register line 178. Retail chain verified end-to-end (all four addresses correct). Honest and correctly directional. Caveats in Q3. |
|
||||
| **AD-2** amendment | Factually correct on every retail claim I checked: `{3,5,8,11,15,25}` @0x007CA988, default 8, `SmartBox::SetRegion` @0x004531F0, `LScape::PreFetchCells` @0x00505660, `SmartBox::set_mid_radius` @0x00453180, `LScape::SetMidRadius` @0x00504C00. The sentence *"out to `FarRadius`, terrain publication only (`IsRenderReady`, which a `PublicationKind.Far` landblock satisfies through its empty spawn-adapter registration)"* is true as written and **false for the demote-produced Far tier** — this is the register's statement of F1's wrong assumption and must be corrected with the fix. |
|
||||
| **#326** (Viewing Distance option) | Correctly filed and well-scoped. One wrong address (F4). |
|
||||
| **#327** (DDD progress readout) | Correctly filed; `ECM_DDD::SendNotice_RuntimeDDDStatus` confirmed live at `CellManager::PreFetchCells` @0x004558DE. |
|
||||
| **#328** (5000 f far plane vs retail 4000) | Correctly filed; `Render::zfar = 4000.0f` byte-verified at VA 0x0081EC88 (`00 00 7a 45`). See F5 on the "never bounds" phrasing. |
|
||||
| **CLAUDE.md `ACDREAM_STREAM_RADIUS` rewrite** | Verified against source and correct on all four clauses: default unset (`QualityPreset.WithEnvOverrides` / `RuntimeOptions.LegacyStreamRadius`), forces `NearRadius` and only raises `FarRadius` (`SessionPlayerComposition.cs:249-257`), silently discarded by `RuntimeSettingsTargets.ApplyQuality` → `ReconfigureStreamingRadii` (`RuntimeSettingsTargets.cs:251-252`), and `ACDREAM_NEAR_RADIUS`/`ACDREAM_FAR_RADIUS` are the modern spelling (`QualityPreset.cs:45-46`). |
|
||||
| **`reference_two_tier_streaming.md` corrections** | Verified correct, including the load-bearing one: a Far publication *does* reach `LandblockPhysicsPublisher` (no `Kind != Far` guard in `LandblockPresentationPipeline.Advance`'s physics arm, `:600-660`) and terrain collision is published from the heightmap, which `PhysicsEngine.DemoteLandblockToTerrain` is explicitly written to preserve. The preset table, the Chebyshev note, and the retail `mid_radius` paragraph are all accurate. Its "`IsRenderReady` … a Far publication registers with `WantsLoaded = true` and an EMPTY desired mesh set, so it is render-ready" bullet inherits F1's error and needs the same correction. |
|
||||
| **Left unfiled** | (a) F1 — no issue exists for the demote hole. (b) F2 — the 5 s wait-cue arming has no divergence row; AD-2/AP-115 describe it as acdream behaviour without naming retail's actual trigger. (c) F3 — the mesh-build/GPU-upload strictness of the gate versus retail's DAT-residency predicate has no row. |
|
||||
|
||||
---
|
||||
|
||||
## Gates run
|
||||
|
||||
- `dotnet build -c Release AcDream.slnx` → **0 errors**, 18 warnings (all pre-existing xUnit analyzer warnings).
|
||||
- `dotnet test -c Release AcDream.slnx --no-build` → 3 failures, **all three from a
|
||||
previous review session's deleted probe file** still present in the stale
|
||||
`AcDream.App.Tests.dll`. After
|
||||
`dotnet build -c Release tests/AcDream.App.Tests --no-incremental`:
|
||||
**App.Tests 4,157 passed / 0 failed / 3 skipped**. Other assemblies observed
|
||||
green in the same run: Runtime 1,217, Core 4,263 / 1 skipped, Core.Net 764,
|
||||
Headless 89, Bake 15. (My solution-wide invocation piped through `tail`, so I
|
||||
do not have reliable totals for Cli/Content/UI.Abstractions; nothing failed in
|
||||
the captured portion.)
|
||||
- Binary verification: `py tools/pdb-extract/check_exe_pdb.py
|
||||
"C:/Users/erikn/Downloads/acclient.exe"` → `=== MATCH ===`, GUID
|
||||
`9e847e2f-777c-4bd9-886c-22256bb87f32`, linker UTC 2013-09-06T00:17:56.
|
||||
Raw byte reads at VA 0x007CA988, 0x007991B0, 0x0081EC88, 0x007BD6A8 and code
|
||||
bytes 0x004D6FC0-0x004D7070 taken directly from the PE via section-mapped file
|
||||
offsets.
|
||||
|
||||
## What I checked and found clean
|
||||
|
||||
So the PASS portions are auditable, these were examined and found correct:
|
||||
retail's six-value ladder and default; the one-square prefetch/loaded/drawn
|
||||
identity; the `>= 0x7F8` bounds test and acdream's matching skip; the blocking
|
||||
`SmartBox::UseTime` arm; the 5.0 s `CheckPrefetchStatus` constant and its
|
||||
comparison sense; the `Render::zfar` initialiser and its only two writers; the
|
||||
wait-cue string bytes; `LScape::SetMidRadius`'s `mid_width` formula and its
|
||||
`land_blocks` refusal; `SmartBox::set_mid_radius`'s save-reset-re-radius-re-arm
|
||||
order; `CellManager::Reset`'s conditional `release_all`; the full
|
||||
`CLandBlock` → `CLandBlockInfo` → `CBldPortal` prefetch chain; acdream's Far
|
||||
build contents; that a Far publication reaches the physics publisher and
|
||||
registers terrain collision; that terrain mesh upload precedes spawn-adapter
|
||||
registration so `IsRenderReady` is not a stamp for freshly-loaded Far tier; the
|
||||
indoor arm's unchanged behaviour; the composite-warmup scoping argument; the
|
||||
`ACDREAM_PROBE_REVEAL_RADIUS=1` A/B equivalence; the D6 scratch-set safety; the
|
||||
origin/destination coincidence that keeps the gate square inside the streaming
|
||||
square after a recentring teleport; and every documentation claim listed in the
|
||||
bookkeeping table.
|
||||
|
|
@ -1556,8 +1556,62 @@ public sealed class GpuWorldState : ILiveEntitySpatialQuery
|
|||
}
|
||||
}
|
||||
|
||||
internal void ReleaseLandblockMeshReferences(uint landblockId) =>
|
||||
_wbSpawnAdapter?.OnLandblockUnloaded(landblockId);
|
||||
/// <summary>
|
||||
/// Reconciles the spawn-adapter registration to the landblock's state
|
||||
/// AFTER a retirement has committed, which is not the same thing as
|
||||
/// "unregister".
|
||||
///
|
||||
/// <para>
|
||||
/// A <b>full</b> retirement removes the landblock from <c>_loaded</c> and
|
||||
/// from <c>_tierByLandblock</c>, so releasing every mesh reference and
|
||||
/// dropping the registration is the whole job.
|
||||
/// </para>
|
||||
///
|
||||
/// <para>
|
||||
/// A <b>Near-layer</b> retirement (the Near→Far demote) is different:
|
||||
/// <see cref="DetachNearLayer"/> keeps the landblock loaded, keeps its
|
||||
/// terrain mesh uploaded and drawn, keeps its terrain collision resident
|
||||
/// (<c>PhysicsEngine.DemoteLandblockToTerrain</c>), and flips its tier to
|
||||
/// <see cref="LandblockStreamTier.Far"/>. Only the Near entity layer's
|
||||
/// mesh references go away. Releasing them leaves
|
||||
/// <c>LandblockSpawnAdapter.WantsLoaded == false</c>, so
|
||||
/// <see cref="IsRenderReady"/> would report false for a landblock that is
|
||||
/// on screen — and nothing ever re-publishes an already-loaded landblock,
|
||||
/// so it would stay false for the rest of the window's life.
|
||||
/// </para>
|
||||
///
|
||||
/// <para>
|
||||
/// Re-asserting the empty registration here is what makes the two ways of
|
||||
/// reaching Far tier equivalent: a demoted landblock ends up with exactly
|
||||
/// the registration a <c>PublicationKind.Far</c> activation installs
|
||||
/// (<c>WantsLoaded = true</c>, no ordinary or prepared references, because
|
||||
/// the retained entity list holds only live server projections, which the
|
||||
/// adapter's atlas-tier filter skips). #280's reveal gate — and every
|
||||
/// future <see cref="IsRenderReady"/> caller — can then read the predicate
|
||||
/// as "drawable at its current tier" without needing to know how the
|
||||
/// landblock got there.
|
||||
/// </para>
|
||||
/// </summary>
|
||||
internal void ReleaseLandblockMeshReferences(uint landblockId)
|
||||
{
|
||||
if (_wbSpawnAdapter is null)
|
||||
return;
|
||||
|
||||
// A throwing release stays retryable: the re-assertion below is only
|
||||
// reached once the adapter has actually converged.
|
||||
_wbSpawnAdapter.OnLandblockUnloaded(landblockId);
|
||||
|
||||
uint canonical = (landblockId & 0xFFFF0000u) | 0xFFFFu;
|
||||
if (!_loaded.TryGetValue(canonical, out LoadedLandblock? retained))
|
||||
return;
|
||||
if (!_tierByLandblock.TryGetValue(canonical, out LandblockStreamTier tier)
|
||||
|| tier != LandblockStreamTier.Far)
|
||||
{
|
||||
return;
|
||||
}
|
||||
|
||||
_wbSpawnAdapter.OnLandblockLoaded(retained);
|
||||
}
|
||||
|
||||
internal void InvalidateLandblockClassification(uint landblockId) =>
|
||||
_onLandblockUnloaded?.Invoke(landblockId);
|
||||
|
|
|
|||
|
|
@ -268,9 +268,15 @@ public sealed class StreamingController
|
|||
continue;
|
||||
|
||||
uint canonical = ((uint)nx << 24) | ((uint)ny << 16) | 0xFFFFu;
|
||||
// GpuWorldState.IsRenderReady already implies IsLoaded; a Far-tier
|
||||
// publication registers with the spawn adapter and an empty mesh
|
||||
// set, so this is a real drawability test out there, not a stamp.
|
||||
// GpuWorldState.IsRenderReady already implies IsLoaded, and it is a
|
||||
// real drawability test out here rather than a stamp: a Far-tier
|
||||
// landblock carries a spawn-adapter registration with an empty mesh
|
||||
// set, installed after its terrain upload crossed the render-thread
|
||||
// barrier. Both routes to Far tier install it — a PublicationKind.Far
|
||||
// activation and a Near->Far demote (GpuWorldState
|
||||
// .ReleaseLandblockMeshReferences re-asserts it after retiring the
|
||||
// Near layer). If those two ever diverge again, this gate becomes
|
||||
// unsatisfiable for the whole life of the streaming window.
|
||||
if (!_state.IsRenderReady(canonical))
|
||||
return false;
|
||||
bool isInnerRing = Math.Abs(dx) <= nearRadius
|
||||
|
|
|
|||
|
|
@ -51,6 +51,14 @@ internal static class StreamingDiagnostics
|
|||
far);
|
||||
}
|
||||
|
||||
private static int? ParseRadius(string? raw) =>
|
||||
int.TryParse(raw, out int value) && value >= 0 ? value : null;
|
||||
/// <summary>
|
||||
/// The floor is 1, not 0. An outdoor destination's acknowledgement must
|
||||
/// carry <c>RequiredRenderRadius >= 1</c> or
|
||||
/// <c>RuntimeWorldTransitState.AcknowledgeDestinationReadiness</c> fails
|
||||
/// <c>invalid-readiness-shape</c> on every frame — so accepting 0 here
|
||||
/// would hang the very A/B route this probe exists to measure. Reject it
|
||||
/// at the parser rather than let the two halves of one commit disagree.
|
||||
/// </summary>
|
||||
internal static int? ParseRadius(string? raw) =>
|
||||
int.TryParse(raw, out int value) && value >= 1 ? value : null;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -109,8 +109,29 @@ internal sealed class WorldRevealReadinessBarrier
|
|||
public void Begin() => _invalidateCompositeTextures();
|
||||
|
||||
/// <summary>
|
||||
/// Advances render-thread texture preparation after all required static
|
||||
/// meshes for the destination neighborhood have been published.
|
||||
/// Advances render-thread texture preparation once the static meshes the
|
||||
/// composite domain covers have been published.
|
||||
///
|
||||
/// <para>
|
||||
/// D3: the composite domain is entity-scoped
|
||||
/// (<c>WbDrawDispatcher.IsCompositeWarmupCandidate</c> filters entities by
|
||||
/// Chebyshev landblock radius) and Far-tier builds carry no entities at all
|
||||
/// (<c>LandblockBuildFactory</c>), so the honest composite domain is the
|
||||
/// NEAR radius. Widening it over Far rings would walk the whole outer
|
||||
/// window to warm nothing.
|
||||
/// </para>
|
||||
///
|
||||
/// <para>
|
||||
/// The TRIGGER is therefore scoped the same way. Pre-#280 the gate and the
|
||||
/// composite domain were the same radius-1 square, so gating warmup on the
|
||||
/// whole gate cost nothing. #280 widened the gate to the entire Far window
|
||||
/// without widening the domain; keeping the old trigger would have pushed
|
||||
/// every composite upload behind the last outer-ring landblock and made
|
||||
/// the hold longer than the streaming work alone requires. Warming as soon
|
||||
/// as the Near sub-window is published restores the overlap. The reveal
|
||||
/// gate itself is untouched: <see cref="Evaluate"/> still requires the
|
||||
/// full window AND composite readiness.
|
||||
/// </para>
|
||||
/// </summary>
|
||||
public void Prepare(uint destinationCell)
|
||||
{
|
||||
|
|
@ -121,14 +142,8 @@ internal sealed class WorldRevealReadinessBarrier
|
|||
if (_isRenderNeighborhoodReady(
|
||||
destinationCell,
|
||||
required.NearRadius,
|
||||
required.FarRadius))
|
||||
required.NearRadius))
|
||||
{
|
||||
// D3: the composite domain is entity-scoped
|
||||
// (WbDrawDispatcher.IsCompositeWarmupCandidate filters entities by
|
||||
// Chebyshev landblock radius) and Far-tier builds carry no
|
||||
// entities at all (LandblockBuildFactory), so the honest composite
|
||||
// domain is the NEAR radius. Widening it over Far rings would walk
|
||||
// the whole outer window to warm nothing.
|
||||
_prepareCompositeTextures(destinationCell, required.NearRadius);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -565,9 +565,13 @@ public sealed class StreamingControllerReadinessTests
|
|||
publishBeforeSpatialCommit: (_, _) => { },
|
||||
state);
|
||||
|
||||
// A Near-shaped completion that the streaming window has since demoted
|
||||
// to Far: entities and physics payload are stripped by PublishAsFar,
|
||||
// which is exactly the shape an outer-ring landblock is published in.
|
||||
// A Near-shaped completion the streaming window has DOWNGRADED before
|
||||
// publication: entities and physics payload are stripped by
|
||||
// PublishAsFar, which is exactly the shape an outer-ring landblock
|
||||
// ARRIVES in. This is not the demote transition — a landblock that was
|
||||
// already published Near and is later retired to Far takes a different
|
||||
// route (LandblockPresentationPipeline.BeginNearLayerRetirement) and is
|
||||
// covered separately below.
|
||||
var entity = new WorldEntity
|
||||
{
|
||||
Id = 1,
|
||||
|
|
@ -606,6 +610,178 @@ public sealed class StreamingControllerReadinessTests
|
|||
controller.IsRenderNeighborhoodResident(landblockId, 0, 0));
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// #280 D-1 regression, the OTHER way a landblock reaches Far tier. A
|
||||
/// Near→Far demote retires the Near mesh layer while
|
||||
/// <c>GpuWorldState.DetachNearLayer</c> keeps the landblock loaded,
|
||||
/// terrain-resident and drawn. Before the fix the retirement's
|
||||
/// MeshReferences stage left <c>WantsLoaded == false</c> forever — nothing
|
||||
/// re-publishes an already-loaded landblock — so the demoted block
|
||||
/// satisfied NEITHER arm of the widened reveal gate and the client hung in
|
||||
/// portal space until relog.
|
||||
///
|
||||
/// <para>
|
||||
/// Driven through the real <see cref="GpuWorldState"/>, the real
|
||||
/// <see cref="LandblockSpawnAdapter"/> and the real
|
||||
/// <see cref="LandblockPresentationPipeline"/>, which is the production
|
||||
/// route <c>StreamingController.DemoteLandblock</c> takes.
|
||||
/// </para>
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void NearToFarDemote_LeavesTheLandblockRenderReadyThroughTheRealPipeline()
|
||||
{
|
||||
const uint landblockId = 0x1236FFFFu;
|
||||
const ulong gfxObjId = 0x01000010ul;
|
||||
var meshes = new ReadinessMeshAdapter();
|
||||
var state = new GpuWorldState(new LandblockSpawnAdapter(meshes));
|
||||
var pipeline = new LandblockPresentationPipeline(
|
||||
publishBeforeSpatialCommit: (_, _) => { },
|
||||
state);
|
||||
meshes.ReadyIds.Add(gfxObjId);
|
||||
|
||||
var entity = new WorldEntity
|
||||
{
|
||||
Id = 1,
|
||||
ServerGuid = 0,
|
||||
SourceGfxObjOrSetupId = (uint)gfxObjId,
|
||||
Position = System.Numerics.Vector3.Zero,
|
||||
Rotation = System.Numerics.Quaternion.Identity,
|
||||
MeshRefs = [new MeshRef((uint)gfxObjId, System.Numerics.Matrix4x4.Identity)],
|
||||
};
|
||||
state.AddLandblock(
|
||||
new LoadedLandblock(landblockId, new LandBlock(), new[] { entity }));
|
||||
|
||||
Assert.True(state.IsNearTier(landblockId));
|
||||
Assert.True(state.IsRenderReady(landblockId));
|
||||
Assert.Equal(1, meshes.ReferenceCounts[gfxObjId]);
|
||||
|
||||
pipeline.BeginNearLayerRetirement(landblockId);
|
||||
|
||||
// The demote really happened: Near layer gone, mesh reference released.
|
||||
Assert.False(state.IsNearTier(landblockId));
|
||||
Assert.DoesNotContain(gfxObjId, meshes.ReferenceCounts.Keys);
|
||||
// ...and the landblock is still loaded and still drawn.
|
||||
Assert.True(state.IsLoaded(landblockId));
|
||||
// The demoted block must now be indistinguishable from one that
|
||||
// arrived as Far: registered, empty desired set, render-ready.
|
||||
Assert.True(state.IsRenderReady(landblockId));
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// #280 D-1, at the gate rather than at the predicate. A full Near window
|
||||
/// satisfies the tiered gate; demoting one OUTER-ring member — which is
|
||||
/// what an ordinary region recenter or a mid-hold quality drop does — must
|
||||
/// not make the gate permanently unsatisfiable.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void TieredWindow_StaysResidentAfterAnOuterRingDemote()
|
||||
{
|
||||
var meshes = new ReadinessMeshAdapter();
|
||||
var state = new GpuWorldState(new LandblockSpawnAdapter(meshes));
|
||||
var pipeline = new LandblockPresentationPipeline(
|
||||
publishBeforeSpatialCommit: (_, _) => { },
|
||||
state);
|
||||
StreamingController controller = CreateController(state);
|
||||
|
||||
for (int dx = -2; dx <= 2; dx++)
|
||||
for (int dy = -2; dy <= 2; dy++)
|
||||
AddPublished(state, 0x12 + dx, 0x36 + dy);
|
||||
|
||||
Assert.True(controller.IsRenderNeighborhoodResident(0x12360022u, 1, 2));
|
||||
|
||||
// Chebyshev distance 2 from the destination: inside the gate's far
|
||||
// window, outside its near ring — the exact band a demote can land in.
|
||||
pipeline.BeginNearLayerRetirement(0x1434FFFFu);
|
||||
|
||||
Assert.True(state.IsLoaded(0x1434FFFFu));
|
||||
Assert.False(state.IsNearTier(0x1434FFFFu));
|
||||
Assert.True(controller.IsRenderNeighborhoodResident(0x12360022u, 1, 2));
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// The same transition through the METERED retirement coordinator
|
||||
/// production actually composes (<c>CreateBudgeted</c>), whose
|
||||
/// MeshReferences stage is a separate call site from the legacy pipeline's.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void NearToFarDemote_LeavesTheLandblockRenderReadyUnderBudgetedRetirement()
|
||||
{
|
||||
const uint landblockId = 0x1236FFFFu;
|
||||
const ulong gfxObjId = 0x01000010ul;
|
||||
var meshes = new ReadinessMeshAdapter();
|
||||
var state = new GpuWorldState(new LandblockSpawnAdapter(meshes));
|
||||
meshes.ReadyIds.Add(gfxObjId);
|
||||
var entity = new WorldEntity
|
||||
{
|
||||
Id = 1,
|
||||
ServerGuid = 0,
|
||||
SourceGfxObjOrSetupId = (uint)gfxObjId,
|
||||
Position = System.Numerics.Vector3.Zero,
|
||||
Rotation = System.Numerics.Quaternion.Identity,
|
||||
MeshRefs = [new MeshRef((uint)gfxObjId, System.Numerics.Matrix4x4.Identity)],
|
||||
};
|
||||
state.AddLandblock(
|
||||
new LoadedLandblock(landblockId, new LandBlock(), new[] { entity }));
|
||||
|
||||
LandblockRetirementCoordinator coordinator =
|
||||
LandblockRetirementCoordinator.CreateBudgeted(
|
||||
state,
|
||||
AdvanceNoopPresentationStep,
|
||||
static ticket =>
|
||||
{
|
||||
while (AdvanceNoopPresentationStep(ticket)
|
||||
!= LandblockRetirementOperationResult.NoWork)
|
||||
{
|
||||
}
|
||||
});
|
||||
coordinator.BeginNearLayer(landblockId);
|
||||
|
||||
int frames = 0;
|
||||
while (coordinator.PendingCount != 0 && frames++ < 64)
|
||||
{
|
||||
var meter = new StreamingWorkMeter(new StreamingWorkBudget(
|
||||
maxUpdateTime: TimeSpan.FromSeconds(1),
|
||||
maxCompletionAdmissions: 64,
|
||||
maxAdoptedCpuBytes: 64 * 1024 * 1024,
|
||||
maxEntityOperations: 64,
|
||||
maxGpuUploadBytes: 64 * 1024 * 1024,
|
||||
maxGlRetireOperations: 64,
|
||||
destinationReserveFraction: 0.75f));
|
||||
coordinator.Advance(meter);
|
||||
meter.FinishFrame();
|
||||
}
|
||||
|
||||
Assert.Equal(0, coordinator.PendingCount);
|
||||
Assert.True(state.IsLoaded(landblockId));
|
||||
Assert.False(state.IsNearTier(landblockId));
|
||||
Assert.DoesNotContain(gfxObjId, meshes.ReferenceCounts.Keys);
|
||||
Assert.True(state.IsRenderReady(landblockId));
|
||||
}
|
||||
|
||||
private static LandblockRetirementOperationResult AdvanceNoopPresentationStep(
|
||||
LandblockRetirementTicket ticket) =>
|
||||
ticket.NextIncompleteStage switch
|
||||
{
|
||||
LandblockRetirementStage.EntityLighting
|
||||
or LandblockRetirementStage.EntityTranslucency =>
|
||||
ticket.RunEntityStep(
|
||||
ticket.NextIncompleteStage,
|
||||
static _ => true,
|
||||
static _ => { }),
|
||||
LandblockRetirementStage.PluginProjection =>
|
||||
ticket.RunEntityStep(
|
||||
LandblockRetirementStage.PluginProjection,
|
||||
static entity => entity.ServerGuid == 0,
|
||||
static _ => { }),
|
||||
LandblockRetirementStage.Terrain
|
||||
or LandblockRetirementStage.Physics
|
||||
or LandblockRetirementStage.CellVisibility
|
||||
or LandblockRetirementStage.BuildingRegistry
|
||||
or LandblockRetirementStage.EnvironmentCells =>
|
||||
ticket.RunOnceStep(ticket.NextIncompleteStage, static () => { }),
|
||||
_ => LandblockRetirementOperationResult.NoWork,
|
||||
};
|
||||
|
||||
private static StreamingController CreateController(GpuWorldState state)
|
||||
=> new(
|
||||
(_, _) => { },
|
||||
|
|
|
|||
|
|
@ -1,3 +1,4 @@
|
|||
using AcDream.App.Rendering.Wb;
|
||||
using AcDream.App.Streaming;
|
||||
using AcDream.Core.Physics;
|
||||
using AcDream.Core.World;
|
||||
|
|
@ -36,7 +37,7 @@ public sealed class WorldRevealDerivedWindowIntegrationTests
|
|||
int nearRadius,
|
||||
int farRadius)
|
||||
{
|
||||
var world = new GpuWorldState();
|
||||
GpuWorldState world = CreateWorld();
|
||||
var physics = new PhysicsEngine();
|
||||
var transit = new RuntimeWorldTransitState();
|
||||
var streaming = new RecordingReservations();
|
||||
|
|
@ -94,7 +95,7 @@ public sealed class WorldRevealDerivedWindowIntegrationTests
|
|||
{
|
||||
const int nearRadius = 1;
|
||||
const int farRadius = 3;
|
||||
var world = new GpuWorldState();
|
||||
GpuWorldState world = CreateWorld();
|
||||
var physics = new PhysicsEngine();
|
||||
var transit = new RuntimeWorldTransitState();
|
||||
StreamingController controller = CreateController(
|
||||
|
|
@ -137,7 +138,7 @@ public sealed class WorldRevealDerivedWindowIntegrationTests
|
|||
const uint indoorCell = (uint)CenterX << 24
|
||||
| (uint)CenterY << 16
|
||||
| 0x0100u;
|
||||
var world = new GpuWorldState();
|
||||
GpuWorldState world = CreateWorld();
|
||||
var physics = new PhysicsEngine();
|
||||
var transit = new RuntimeWorldTransitState();
|
||||
StreamingController controller = CreateController(world, 2, 6);
|
||||
|
|
@ -159,6 +160,78 @@ public sealed class WorldRevealDerivedWindowIntegrationTests
|
|||
Assert.Equal(0, transit.Snapshot.InvariantFailureCount);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// #280 D-1, end to end. The reveal gate opens on a fully published
|
||||
/// window, then a Near→Far demote lands on an outer-ring member —
|
||||
/// the ordinary outcome of a region recenter that does not move the world
|
||||
/// origin, and of a mid-hold quality-preset drop. Before the fix, the
|
||||
/// demoted member could never become <c>IsRenderReady</c> again, so the
|
||||
/// coordinator never returned <c>IsReady</c> and the client stayed in
|
||||
/// portal space until relog.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void OutdoorReveal_SurvivesAnOuterRingDemoteDuringTheHold()
|
||||
{
|
||||
const int nearRadius = 1;
|
||||
const int farRadius = 3;
|
||||
var meshes = new RecordingMeshAdapter();
|
||||
var world = new GpuWorldState(new LandblockSpawnAdapter(meshes));
|
||||
var physics = new PhysicsEngine();
|
||||
var transit = new RuntimeWorldTransitState();
|
||||
var pipeline = new LandblockPresentationPipeline(
|
||||
publishBeforeSpatialCommit: (_, _) => { },
|
||||
world);
|
||||
StreamingController controller = CreateController(
|
||||
world,
|
||||
nearRadius,
|
||||
farRadius);
|
||||
WorldRevealCoordinator coordinator = CreateCoordinator(
|
||||
transit,
|
||||
controller,
|
||||
physics,
|
||||
streaming: null);
|
||||
|
||||
coordinator.BeginLogin(DestinationCell);
|
||||
// Publish the WHOLE window at Near tier, entities included. That is
|
||||
// what a region centred on the destination actually produces before it
|
||||
// starts trimming its trailing edge.
|
||||
for (int radius = 0; radius <= farRadius; radius++)
|
||||
PublishRing(world, physics, radius, LandblockStreamTier.Near, withEntity: true);
|
||||
|
||||
Assert.True(coordinator.Evaluate(DestinationCell).IsReady);
|
||||
|
||||
// A single outer-ring member demotes. Chebyshev 3: inside the far
|
||||
// window, outside the near ring.
|
||||
uint demoted = ((uint)(CenterX + farRadius) << 24)
|
||||
| ((uint)(CenterY - farRadius) << 16)
|
||||
| 0xFFFFu;
|
||||
pipeline.BeginNearLayerRetirement(demoted);
|
||||
|
||||
Assert.True(world.IsLoaded(demoted));
|
||||
Assert.False(world.IsNearTier(demoted));
|
||||
Assert.True(coordinator.Evaluate(DestinationCell).IsReady);
|
||||
Assert.Equal(0, transit.Snapshot.InvariantFailureCount);
|
||||
}
|
||||
|
||||
private sealed class RecordingMeshAdapter : IWbMeshAdapter
|
||||
{
|
||||
public Dictionary<ulong, int> ReferenceCounts { get; } = new();
|
||||
|
||||
public void IncrementRefCount(ulong id) =>
|
||||
ReferenceCounts[id] = ReferenceCounts.GetValueOrDefault(id) + 1;
|
||||
|
||||
public void DecrementRefCount(ulong id)
|
||||
{
|
||||
int next = ReferenceCounts.GetValueOrDefault(id) - 1;
|
||||
if (next <= 0)
|
||||
ReferenceCounts.Remove(id);
|
||||
else
|
||||
ReferenceCounts[id] = next;
|
||||
}
|
||||
|
||||
public bool IsRenderDataReady(ulong id) => true;
|
||||
}
|
||||
|
||||
private sealed class RecordingReservations : IWorldRevealStreamingScheduler
|
||||
{
|
||||
public List<(long Generation, uint Cell, int Radius)> Begins { get; } = [];
|
||||
|
|
@ -206,11 +279,22 @@ public sealed class WorldRevealDerivedWindowIntegrationTests
|
|||
nearRadius: nearRadius,
|
||||
farRadius: farRadius);
|
||||
|
||||
/// <summary>
|
||||
/// The gate's most load-bearing predicate is
|
||||
/// <c>GpuWorldState.IsRenderReady</c>, which degenerates to
|
||||
/// <c>IsLoaded</c> when no spawn adapter is wired. Every fixture here owns
|
||||
/// a real <see cref="LandblockSpawnAdapter"/> so the predicate is actually
|
||||
/// under test.
|
||||
/// </summary>
|
||||
private static GpuWorldState CreateWorld() =>
|
||||
new(new LandblockSpawnAdapter(new RecordingMeshAdapter()));
|
||||
|
||||
private static void PublishRing(
|
||||
GpuWorldState world,
|
||||
PhysicsEngine physics,
|
||||
int radius,
|
||||
LandblockStreamTier tier)
|
||||
LandblockStreamTier tier,
|
||||
bool withEntity = false)
|
||||
{
|
||||
for (int dx = -radius; dx <= radius; dx++)
|
||||
for (int dy = -radius; dy <= radius; dy++)
|
||||
|
|
@ -221,8 +305,27 @@ public sealed class WorldRevealDerivedWindowIntegrationTests
|
|||
uint id = ((uint)(CenterX + dx) << 24)
|
||||
| ((uint)(CenterY + dy) << 16)
|
||||
| 0xFFFFu;
|
||||
WorldEntity[] entities = withEntity
|
||||
?
|
||||
[
|
||||
new WorldEntity
|
||||
{
|
||||
Id = 1,
|
||||
ServerGuid = 0,
|
||||
SourceGfxObjOrSetupId = 0x01000010u,
|
||||
Position = System.Numerics.Vector3.Zero,
|
||||
Rotation = System.Numerics.Quaternion.Identity,
|
||||
MeshRefs =
|
||||
[
|
||||
new MeshRef(
|
||||
0x01000010u,
|
||||
System.Numerics.Matrix4x4.Identity),
|
||||
],
|
||||
},
|
||||
]
|
||||
: Array.Empty<WorldEntity>();
|
||||
world.AddLandblock(
|
||||
new LoadedLandblock(id, new LandBlock(), Array.Empty<WorldEntity>()),
|
||||
new LoadedLandblock(id, new LandBlock(), entities),
|
||||
tier: tier);
|
||||
// The Far tier publishes terrain COLLISION as well as terrain
|
||||
// render (LandblockPhysicsPublisher, reached for
|
||||
|
|
|
|||
|
|
@ -7,6 +7,13 @@ public sealed class WorldRevealReadinessBarrierTests
|
|||
private sealed class State
|
||||
{
|
||||
public bool RenderReady;
|
||||
|
||||
/// <summary>
|
||||
/// Radius-aware override, so a test can distinguish "the Near
|
||||
/// sub-window is published" from "the whole Far window is published".
|
||||
/// </summary>
|
||||
public Func<int, int, bool>? RenderReadyByRadius;
|
||||
|
||||
public bool SpawnCellReady;
|
||||
public bool TerrainReady;
|
||||
public bool CompositeReady;
|
||||
|
|
@ -33,7 +40,8 @@ public sealed class WorldRevealReadinessBarrierTests
|
|||
{
|
||||
RenderNearRadius = nearRadius;
|
||||
RenderFarRadius = farRadius;
|
||||
return RenderReady;
|
||||
return RenderReadyByRadius?.Invoke(nearRadius, farRadius)
|
||||
?? RenderReady;
|
||||
},
|
||||
isSpawnCellReady: _ => SpawnCellReady,
|
||||
isTerrainNeighborhoodReady: (cell, radius) =>
|
||||
|
|
@ -197,6 +205,55 @@ public sealed class WorldRevealReadinessBarrierTests
|
|||
Assert.Equal(0, state.Preparations);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// The composite warmup TRIGGER is scoped to the composite DOMAIN. #280
|
||||
/// widened the reveal gate to the whole Far window but left the domain at
|
||||
/// <c>NearRadius</c>; leaving the trigger on the gate would serialise every
|
||||
/// composite upload behind the last outer-ring landblock and lengthen the
|
||||
/// hold by the whole warmup duration for no readiness benefit.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void Prepare_StartsWarmupOnceTheNearSubWindowIsPublished()
|
||||
{
|
||||
const uint outdoorCell = 0x11340021u;
|
||||
var state = new State
|
||||
{
|
||||
Window = new StreamingRevealWindow(4, 12),
|
||||
// Published out to the Near radius only — the Far ring is still
|
||||
// streaming, which is the normal state for most of the hold.
|
||||
RenderReadyByRadius = (_, farRadius) => farRadius <= 4,
|
||||
};
|
||||
var barrier = state.Build();
|
||||
|
||||
barrier.Prepare(outdoorCell);
|
||||
|
||||
Assert.Equal(1, state.Preparations);
|
||||
Assert.Equal(state.Window.NearRadius, state.PreparedRadius);
|
||||
Assert.Equal(state.Window.NearRadius, state.RenderFarRadius);
|
||||
// ...and the gate itself has NOT opened: it still measures the whole
|
||||
// derived window, and composites are not ready yet either.
|
||||
Assert.False(barrier.IsReady(outdoorCell));
|
||||
Assert.Equal(state.Window.FarRadius, state.RenderFarRadius);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// The Near sub-window is a real precondition, not a formality: an
|
||||
/// unpublished destination neighbourhood still blocks warmup.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void Prepare_StillWaitsWhenTheNearSubWindowIsIncomplete()
|
||||
{
|
||||
var state = new State
|
||||
{
|
||||
Window = new StreamingRevealWindow(4, 12),
|
||||
RenderReadyByRadius = (_, _) => false,
|
||||
};
|
||||
|
||||
state.Build().Prepare(0x11340021u);
|
||||
|
||||
Assert.Equal(0, state.Preparations);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ImpossibleClaim_CrossesExistingLoudRecoveryPath()
|
||||
{
|
||||
|
|
@ -290,4 +347,24 @@ public sealed class WorldRevealReadinessBarrierTests
|
|||
new StreamingRevealWindow(4, 25),
|
||||
StreamingDiagnostics.ApplyRevealRadiusOverride(window, 25));
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// The probe's parser floor is 1, not 0. A zero override makes
|
||||
/// <c>RequiredWindow</c> return far = 0 for an OUTDOOR destination, which
|
||||
/// Runtime's <c>invalid-readiness-shape</c> invariant rejects on every
|
||||
/// acknowledgement — i.e. the probe would hang the exact A/B route it
|
||||
/// exists to measure. Reject it where it is read, not where it detonates.
|
||||
/// </summary>
|
||||
[Theory]
|
||||
[InlineData(null, null)]
|
||||
[InlineData("", null)]
|
||||
[InlineData("nonsense", null)]
|
||||
[InlineData("0", null)]
|
||||
[InlineData("-1", null)]
|
||||
[InlineData("1", 1)]
|
||||
[InlineData("12", 12)]
|
||||
public void RevealRadiusOverride_ParserRefusesRadiiRuntimeWouldReject(
|
||||
string? raw,
|
||||
int? expected) =>
|
||||
Assert.Equal(expected, StreamingDiagnostics.ParseRadius(raw));
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue