b44dd147bc
Ports retail CELLARRAY::add_cell (acclient_2013_pseudo_c.txt:701036): ordered list, dedup by cell_id, append at end. The order is load-bearing for the verbatim find_cell_list current-cell-first interior-wins pick (next commits) that fixes the R1 cottage membership flap. Implements ICollection<uint> (helper-facing) + IReadOnlyCollection<uint> (consumer-facing). 5 unit tests. Also lands the membership-port pseudocode (workflow step 3) + the Stage-1 plan. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
636 lines
34 KiB
Markdown
636 lines
34 KiB
Markdown
# Cell-Membership Ordered-CELLARRAY Port — Implementation Plan (Stage 1, the R1 flap fix)
|
||
|
||
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||
|
||
**Goal:** Replace acdream's unordered-`HashSet` cell-membership pick with retail's **ordered CELLARRAY, current-cell-first, interior-wins-break** pick (a verbatim port of `CObjCell::find_cell_list`), so the player's cell stops ping-ponging at cottage doorways/rooms — the "flap" the R1 render redesign exposed.
|
||
|
||
**Architecture:** Introduce a small ordered+deduped `CellArray` (List for order + HashSet for O(1) dedup, modeling retail `CELLARRAY::add_cell`). Rewrite `CellTransit.BuildCellSetAndPickContaining` to build candidates into a `CellArray` (current cell at index 0, neighbours appended in `find_transit_cells` order) and pick **in array order, interior-wins-break** — the retail hysteresis. The persistent-state half (the `change_cell` equivalent) is **already ported** (`ValidateTransition` commits `sp.CurCellId`; `ResolveWithTransition`→`SetCurrAndReturn` writes `CellGraph.CurrCell`; `PlayerMovementController.UpdateCellId` applies it) — only the pick was wrong. Delete the `5ca2f44` current-first pre-check (the ordered pick subsumes it); keep its regression test.
|
||
|
||
**Tech Stack:** C# .NET 10, xUnit (Core tests under `tests/AcDream.Core.Tests/`). Pure Core logic — GL-free, fully unit-testable.
|
||
|
||
**Decomp oracle (verified 2026-06-02/03):** `docs/research/2026-06-03-cell-membership-ordered-cellarray-pseudocode.md` — the pseudocode this plan ports. Anchors: `CELLARRAY::add_cell` @ `acclient_2013_pseudo_c.txt:701036`; `CObjCell::find_cell_list` @ 308742 (pick 308788-308825); `CEnvCell::find_transit_cells` @ 309968; `CTransition::check_other_cells` @ 272717; `validate_transition` @ 272547; `SetPositionInternal` @ 283399.
|
||
|
||
**Key finding (why this is surgical, not a new state machine):** the decomp shows retail's stability is *emergent* from the ordered current-first pick + the carried-forward seed (`sphere_path.check_pos.objcell_id`) + multi-valued `check_other_cells` collision. There is **no separate portal-crossing detector** — `find_cell_list` rebuilds the array and re-picks every call. The §4.4 "mutate only at portal crossings" framing is the *effect*; the *mechanism* (handoff §4.5, confirmed) is the ordered pick. acdream already has the persistent-state half, so Stage 1 is purely the pick.
|
||
|
||
**Scope:** Stage 1 ONLY (the flap). Stage 2 (uniform collision + intrinsic building entry: remove the `0x0100` fork / `TryFindIndoorWalkablePlane` / `CheckBuildingTransit` / the line-1958 pre-derive) is a **separate later plan** — do NOT do it here. The line-1958 `FindCellSet` pre-derive is load-bearing for outdoor→indoor seed promotion under the current forked structure and is KEPT in Stage 1.
|
||
|
||
---
|
||
|
||
## File Structure
|
||
|
||
| File | Responsibility | New/Modified |
|
||
|---|---|---|
|
||
| `src/AcDream.Core/Physics/CellArray.cs` | Ordered, deduped cell-id collection (retail CELLARRAY). `ICollection<uint>` + `IReadOnlyCollection<uint>`. Pure, unit-tested. | **Create** |
|
||
| `tests/AcDream.Core.Tests/Physics/CellArrayTests.cs` | Unit tests: ordered append, dedup-by-id, order preserved, interface enumeration. | **Create** |
|
||
| `src/AcDream.Core/Physics/CellTransit.cs` | (Task 2) widen helper params `HashSet<uint>`→`ICollection<uint>`. (Task 3) rewrite `BuildCellSetAndPickContaining` to the ordered CellArray + verbatim pick; delete the `5ca2f44` pre-check. | **Modify** |
|
||
| `tests/AcDream.Core.Tests/Physics/CellTransitFindCellSetTests.cs` | Keep the `TwoOverlappingCells` guard; add ordered-pick conformance tests. | **Modify** |
|
||
|
||
No other production files change in Stage 1. `FindCellSet`'s public signature (`out IReadOnlyCollection<uint>`), `CheckOtherCells`, and `LogCellSetBuild` already take `IReadOnlyCollection<uint>` — `CellArray` implements it, so they need no edit.
|
||
|
||
---
|
||
|
||
## Task 1: `CellArray` — the ordered, deduped collection (pure, TDD)
|
||
|
||
**Files:**
|
||
- Create: `src/AcDream.Core/Physics/CellArray.cs`
|
||
- Test: `tests/AcDream.Core.Tests/Physics/CellArrayTests.cs`
|
||
|
||
**Why:** Retail `CELLARRAY` is an ordered list deduped by `cell_id` (`add_cell` @701036: linear search → return-if-present → append at end). The **order** is load-bearing: `find_cell_list` adds the current cell at index 0 and the pick iterates in order, so the current cell wins a boundary straddle. acdream's `HashSet` discarded that order. This type restores it. Pure logic — TDD.
|
||
|
||
- [ ] **Step 1: Write the failing test**
|
||
|
||
```csharp
|
||
// tests/AcDream.Core.Tests/Physics/CellArrayTests.cs
|
||
using System.Collections.Generic;
|
||
using System.Linq;
|
||
using AcDream.Core.Physics;
|
||
using Xunit;
|
||
|
||
namespace AcDream.Core.Tests.Physics;
|
||
|
||
public class CellArrayTests
|
||
{
|
||
[Fact]
|
||
public void Add_PreservesInsertionOrder()
|
||
{
|
||
var a = new CellArray();
|
||
a.Add(0xA9B40170u);
|
||
a.Add(0xA9B40031u);
|
||
a.Add(0xA9B40171u);
|
||
Assert.Equal(new[] { 0xA9B40170u, 0xA9B40031u, 0xA9B40171u }, a.OrderedIds.ToArray());
|
||
}
|
||
|
||
[Fact]
|
||
public void Add_DedupsById_KeepingFirstPosition()
|
||
{
|
||
var a = new CellArray();
|
||
a.Add(0xA9B40170u);
|
||
a.Add(0xA9B40171u);
|
||
a.Add(0xA9B40170u); // duplicate of index 0 — no-op (retail add_cell)
|
||
Assert.Equal(2, a.Count);
|
||
Assert.Equal(new[] { 0xA9B40170u, 0xA9B40171u }, a.OrderedIds.ToArray());
|
||
}
|
||
|
||
[Fact]
|
||
public void Contains_TracksMembership()
|
||
{
|
||
var a = new CellArray();
|
||
a.Add(0xA9B40170u);
|
||
Assert.True(a.Contains(0xA9B40170u));
|
||
Assert.False(a.Contains(0xA9B40171u));
|
||
}
|
||
|
||
[Fact]
|
||
public void EnumeratesInInsertionOrder_AsICollection()
|
||
{
|
||
var a = new CellArray();
|
||
a.Add(3u); a.Add(1u); a.Add(2u);
|
||
ICollection<uint> c = a; // helper-facing interface
|
||
Assert.Equal(new[] { 3u, 1u, 2u }, c.ToArray());
|
||
}
|
||
|
||
[Fact]
|
||
public void IsReadOnlyCollection_ForConsumers()
|
||
{
|
||
var a = new CellArray();
|
||
a.Add(7u); a.Add(7u);
|
||
IReadOnlyCollection<uint> ro = a; // consumer-facing interface (FindCellSet out)
|
||
Assert.Equal(1, ro.Count);
|
||
Assert.Equal(new[] { 7u }, ro.ToArray());
|
||
}
|
||
}
|
||
```
|
||
|
||
- [ ] **Step 2: Run test to verify it fails**
|
||
|
||
Run: `dotnet test tests/AcDream.Core.Tests/AcDream.Core.Tests.csproj --filter CellArrayTests`
|
||
Expected: FAIL — `CellArray` does not exist (compile error).
|
||
|
||
- [ ] **Step 3: Write the implementation**
|
||
|
||
```csharp
|
||
// src/AcDream.Core/Physics/CellArray.cs
|
||
using System.Collections;
|
||
using System.Collections.Generic;
|
||
|
||
namespace AcDream.Core.Physics;
|
||
|
||
/// <summary>
|
||
/// Ordered, deduped cell-id collection — a faithful model of retail's CELLARRAY
|
||
/// (<c>CELLARRAY::add_cell</c> @ <c>acclient_2013_pseudo_c.txt:701036</c>: linear
|
||
/// dedup by cell_id, append at the END, insertion order preserved). The order is
|
||
/// load-bearing for <c>CObjCell::find_cell_list</c>'s current-cell-first,
|
||
/// interior-wins pick (pc:308742) — the current cell is added at index 0 and the
|
||
/// pick iterates in order, so the current cell wins a boundary straddle and the
|
||
/// membership does not ping-pong (the R1 flap fix). Replaces the unordered
|
||
/// <see cref="HashSet{T}"/> the candidate build used to use.
|
||
///
|
||
/// <para>Implements <see cref="ICollection{T}"/> (so the candidate-building
|
||
/// helpers can take it where they used to take <c>HashSet<uint></c>) and
|
||
/// <see cref="IReadOnlyCollection{T}"/> (so it satisfies the
|
||
/// <c>out IReadOnlyCollection<uint></c> on <c>FindCellSet</c> and the
|
||
/// <c>CheckOtherCells</c> / diagnostics consumers). Enumeration is always
|
||
/// insertion order.</para>
|
||
/// </summary>
|
||
public sealed class CellArray : ICollection<uint>, IReadOnlyCollection<uint>
|
||
{
|
||
private readonly List<uint> _order = new();
|
||
private readonly HashSet<uint> _seen = new();
|
||
|
||
public int Count => _order.Count;
|
||
public bool IsReadOnly => false;
|
||
|
||
/// <summary>Ordered cell ids; index 0 is the cell added first (the current cell).</summary>
|
||
public IReadOnlyList<uint> OrderedIds => _order;
|
||
|
||
/// <summary>Append <paramref name="id"/> iff not already present (retail add_cell dedup).</summary>
|
||
public void Add(uint id)
|
||
{
|
||
if (_seen.Add(id))
|
||
_order.Add(id);
|
||
}
|
||
|
||
public bool Contains(uint id) => _seen.Contains(id);
|
||
|
||
public void Clear() { _order.Clear(); _seen.Clear(); }
|
||
|
||
public bool Remove(uint id)
|
||
{
|
||
if (!_seen.Remove(id)) return false;
|
||
_order.Remove(id);
|
||
return true;
|
||
}
|
||
|
||
public void CopyTo(uint[] array, int arrayIndex) => _order.CopyTo(array, arrayIndex);
|
||
|
||
public IEnumerator<uint> GetEnumerator() => _order.GetEnumerator();
|
||
IEnumerator IEnumerable.GetEnumerator() => _order.GetEnumerator();
|
||
}
|
||
```
|
||
|
||
- [ ] **Step 4: Run test to verify it passes**
|
||
|
||
Run: `dotnet test tests/AcDream.Core.Tests/AcDream.Core.Tests.csproj --filter CellArrayTests`
|
||
Expected: PASS (5 tests).
|
||
|
||
- [ ] **Step 5: Commit**
|
||
|
||
```bash
|
||
git add src/AcDream.Core/Physics/CellArray.cs tests/AcDream.Core.Tests/Physics/CellArrayTests.cs
|
||
git commit -m "feat(physics): Stage 1 — CellArray ordered/deduped cell collection (retail CELLARRAY)
|
||
|
||
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>"
|
||
```
|
||
|
||
---
|
||
|
||
## Task 2: Widen the candidate-building helper params `HashSet<uint>` → `ICollection<uint>`
|
||
|
||
**Files:**
|
||
- Modify: `src/AcDream.Core/Physics/CellTransit.cs` (the 3 public helpers + the private `AddOutsideCell`)
|
||
|
||
**Why:** The candidate-building helpers (`FindTransitCellsSphere` ×2 overloads, `AddAllOutsideCells` ×2 overloads, `AddOutsideCell`, `CheckBuildingTransit`) currently take `HashSet<uint> candidates`. Task 3 passes them a `CellArray` instead. Widen the parameter to `ICollection<uint>` (which both `HashSet<uint>` and `CellArray` implement) so production passes an ordered `CellArray` while the existing direct test callers (`CellTransitFindTransitCellsSphereTests`, `CellTransitAddAllOutsideCellsTests`, `CellTransitCheckBuildingTransitTests`) keep passing `new HashSet<uint>()` unchanged. This is a **non-behavioral type widening** — bodies only use `.Add` / `.Count` / `.Contains`, all on `ICollection<uint>`.
|
||
|
||
- [ ] **Step 1: Widen `FindTransitCellsSphere` (multi-sphere overload, ~line 74)**
|
||
|
||
In `CellTransit.cs`, change the signature parameter `HashSet<uint> candidates` to `ICollection<uint> candidates` on the multi-sphere `FindTransitCellsSphere`:
|
||
|
||
```csharp
|
||
public static void FindTransitCellsSphere(
|
||
PhysicsDataCache cache,
|
||
CellPhysics currentCell,
|
||
uint currentCellId,
|
||
IReadOnlyList<Sphere> worldSpheres,
|
||
int numSpheres,
|
||
ICollection<uint> candidates,
|
||
out bool exitOutside)
|
||
```
|
||
|
||
- [ ] **Step 2: Widen `FindTransitCellsSphere` (single-sphere overload, ~line 46)**
|
||
|
||
```csharp
|
||
public static void FindTransitCellsSphere(
|
||
PhysicsDataCache cache,
|
||
CellPhysics currentCell,
|
||
uint currentCellId,
|
||
Vector3 worldSphereCenter,
|
||
float sphereRadius,
|
||
ICollection<uint> candidates,
|
||
out bool exitOutside)
|
||
```
|
||
|
||
- [ ] **Step 3: Widen both `AddAllOutsideCells` overloads (~line 212 and ~line 256) and `AddOutsideCell` (~line 270)**
|
||
|
||
```csharp
|
||
public static void AddAllOutsideCells(
|
||
Vector3 worldSphereCenter,
|
||
float sphereRadius,
|
||
uint currentCellId,
|
||
ICollection<uint> candidates)
|
||
```
|
||
```csharp
|
||
public static void AddAllOutsideCells(
|
||
IReadOnlyList<Sphere> worldSpheres,
|
||
int numSpheres,
|
||
uint currentCellId,
|
||
ICollection<uint> candidates)
|
||
```
|
||
```csharp
|
||
private static void AddOutsideCell(ICollection<uint> candidates, uint lbPrefix, int gridX, int gridY)
|
||
```
|
||
|
||
- [ ] **Step 4: Widen `CheckBuildingTransit` (~line 299)**
|
||
|
||
```csharp
|
||
public static void CheckBuildingTransit(
|
||
PhysicsDataCache cache,
|
||
BuildingPhysics building,
|
||
Vector3 worldSphereCenter,
|
||
float sphereRadius,
|
||
ICollection<uint> candidates)
|
||
```
|
||
|
||
- [ ] **Step 5: Build**
|
||
|
||
Run: `dotnet build src/AcDream.Core/AcDream.Core.csproj`
|
||
Expected: build succeeds. (`BuildCellSetAndPickContaining` still passes its `HashSet` here — that's Task 3. `HashSet<uint>` IS an `ICollection<uint>`, so the existing internal call still compiles.)
|
||
|
||
- [ ] **Step 6: Run the affected helper tests (no behavior change)**
|
||
|
||
Run: `dotnet test tests/AcDream.Core.Tests/AcDream.Core.Tests.csproj --filter "CellTransitFindTransitCellsSphereTests|CellTransitAddAllOutsideCellsTests|CellTransitCheckBuildingTransitTests"`
|
||
Expected: PASS (unchanged — they pass `new HashSet<uint>()`, which is still an `ICollection<uint>`).
|
||
|
||
- [ ] **Step 7: Commit**
|
||
|
||
```bash
|
||
git add src/AcDream.Core/Physics/CellTransit.cs
|
||
git commit -m "refactor(physics): Stage 1 — widen cell-candidate helpers to ICollection<uint>
|
||
|
||
Non-behavioral: lets BuildCellSetAndPickContaining pass an ordered CellArray (next
|
||
commit) while existing HashSet-passing test callers compile unchanged.
|
||
|
||
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>"
|
||
```
|
||
|
||
---
|
||
|
||
## Task 3: Rewrite `BuildCellSetAndPickContaining` — ordered CellArray + verbatim pick; delete the `5ca2f44` pre-check
|
||
|
||
**Files:**
|
||
- Modify: `src/AcDream.Core/Physics/CellTransit.cs` (`FindCellSet` multi-sphere overload ~line 412, and `BuildCellSetAndPickContaining` ~line 426-571)
|
||
- Test: `tests/AcDream.Core.Tests/Physics/CellTransitFindCellSetTests.cs` (add conformance tests; keep the guard)
|
||
|
||
**Why:** This is the core fix. Retail `CObjCell::find_cell_list` (pc:308742) builds an **ordered** array (current cell at index 0 via `add_cell`), expands via `find_transit_cells` in array order, and picks **in order with interior-wins-break** (pc:308788-308825). acdream's unordered `HashSet` lost the ordering, so at a doorway/room boundary where several cells' BSPs overlap the sphere centre, the enumeration could surface a neighbour before the current cell → the cell flips every tick → the flap. The ordered `CellArray` + verbatim pick restores the retail hysteresis: the current cell (index 0) wins while it still contains the centre. The `5ca2f44` pre-check (an indoor-only current-first approximation) is then redundant — delete it.
|
||
|
||
- [ ] **Step 1: Add/keep the conformance tests (write them first)**
|
||
|
||
In `tests/AcDream.Core.Tests/Physics/CellTransitFindCellSetTests.cs`, KEEP the existing `TwoOverlappingCells_CurrentCellWinsTheStraddle` `[Theory]` unchanged, and APPEND these tests inside the class:
|
||
|
||
```csharp
|
||
// The ordered-CELLARRAY contract: FindCellSet returns the candidate set in
|
||
// retail add-order with the CURRENT cell at index 0 (retail add_cell @308766).
|
||
// This is the invariant the verbatim pick relies on; the unordered HashSet
|
||
// could not guarantee it.
|
||
[Fact]
|
||
public void FindCellSet_CurrentCellIsFirstInTheSet()
|
||
{
|
||
var cellA = MakeCellWithPortalAtRightWall(Matrix4x4.Identity, otherCellId: 0x0101, flags: 0);
|
||
var cellBT = Matrix4x4.CreateTranslation(new Vector3(5f, 0f, 0f));
|
||
Matrix4x4.Invert(cellBT, out var cellBInv);
|
||
var cellB = new CellPhysics
|
||
{
|
||
WorldTransform = cellBT,
|
||
InverseWorldTransform = cellBInv,
|
||
Resolved = new Dictionary<ushort, ResolvedPolygon>(),
|
||
CellBSP = new CellBSPTree { Root = new CellBSPNode { Type = BSPNodeType.Leaf } },
|
||
};
|
||
var cache = new PhysicsDataCache();
|
||
cache.RegisterCellStructForTest(0xA9B40100u, cellA);
|
||
cache.RegisterCellStructForTest(0xA9B40101u, cellB);
|
||
|
||
// Straddle the portal plane so both cells are in the set.
|
||
var sphereCenter = new Vector3(2.0f, 0f, 2.5f);
|
||
|
||
CellTransit.FindCellSet(cache, sphereCenter, 0.5f, 0xA9B40100u, out var cellSet);
|
||
Assert.Equal(0xA9B40100u, cellSet.First()); // current cell at index 0
|
||
}
|
||
|
||
// Interior-wins over the outdoor fallback: while an interior cell still
|
||
// contains the centre, it wins even though the exit portal also added the
|
||
// outdoor landcell to the set (retail interior-wins-break, pc:308814-308819).
|
||
[Fact]
|
||
public void IndoorWithExitPortal_InteriorWinsWhileItContainsCentre()
|
||
{
|
||
// Interior cell at the landblock origin with an exit portal at local x=2.5;
|
||
// Leaf BSP contains any point. Centre at local (0,12,2.5) is INSIDE the cell
|
||
// and NOT across the exit plane, so interior must win even though the head
|
||
// sphere / exit logic may add the outdoor landcell.
|
||
var exitCell = MakeCellWithPortalAtRightWall(Matrix4x4.Identity, otherCellId: 0xFFFF, flags: 0);
|
||
var cache = new PhysicsDataCache();
|
||
cache.RegisterCellStructForTest(0xA9B40100u, exitCell);
|
||
|
||
var sphereCenter = new Vector3(0f, 12f, 2.5f);
|
||
uint containing = CellTransit.FindCellSet(cache, sphereCenter, 0.5f, 0xA9B40100u, out _);
|
||
Assert.Equal(0xA9B40100u, containing); // interior-wins, not the outdoor landcell
|
||
}
|
||
```
|
||
|
||
Add `using System.Linq;` at the top of the test file if not present (it is — line 2).
|
||
|
||
- [ ] **Step 2: Run the new tests against the CURRENT code (capture baseline)**
|
||
|
||
Run: `dotnet test tests/AcDream.Core.Tests/AcDream.Core.Tests.csproj --filter CellTransitFindCellSetTests`
|
||
Expected: the existing tests + `IndoorWithExitPortal_...` PASS; `FindCellSet_CurrentCellIsFirstInTheSet` MAY pass or fail under the current `HashSet` (its enumeration order is incidental). Record the result. Per the handoff §7 the static guards don't reliably go RED against the unordered pick — the real verification is the membership net + the visual gate (Task 4/5). Proceed regardless.
|
||
|
||
- [ ] **Step 3: Change `FindCellSet` (multi-sphere overload) to read the CellArray**
|
||
|
||
Replace the body of the multi-sphere `FindCellSet` (~line 412-424) so it reads the new `out CellArray`:
|
||
|
||
```csharp
|
||
public static uint FindCellSet(
|
||
PhysicsDataCache cache,
|
||
IReadOnlyList<Sphere> worldSpheres,
|
||
int numSpheres,
|
||
uint currentCellId,
|
||
out IReadOnlyCollection<uint> cellSet)
|
||
{
|
||
var containing = BuildCellSetAndPickContaining(
|
||
cache, worldSpheres, numSpheres, currentCellId,
|
||
out var candidates);
|
||
cellSet = candidates; // CellArray IS IReadOnlyCollection<uint>; enumerates in order
|
||
return containing;
|
||
}
|
||
```
|
||
|
||
- [ ] **Step 4: Replace `BuildCellSetAndPickContaining` in full**
|
||
|
||
Replace the entire `BuildCellSetAndPickContaining` method (~line 426 through its closing brace ~line 571) with:
|
||
|
||
```csharp
|
||
private static uint BuildCellSetAndPickContaining(
|
||
PhysicsDataCache cache,
|
||
IReadOnlyList<Sphere> worldSpheres,
|
||
int numSpheres,
|
||
uint currentCellId,
|
||
out CellArray candidates)
|
||
{
|
||
candidates = new CellArray();
|
||
int sphereCount = EffectiveSphereCount(worldSpheres, numSpheres);
|
||
if (sphereCount == 0) return currentCellId;
|
||
|
||
Vector3 worldSphereCenter = worldSpheres[0].Origin;
|
||
float sphereRadius = worldSpheres[0].Origin == default ? worldSpheres[0].Radius : worldSpheres[0].Radius;
|
||
uint currentLow = currentCellId & 0xFFFFu;
|
||
uint lbPrefix = currentCellId & 0xFFFF0000u;
|
||
|
||
if (currentLow >= 0x0100u)
|
||
{
|
||
// Indoor seed: the CURRENT cell is added at INDEX 0 (retail
|
||
// CObjCell::find_cell_list add_cell @ pseudo_c:308766). Index 0 is what
|
||
// makes the pick current-cell-first — the hysteresis that stops the flap.
|
||
var currentCell = cache.GetCellStruct(currentCellId);
|
||
if (currentCell is null) return currentCellId;
|
||
candidates.Add(currentCellId);
|
||
|
||
// EXPAND — a single forward walk over the GROWING array, mirroring
|
||
// retail's `for (i=0; i<num_cells; i++) cells[i].find_transit_cells(...)`
|
||
// loop (pseudo_c:308775-308785). find_transit_cells APPENDS portal
|
||
// neighbours (and, on an exit portal, the outdoor landcells) to the same
|
||
// array; CellArray.Add dedups, so the walk terminates when no new cell
|
||
// is appended. We read OrderedIds[i] by index because the list grows.
|
||
bool outdoorAdded = false;
|
||
for (int i = 0; i < candidates.Count; i++)
|
||
{
|
||
uint cellId = candidates.OrderedIds[i];
|
||
var cell = cache.GetCellStruct(cellId);
|
||
if (cell is null) continue;
|
||
|
||
FindTransitCellsSphere(
|
||
cache, cell, cellId, worldSpheres, sphereCount,
|
||
candidates, out bool exitOutside);
|
||
|
||
// A6.P5 (kept): the first exit-portal cell triggers the outdoor
|
||
// neighbourhood add once. Appended AFTER the interior cells, matching
|
||
// retail (CEnvCell::find_transit_cells calls add_all_outside_cells at
|
||
// the end, pseudo_c:310120) — so interior cells precede outdoor in the
|
||
// pick order and interior-wins is preserved.
|
||
if (exitOutside && !outdoorAdded)
|
||
{
|
||
AddAllOutsideCells(worldSpheres, sphereCount, currentCellId, candidates);
|
||
outdoorAdded = true;
|
||
}
|
||
}
|
||
}
|
||
else
|
||
{
|
||
// Outdoor seed: expand neighbour landcells (added first), then check each
|
||
// for a building stab whose portals cross into an interior EnvCell.
|
||
// (Stage 2 will make building entry intrinsic and remove CheckBuildingTransit.)
|
||
AddAllOutsideCells(worldSpheres, sphereCount, currentCellId, candidates);
|
||
|
||
var landcellSnapshot = new List<uint>(candidates.OrderedIds);
|
||
foreach (uint landcellId in landcellSnapshot)
|
||
{
|
||
var building = cache.GetBuilding(landcellId);
|
||
if (building is null) continue;
|
||
CheckBuildingTransit(cache, building, worldSphereCenter, sphereRadius, candidates);
|
||
}
|
||
}
|
||
|
||
if (PhysicsDiagnostics.ProbeCellSetEnabled)
|
||
PhysicsDiagnostics.LogCellSetBuild(currentCellId, worldSphereCenter, candidates);
|
||
|
||
// THE PICK — verbatim CObjCell::find_cell_list containing-cell pick
|
||
// (pseudo_c:308788-308825): iterate the array IN ORDER from index 0; for each
|
||
// cell, point_in_cell; set the running result on ANY containing cell;
|
||
// INTERIOR-WINS-BREAK. The current cell is at index 0, so if the sphere
|
||
// centre is still inside it, it wins and the search stops — the retail
|
||
// hysteresis. (This replaces the 5ca2f44 current-first pre-check, which
|
||
// approximated this for the indoor-current case only; the ordered array now
|
||
// delivers it for every seed by construction.)
|
||
uint outdoorResult = 0u;
|
||
foreach (uint candId in candidates.OrderedIds)
|
||
{
|
||
if ((candId & 0xFFFFu) >= 0x0100u)
|
||
{
|
||
// Interior candidate — point_in_cell via the cell BSP.
|
||
var cand = cache.GetCellStruct(candId);
|
||
if (cand?.CellBSP?.Root is null) continue;
|
||
var local = Vector3.Transform(worldSphereCenter, cand.InverseWorldTransform);
|
||
if (BSPQuery.PointInsideCellBsp(cand.CellBSP.Root, local))
|
||
return candId; // interior-wins, stop (308819)
|
||
}
|
||
else if (outdoorResult == 0u)
|
||
{
|
||
// Outdoor candidate — CLandCell::point_in_cell is the XY-column the
|
||
// sphere is over (acdream landcells have no BSP point_in_cell; this is
|
||
// the documented adaptation). Record it as the running result but DO
|
||
// NOT break — an interior cell later in the array can still win.
|
||
int gx = (int)(worldSphereCenter.X / 24f);
|
||
int gy = (int)(worldSphereCenter.Y / 24f);
|
||
if (gx >= 0 && gx < 8 && gy >= 0 && gy < 8)
|
||
{
|
||
uint outdoorId = lbPrefix | (uint)(gx * 8 + gy + 1);
|
||
if (candId == outdoorId)
|
||
outdoorResult = candId;
|
||
}
|
||
}
|
||
}
|
||
|
||
// No interior cell contained the centre. Return the outdoor XY-column cell if
|
||
// it was a candidate, else stay on the current cell (retail leaves *result
|
||
// null → caller keeps curr_cell).
|
||
return outdoorResult != 0u ? outdoorResult : currentCellId;
|
||
}
|
||
```
|
||
|
||
> **Note on the `sphereRadius` line:** simplify it — the `== default` ternary above is intentionally a no-op placeholder to flag that `sphereRadius` is just `worldSpheres[0].Radius`. Write it plainly:
|
||
> ```csharp
|
||
> float sphereRadius = worldSpheres[0].Radius;
|
||
> ```
|
||
|
||
- [ ] **Step 5: Build**
|
||
|
||
Run: `dotnet build src/AcDream.Core/AcDream.Core.csproj`
|
||
Expected: build succeeds. (`FindTransitCellsSphere` / `AddAllOutsideCells` / `CheckBuildingTransit` now take `ICollection<uint>` from Task 2 and accept the `CellArray`; `LogCellSetBuild` takes `IReadOnlyCollection<uint>` and accepts the `CellArray`.)
|
||
|
||
- [ ] **Step 6: Run the full membership net**
|
||
|
||
Run: `dotnet test tests/AcDream.Core.Tests/AcDream.Core.Tests.csproj --filter "CellTransit|FindEnvCollisions|CellGraph|Doorway|ResolveCellId|IndoorContactPlane"`
|
||
Expected: PASS, including `TwoOverlappingCells_CurrentCellWinsTheStraddle` (both `[Theory]` cases), the new conformance tests, and `DoorwayMembershipReplayTests` (no strobe). If a membership test fails, read it: is it asserting an old unordered-pick behaviour the verbatim port legitimately changed? If so, update it with a retail-cited reason (don't pin wrong values). If it reveals a real pick bug, fix the pick. Do NOT proceed to commit with a RED membership net unless the failure is a pre-existing baseline item (Task 4 confirms the baseline).
|
||
|
||
- [ ] **Step 7: Commit**
|
||
|
||
```bash
|
||
git add src/AcDream.Core/Physics/CellTransit.cs tests/AcDream.Core.Tests/Physics/CellTransitFindCellSetTests.cs
|
||
git commit -m "fix(physics): Stage 1 — verbatim ordered-CELLARRAY membership pick (the R1 flap)
|
||
|
||
Port CObjCell::find_cell_list (pseudo_c:308742) faithfully: build candidates into an
|
||
ordered CellArray (current cell at index 0), expand via find_transit_cells in array
|
||
order, pick in order with interior-wins-break. Restores retail's current-cell-first
|
||
hysteresis so the player's cell no longer ping-pongs at doorways/rooms. Deletes the
|
||
5ca2f44 current-first pre-check (subsumed by the ordered pick); keeps its guard test.
|
||
|
||
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>"
|
||
```
|
||
|
||
---
|
||
|
||
## Task 4: Full physics suite + baseline diff (breakage triage)
|
||
|
||
**Files:** none (verification); fixes to whatever genuinely broke.
|
||
|
||
**Why:** The user authorized breaking physics/movement tests to land the faithful port. Run the FULL Core suite, diff against the §10 baseline, and fix genuinely-new breakage (the port may legitimately change membership-dependent expectations — update those with retail-cited reasoning).
|
||
|
||
**Baseline (handoff §10):** deterministic membership net was 66 pass + 2 pre-existing `DoorBugTrajectoryReplayTests` failures (`TransientState live=0x87 harness=0x83`). Plus documented `PhysicsResolveCapture`/`PhysicsDiagnostics` static-leak flakiness (8–19 failures across runs of identical code). `tests/AcDream.App.Tests`: 174 green.
|
||
|
||
- [ ] **Step 1: Run the full Core suite**
|
||
|
||
Run: `dotnet test tests/AcDream.Core.Tests/AcDream.Core.Tests.csproj`
|
||
Capture the failure list.
|
||
|
||
- [ ] **Step 2: Diff against baseline**
|
||
|
||
For each failure: is it one of the documented pre-existing items (the 2 `DoorBug` `TransientState`, or the static-leak flakiness)? If so, ignore. If it's NEW:
|
||
- Membership-dependent expectation that the verbatim pick legitimately changed → update the test with a retail decomp citation (address + pc line). Do NOT pin a wrong value.
|
||
- A real regression in the pick → fix `BuildCellSetAndPickContaining` (re-read the decomp; the pseudocode doc is the oracle). Use `superpowers:systematic-debugging` if it resists.
|
||
|
||
- [ ] **Step 3: Re-run to confirm the failure set is a subset of baseline (modulo intentionally-updated tests)**
|
||
|
||
Run: `dotnet test tests/AcDream.Core.Tests/AcDream.Core.Tests.csproj`
|
||
Expected: only baseline-documented failures remain (or intentionally-updated tests now pass).
|
||
|
||
- [ ] **Step 4: Build the App layer (no signature break leaked out)**
|
||
|
||
Run: `dotnet build`
|
||
Expected: solution builds. (`FindCellSet`'s public signature is unchanged, so `GameWindow`/`TransitionTypes`/`PhysicsEngine` callers compile untouched.)
|
||
|
||
- [ ] **Step 5: Commit any test updates / fixes**
|
||
|
||
```bash
|
||
git add -A
|
||
git commit -m "test(physics): Stage 1 — reconcile membership tests with the verbatim pick
|
||
|
||
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>"
|
||
```
|
||
|
||
---
|
||
|
||
## Task 5: Visual flap gate (USER — the acceptance test)
|
||
|
||
**Files:** none (verification). **This task requires the user at the running client — no subagent.**
|
||
|
||
**Why:** Rendering/visual seal is verified on screen, never off the suite (CLAUDE.md). The deterministic harness proves the pick; the flap-gone is proven by the user's eyes + the auto-logging `ACDREAM_PROBE_CELL` count drop (no manual probe walk — the probe logs every `[cell-transit]` automatically while the user just walks normally).
|
||
|
||
- [ ] **Step 1: Confirm build is green before launch**
|
||
|
||
Run: `dotnet build`
|
||
Expected: green. (Never launch on a red build — it wastes the user's test time and can wedge the ACE session.)
|
||
|
||
- [ ] **Step 2: Launch with the cell probe (background, per CLAUDE.md "Running the client")**
|
||
|
||
```powershell
|
||
$env:ACDREAM_DAT_DIR="$env:USERPROFILE\Documents\Asheron's Call"; $env:ACDREAM_LIVE="1"
|
||
$env:ACDREAM_TEST_HOST="127.0.0.1"; $env:ACDREAM_TEST_PORT="9000"
|
||
$env:ACDREAM_TEST_USER="testaccount"; $env:ACDREAM_TEST_PASS="testpassword"
|
||
$env:ACDREAM_PROBE_CELL="1" # auto-logs [cell-transit]; confirms the count drop, no manual walk
|
||
dotnet run --project src\AcDream.App\AcDream.App.csproj --no-build -c Debug *>&1 | Tee-Object -FilePath launch-membership.log
|
||
```
|
||
Give it ~8 s to reach in-world. Logs are UTF-16 — read with `Select-String` / the Grep tool `--encoding utf-16-le`, NOT GNU grep. Close gracefully before relaunch (CloseMainWindow, not Stop-Process) so ACE clears the session in ~3–5 s.
|
||
|
||
- [ ] **Step 3: USER walks the cottage normally**
|
||
|
||
Outside `0031` → vestibule `0170` → room `0171` → stairs `0175` → cellar `0174`, and back.
|
||
|
||
- [ ] **Step 4: The visual flap gate (user confirms)**
|
||
|
||
PASS criteria:
|
||
- **Room/doorway flap GONE:** standing in / walking through the cottage room + vestibule, the interior is stable — no full-world/sky flash, no walls flickering transparent, no terrain bleeding in and out.
|
||
- **`[cell-transit]` count:** in `launch-membership.log`, the transition count for a single cottage walk drops from ~59 to ~6-8 (one transit per genuine boundary crossing).
|
||
|
||
- [ ] **Step 5: If the stairs still flap (expected per §8), record it as the next target — do NOT block Stage 1**
|
||
|
||
The stairs `0175↔0174` flip is paired with a foot-Z oscillation (~0.2 m/tick) = a SEPARATE physics-movement bug (#98 family). If the room/door flaps are gone but the stairs still flick, Stage 1 is COMPLETE — file the stairs Z-oscillation as the follow-up (diagnose via `ACDREAM_CAPTURE_RESOLVE` + the trajectory-replay harness, evidence-first).
|
||
|
||
- [ ] **Step 6: On gate pass — update the docs + roadmap, then commit**
|
||
|
||
- Note Stage 1 shipped in the render design spec §7 (R1 gate's membership blocker cleared) and the milestones doc M1.5 block.
|
||
- Update `docs/ISSUES.md` / the roadmap "shipped" table.
|
||
- Write a memory note if there's a durable lesson (e.g. the §4.3-vs-§4.4 reconciliation: the decomp mechanism is the ordered pick, not a separate crossing-detector).
|
||
|
||
```bash
|
||
git add -A
|
||
git commit -m "docs(physics): Stage 1 membership port shipped — R1 flap gate passed
|
||
|
||
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>"
|
||
```
|
||
|
||
---
|
||
|
||
## Self-Review
|
||
|
||
**1. Spec coverage** (handoff §4.5 Stage 1):
|
||
- Ordered, deduped CELLARRAY → Task 1 (`CellArray`). ✓
|
||
- Verbatim `find_cell_list` ordered current-first interior-wins pick → Task 3. ✓
|
||
- Thread the ordered collection through the candidate-building helpers → Task 2 (widen to `ICollection<uint>`) + Task 3 (pass the `CellArray`). ✓
|
||
- Multi-valued doorway membership / wake `CheckOtherCells` → already wired (`TransitionTypes.cs:2080-2084`); it now receives the ordered set. No code change needed; verified by the membership net (Task 3 Step 6) + `FindEnvCollisionsMultiCellTests` (Task 4). ✓
|
||
- Delete the `5ca2f44` pre-check; KEEP its test → Task 3 Step 4 (delete) + Step 1 (keep `TwoOverlappingCells`). ✓
|
||
- Persistent state (`change_cell` equivalent) → already present (`ValidateTransition`→`SetCurrAndReturn`→`UpdateCellId`); no change (documented in Architecture + pseudocode §5.5). ✓
|
||
- Stage 2 (uniform collision, intrinsic entry, remove line-1958 pre-derive) → explicitly OUT OF SCOPE (Scope section). ✓
|
||
|
||
**2. Placeholder scan:** One deliberate flag — the `sphereRadius` line in Task 3 Step 4 has a redundant ternary with a `>` Note immediately telling the implementer to write `float sphereRadius = worldSpheres[0].Radius;`. Fixed inline via the Note. No other TBD/TODO/"handle later".
|
||
|
||
**3. Type consistency:** `CellArray` implements both `ICollection<uint>` (Task 1) — matches the widened helper params (Task 2) — and `IReadOnlyCollection<uint>` (Task 1) — matches `FindCellSet`'s `out` (Task 3 Step 3), `LogCellSetBuild`, and `CheckOtherCells`. `BuildCellSetAndPickContaining`'s `out CellArray` (Task 3 Step 4) ↔ `FindCellSet`'s `out var candidates` then `cellSet = candidates` (Task 3 Step 3). `OrderedIds` (`IReadOnlyList<uint>`) used for index walk + snapshot. All consistent.
|
||
|
||
**Risks carried into execution (documented, non-blocking):**
|
||
- The static conformance tests may not go RED against the current `HashSet` (handoff §7) — the real verification is the membership net + visual gate. Acknowledged in Task 3 Step 2.
|
||
- The stairs Z-oscillation (§8) is a separate bug; Stage 1 is not blocked on it (Task 5 Step 5).
|
||
- Keeping the line-1958 pre-derive means a redundant `FindCellSet` call per tick (now ordered, so stable). Removing it is Stage 2. Acceptable perf (N.6 baseline shows large CPU headroom; this is one extra ordered pick over a handful of cells).
|
||
|
||
---
|
||
|
||
## Execution Handoff
|
||
|
||
Plan complete and saved to `docs/superpowers/plans/2026-06-03-membership-ordered-cellarray-port.md`.
|
||
|
||
This is a **physics port** — per CLAUDE.md, faithful ports need full context (the decomp + the existing code), so subagent isolation is discouraged here. Recommend **Inline Execution** (`superpowers:executing-plans`) in this session: Tasks 1-4 are bounded code+test changes with build/test checkpoints; Task 5 is a hard stop for the user's visual flap gate.
|