From 16588824394450c56aadbaad18bf6192b7e16b44 Mon Sep 17 00:00:00 2001 From: Erik Date: Sat, 9 May 2026 22:49:35 +0200 Subject: [PATCH] fix(A.5 T4-T6): bootstrap guard + dead enum + test cleanups MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code review on commits 7bcabab/fb6b61e/326b698 flagged 2 Important + 4 Minor issues. Apply all fixes: Important: - Two-tier RecenterTo + MarkResidentFromBootstrap now throw InvalidOperationException on misuse — calling RecenterTo before the bootstrap silently emitted the entire window as fresh loads (no demotes/unloads since _tierResidence was empty), a correctness hazard that produced no exception. Calling MarkResidentFromBootstrap twice silently dropped accumulated tier state. Both now crash loudly via a _bootstrapped flag. - Dropped TierResidence.None from the enum — never assigned, never checked; absence from the dictionary already encodes "not resident." Minor: - Renamed test: RecenterTo_FirstTick_* → ComputeFirstTickDiff_FirstTick_* (the test calls ComputeFirstTickDiff, not RecenterTo). - Strengthened RecenterTo_PlayerWalks_NullToFar_* with assertions for ToPromote.Count==3 (the x=102 column promoting Far→Near) and ToUnload.Empty (everything within hysteresis). - Replaced System.Math.Abs with Math.Abs in new code to match the file's existing `using System;` convention. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/AcDream.App/Streaming/StreamingRegion.cs | 41 +++++++++++++++---- .../Streaming/StreamingRegionTwoTierTests.cs | 9 +++- 2 files changed, 40 insertions(+), 10 deletions(-) diff --git a/src/AcDream.App/Streaming/StreamingRegion.cs b/src/AcDream.App/Streaming/StreamingRegion.cs index b4c1056..01eb85d 100644 --- a/src/AcDream.App/Streaming/StreamingRegion.cs +++ b/src/AcDream.App/Streaming/StreamingRegion.cs @@ -26,6 +26,13 @@ public sealed class StreamingRegion // Two-tier residence tracking: maps each resident LB to its current tier. private readonly Dictionary _tierResidence = new(); + // Set true after MarkResidentFromBootstrap. The two-tier RecenterTo + // requires this state to be seeded; calling RecenterTo before the + // bootstrap silently emits the entire window as fresh loads (no demotes, + // no unloads, since _tierResidence is empty), which is a correctness + // hazard. The flag converts that into a loud InvalidOperationException. + private bool _bootstrapped; + /// /// Landblock IDs in the current visible window in the AC 8.8 coordinate /// form: (lbX << 24) | (lbY << 16) | 0xFFFF. The trailing @@ -132,6 +139,12 @@ public sealed class StreamingRegion /// public void MarkResidentFromBootstrap() { + if (_bootstrapped) + throw new InvalidOperationException( + "MarkResidentFromBootstrap was already called; calling it again would " + + "reset accumulated tier-residence state and silently drop differential " + + "data built up by interim RecenterTo calls."); + _tierResidence.Clear(); for (int dx = -FarRadius; dx <= FarRadius; dx++) { @@ -140,14 +153,15 @@ public sealed class StreamingRegion int nx = CenterX + dx; int ny = CenterY + dy; if (nx < 0 || nx > 0xFF || ny < 0 || ny > 0xFF) continue; - int absDx = System.Math.Abs(dx); - int absDy = System.Math.Abs(dy); + int absDx = Math.Abs(dx); + int absDy = Math.Abs(dy); var id = EncodeLandblockId(nx, ny); _tierResidence[id] = (absDx <= NearRadius && absDy <= NearRadius) ? TierResidence.Near : TierResidence.Far; } } + _bootstrapped = true; } /// @@ -165,6 +179,13 @@ public sealed class StreamingRegion /// public TwoTierDiff RecenterTo(int newCx, int newCy) { + if (!_bootstrapped) + throw new InvalidOperationException( + "Two-tier RecenterTo called before MarkResidentFromBootstrap. " + + "First call ComputeFirstTickDiff to enqueue the bootstrap loads, " + + "then MarkResidentFromBootstrap to seed _tierResidence, then RecenterTo " + + "for subsequent observer moves."); + int nearUnloadThreshold = NearRadius + 2; int farUnloadThreshold = FarRadius + 2; @@ -183,8 +204,8 @@ public sealed class StreamingRegion int nx = newCx + dx; int ny = newCy + dy; if (nx < 0 || nx > 0xFF || ny < 0 || ny > 0xFF) continue; - int absDx = System.Math.Abs(dx); - int absDy = System.Math.Abs(dy); + int absDx = Math.Abs(dx); + int absDy = Math.Abs(dy); bool inNear = absDx <= NearRadius && absDy <= NearRadius; var id = EncodeLandblockId(nx, ny); newCenterIds.Add(id); @@ -213,9 +234,9 @@ public sealed class StreamingRegion var current = kvp.Value; int lbX = (int)((id >> 24) & 0xFFu); int lbY = (int)((id >> 16) & 0xFFu); - int absDx = System.Math.Abs(lbX - newCx); - int absDy = System.Math.Abs(lbY - newCy); - int distance = System.Math.Max(absDx, absDy); // Chebyshev + int absDx = Math.Abs(lbX - newCx); + int absDy = Math.Abs(lbY - newCy); + int distance = Math.Max(absDx, absDy); // Chebyshev if (newCenterIds.Contains(id)) { @@ -317,6 +338,8 @@ public readonly record struct RegionDiff( IReadOnlyList ToUnload); /// -/// Tracks which tier a landblock currently occupies in the two-tier streaming model. +/// Tracks which tier a landblock currently occupies in the two-tier streaming +/// model. Absence from the dictionary encodes "not resident"; the enum has no +/// None member to avoid suggesting a third runtime state. /// -internal enum TierResidence { None, Far, Near } +internal enum TierResidence { Far, Near } diff --git a/tests/AcDream.Core.Tests/Streaming/StreamingRegionTwoTierTests.cs b/tests/AcDream.Core.Tests/Streaming/StreamingRegionTwoTierTests.cs index 19364cf..5891245 100644 --- a/tests/AcDream.Core.Tests/Streaming/StreamingRegionTwoTierTests.cs +++ b/tests/AcDream.Core.Tests/Streaming/StreamingRegionTwoTierTests.cs @@ -22,7 +22,7 @@ public class StreamingRegionTwoTierTests } [Fact] - public void RecenterTo_FirstTick_SplitsLoadIntoNearAndFar() + public void ComputeFirstTickDiff_FirstTick_SplitsLoadIntoNearAndFar() { // near=1, far=3 → near window is 3×3=9, far window is 7×7-3×3=40 LBs. var region = new StreamingRegion(centerX: 100, centerY: 100, nearRadius: 1, farRadius: 3); @@ -52,6 +52,13 @@ public class StreamingRegionTwoTierTests Assert.Contains(id, diff.ToLoadFar); } Assert.Empty(diff.ToLoadNear); + // The 3 LBs at x=102, y in {99,100,101} were Far from old center + // (distance 2) and are now Near from new center (distance ≤1). + // They should land in ToPromote. + Assert.Equal(3, diff.ToPromote.Count); + // All resident LBs from the old window are within hysteresis of + // the new center (max distance 4 ≤ FarRadius+2=5), so nothing unloads. + Assert.Empty(diff.ToUnload); } [Fact]