feat(physics): Stage 1 — CellArray ordered/deduped cell collection (retail CELLARRAY)
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>
This commit is contained in:
parent
1438d73a43
commit
b44dd147bc
4 changed files with 992 additions and 0 deletions
|
|
@ -0,0 +1,241 @@
|
|||
# Pseudocode — retail cell-membership (ordered CELLARRAY pick) → acdream port — 2026-06-03
|
||||
|
||||
> Mandatory workflow step 3 (WRITE PSEUDOCODE) for the Stage-1 membership port
|
||||
> (the R1 cottage "flap"). Inputs: the canonical handoff
|
||||
> [`2026-06-02-membership-verbatim-port-handoff.md`](2026-06-02-membership-verbatim-port-handoff.md)
|
||||
> §4.4/§4.5; the decomp read this session (addresses below). This doc translates
|
||||
> the verbatim decomp to readable pseudocode BEFORE porting, to catch misreads
|
||||
> (the frame-swap-bug lesson).
|
||||
|
||||
---
|
||||
|
||||
## 0. The finding that grounds the whole port (decomp wins over framing)
|
||||
|
||||
I read the persistence chain verbatim. **Retail membership stability is *emergent*
|
||||
from an ordered, current-first pick over a deduped CELLARRAY — there is NO separate
|
||||
"portal-plane-crossing detector" that gates state mutation.** The handoff's §4.4
|
||||
conceptual framing ("mutate ONLY at a portal crossing, NOT re-derived per tick") is
|
||||
the *effect*; the *mechanism* (handoff §4.5, and verified here) is the ordered pick
|
||||
+ the carried-forward seed + multi-valued collision. `find_cell_list` **rebuilds the
|
||||
array every call** (`num_cells = 0`, pc:308747) and re-picks every transition step —
|
||||
exactly like acdream. The ONLY divergence is acdream's **unordered `HashSet`** pick
|
||||
vs retail's **ordered CELLARRAY** (current at index 0, interior-wins-break).
|
||||
|
||||
⇒ The faithful Stage-1 fix is surgical: an ordered `CellArray` + the verbatim pick.
|
||||
acdream already has the persistent-state half (`CellId`/`CellGraph.CurrCell` ← swept
|
||||
`sp.CurCellId`, committed by `ValidateTransition`, applied by `UpdateCellId` = the
|
||||
`SetPositionInternal`/`change_cell` equivalent). No new state machine is needed.
|
||||
|
||||
### Decomp anchors (verified this session, file `docs/research/named-retail/acclient_2013_pseudo_c.txt`)
|
||||
|
||||
| Function | addr | pc | role |
|
||||
|---|---|---|---|
|
||||
| `CELLARRAY::add_cell` | 0x6b4ff0 | 701036 | ordered append, **dedup by cell_id** (search→return-if-present) |
|
||||
| `CObjCell::find_cell_list` (full) | 0x52b4e0 | 308742 | rebuild array; current@0; expand; **ordered interior-wins pick** |
|
||||
| `CObjCell::find_cell_list` (sphere-path 3-arg) | 0x52b960 | 309085 | seeds from `sphere_path.check_pos.objcell_id` + `global_sphere` |
|
||||
| `CEnvCell::find_transit_cells` (sphere) | 0x52c820 | 309968 | append portal neighbours; exit-portal flag → `add_all_outside_cells` |
|
||||
| `CTransition::check_other_cells` | 0x50ae50 | 272717 | build array; collide ALL other cells; `check_cell = var_4c` |
|
||||
| `CTransition::validate_transition` | 0x50aa70 | 272547 | on accepted move: `curr_cell = check_cell` |
|
||||
| `CEnvCell::find_env_collisions` | 0x52c130 | 309573 | primary BSP vs seed — **NO pre-pick** |
|
||||
| `CPhysicsObj::SetPositionInternal(CTransition*)` | 0x515330 | 283399 | `change_cell` iff `this->cell != sphere_path.curr_cell` |
|
||||
| `CPhysicsObj::change_cell` | 0x513390 | 281192 | remove_object(old)/add_object(new) — pure registry move |
|
||||
|
||||
---
|
||||
|
||||
## 1. CELLARRAY (the ordered, deduped collection) — `add_cell` @ 701036
|
||||
|
||||
```
|
||||
struct CELLARRAY:
|
||||
cells: ordered list of {cell_id, cell_ptr} # insertion order preserved
|
||||
num_cells, added_outside
|
||||
|
||||
add_cell(arr, id, cellPtr):
|
||||
for existing in arr.cells: # linear dedup by id
|
||||
if existing.cell_id == id: return # already present → no-op
|
||||
arr.cells.append({id, cellPtr}) # else append at the END
|
||||
```
|
||||
|
||||
**acdream model:** `List<uint> _order` (order) + `HashSet<uint> _seen` (O(1) dedup).
|
||||
`Add(id)` appends iff `_seen.Add(id)`. Ordered enumeration; `Contains`; `Count`.
|
||||
(retail also stores the `CObjCell*`; acdream re-resolves via `cache.GetCellStruct(id)`
|
||||
at pick time, so the id-only list suffices.)
|
||||
|
||||
---
|
||||
|
||||
## 2. find_cell_list (the build + ordered pick) @ 308742
|
||||
|
||||
```
|
||||
find_cell_list(pos, num_sphere, global_sphere[], arr, out *result, sphere_path):
|
||||
arr.num_cells = 0; arr.added_outside = 0 # REBUILD every call
|
||||
objcell_id = pos.objcell_id # the CURRENT/seed cell
|
||||
cur = (objcell_id >= 0x100) ? CEnvCell::GetVisible(objcell_id)
|
||||
: CLandCell::GetVisible(objcell_id)
|
||||
|
||||
if objcell_id >= 0x100: # interior seed
|
||||
sphere_path.hits_interior_cell = 1
|
||||
add_cell(arr, objcell_id, cur) # CURRENT CELL AT INDEX 0 (308766)
|
||||
else: # outdoor seed
|
||||
CLandCell::add_all_outside_cells(pos, num_sphere, global_sphere, arr)
|
||||
|
||||
if cur != null and num_sphere != 0:
|
||||
# EXPAND — single forward walk over a GROWING array (find_transit_cells appends)
|
||||
for i in 0 .. arr.num_cells-1: # condition re-evaluated; array grows (308775)
|
||||
if arr.cells[i].cell != null:
|
||||
arr.cells[i].cell.find_transit_cells(pos, num_sphere, global_sphere, arr, sphere_path) # vtable[0x80] (308782)
|
||||
|
||||
# THE PICK — iterate IN ORDER from index 0, interior-wins-break (308788-308825)
|
||||
*result = null
|
||||
for i in 0 .. arr.num_cells-1:
|
||||
cell = arr.cells[i].cell
|
||||
if cell == null: continue
|
||||
p = global_sphere[0].center - LandDefs::get_block_offset(pos.objcell_id, cell.cell_id)
|
||||
if cell.point_in_cell(p): # vtable[0x84] (308810)
|
||||
*result = cell # set on ANY containing cell
|
||||
if (int16)cell.cell_id >= 0x100: # interior?
|
||||
sphere_path.hits_interior_cell = 1
|
||||
break # INTERIOR-WINS — stop (308819)
|
||||
|
||||
# do_not_load_cells prune (308829) — OUT OF SCOPE for the flap
|
||||
```
|
||||
|
||||
**Load-bearing facts:** current cell at **index 0**; pick iterates **in order**;
|
||||
**breaks on the first interior cell that contains the center.** ⇒ if the center is
|
||||
still inside the current cell, the current cell wins and the search stops — the
|
||||
hysteresis. An outdoor cell sets `*result` but does NOT break (interior can still win
|
||||
later). If no interior contains the center, the last containing (outdoor) cell wins.
|
||||
|
||||
---
|
||||
|
||||
## 3. find_transit_cells (the per-cell expansion, sphere variant) @ 309968 (CEnvCell)
|
||||
|
||||
```
|
||||
CEnvCell::find_transit_cells(pos, num_sphere, global_sphere[], arr, sphere_path):
|
||||
exits_outside = false
|
||||
for each portal in this.portals:
|
||||
if portal.other_cell_id == 0xFFFFFFFF: # EXIT PORTAL
|
||||
for s in 0 .. num_sphere-1:
|
||||
d = signed_dist(globaltolocal(this.frame, global_sphere[s].center), portal.plane)
|
||||
if sphere crosses the exit plane (d test w/ radius+EPSILON):
|
||||
exits_outside = true; break
|
||||
else:
|
||||
other = portal.GetOtherCell()
|
||||
if other != null: # LOADED neighbour
|
||||
for s in 0 .. num_sphere-1:
|
||||
if CCellStruct::sphere_intersects_cell(other.structure, globaltolocal(other.frame, global_sphere[s])) != OUTSIDE:
|
||||
add_cell(arr, other.cell_id, other); break
|
||||
else: # UNLOADED neighbour
|
||||
for s in 0 .. num_sphere-1:
|
||||
if sphere on outward side of portal plane (radius+EPSILON):
|
||||
add_cell(arr, portal.other_cell_id, null); break
|
||||
if exits_outside:
|
||||
CLandCell::add_all_outside_cells(pos, num_sphere, global_sphere, arr) # appended AFTER interiors (310120)
|
||||
```
|
||||
|
||||
acdream's `FindTransitCellsSphere` already mirrors this (loaded → `SphereIntersectsCellBsp`;
|
||||
unloaded → portal-plane side test; exit → `exitOutside` flag). **Divergence kept (A6.P5,
|
||||
documented):** acdream sets `exitOutside = true` *unconditionally* for an exit portal
|
||||
rather than on the plane test. Harmless under the ordered pick (outdoor only wins if no
|
||||
interior contains the center). Leave as-is for Stage 1.
|
||||
|
||||
---
|
||||
|
||||
## 4. check_other_cells (multi-valued collision + advance) @ 272717
|
||||
|
||||
```
|
||||
check_other_cells(transition, primary_cell):
|
||||
var_4c = null
|
||||
sphere_path.cell_array_valid = 1; sphere_path.hits_interior_cell = 0
|
||||
find_cell_list(&cell_array, &var_4c, &sphere_path) # ordered build + pick
|
||||
|
||||
for i in 0 .. cell_array.num_cells-1: # MULTI-VALUED collision
|
||||
cell = cell_array.cells[i].cell
|
||||
if cell != null and cell != primary_cell: # skip the already-checked primary
|
||||
r = cell.find_collisions(transition) # vtable[0x88]
|
||||
if r in {COLLIDED, ADJUSTED}: return r
|
||||
if r == SLID: contact_plane_valid = 0; return r
|
||||
|
||||
sphere_path.check_cell = var_4c # ADVANCE via the ordered pick (272761)
|
||||
if var_4c != null: adjust_check_pos(var_4c.id); return OK
|
||||
... (outdoor adjust_to_outside fallback when nothing contained) ...
|
||||
```
|
||||
|
||||
**The cell advances ONLY here, to `var_4c` = the ordered current-first pick.** Then
|
||||
`validate_transition` commits `curr_cell = check_cell` (272612) on an accepted move.
|
||||
|
||||
---
|
||||
|
||||
## 5. acdream port (the surgical change in `CellTransit.cs` + `TransitionTypes.cs`)
|
||||
|
||||
### 5.1 New type `CellArray` (Core/Physics) — §1
|
||||
`List<uint>` + `HashSet<uint>`; `Add` (ordered-dedup), `Contains`, `Count`,
|
||||
`IEnumerable<uint>` (ordered), expose ordered ids as `IReadOnlyList<uint>`.
|
||||
Implement `ICollection<uint>` so the helper signatures can widen
|
||||
`HashSet<uint>` → `ICollection<uint>` and existing `HashSet`-passing test callers
|
||||
still compile (they don't assert order). Retail: `CELLARRAY::add_cell` @701036.
|
||||
|
||||
### 5.2 `BuildCellSetAndPickContaining` — the verbatim §2 pick
|
||||
- Replace `candidates = new HashSet<uint>()` with `candidates = new CellArray()`.
|
||||
- Indoor seed: `candidates.Add(currentCellId)` (index 0). BFS expand by walking the
|
||||
growing array by index (mirror §2's forward walk), calling `FindTransitCellsSphere`
|
||||
per cell (appends in order); `AddAllOutsideCells` on `exitOutside` (appended after).
|
||||
- Outdoor seed: `AddAllOutsideCells` + `CheckBuildingTransit` (unchanged; Stage-2 will
|
||||
make entry intrinsic).
|
||||
- **PICK (verbatim):** iterate `candidates` IN ORDER; for each cell, interior point-in
|
||||
via `BSPQuery.PointInsideCellBsp(cell.CellBSP.Root, local)`; `result = candId` on any
|
||||
containing; **interior → break** (interior-wins). Outdoor fallback unchanged
|
||||
(gx/gy XY-column — acdream landcells lack a BSP `point_in_cell`).
|
||||
- **DELETE the `5ca2f44` pre-check** (lines ~522-539) — the ordered pick subsumes it.
|
||||
- `out cellSet` = `candidates.OrderedIds`.
|
||||
|
||||
### 5.3 Helpers re-typed
|
||||
`FindTransitCellsSphere`, `AddAllOutsideCells`, `AddOutsideCell`, `CheckBuildingTransit`:
|
||||
`HashSet<uint> candidates` → `ICollection<uint> candidates`. Bodies unchanged
|
||||
(`candidates.Add(id)`, `.Count`, `.Contains`). Production passes a `CellArray` (ordered);
|
||||
tests passing `new HashSet<uint>()` still compile.
|
||||
|
||||
### 5.4 `FindEnvCollisions` (TransitionTypes.cs)
|
||||
- KEEP the line-1958 pre-derive call **for Stage 1** — it is load-bearing for the
|
||||
outdoor→indoor seed promotion (the indoor BSP block is gated on `cellLow >= 0x100`;
|
||||
removing the pre-derive would strand an outdoor-seeded player who walks into a
|
||||
building). It now calls the ordered pick, so it is stable (returns the current cell
|
||||
when the center is in it). Removing it is **Stage 2** (#4 intrinsic building entry).
|
||||
- The line-2080 `FindCellSet` + `CheckOtherCells` already does the multi-valued
|
||||
collision; it now receives the ordered set. No structural change.
|
||||
|
||||
### 5.5 Persistence chain — UNCHANGED (already the SetPositionInternal/change_cell equivalent)
|
||||
`ValidateTransition` commits `sp.CurCellId = sp.CheckCellId` (TransitionTypes.cs:3427);
|
||||
`ResolveWithTransition` returns it via `SetCurrAndReturn` (writes `CellGraph.CurrCell`);
|
||||
`PlayerMovementController.UpdateCellId` applies it. This IS the `change_cell` equivalent.
|
||||
No change needed — the persistent-state half is already ported; only the pick was wrong.
|
||||
|
||||
---
|
||||
|
||||
## 6. Why this kills the flap (mapping to handoff §3)
|
||||
|
||||
- **Room ↔ room** (`0171↔0173↔0172`, constant Z): pure pick non-determinism — multiple
|
||||
overlapping interior cells contain the center; the unordered `HashSet` returned
|
||||
different ones tick-to-tick. The ordered current-first pick returns the current cell
|
||||
every tick (index 0, interior-wins-break) → **stable**.
|
||||
- **Vestibule ↔ outdoors** (`0170↔0031`): while the center is inside the vestibule BSP,
|
||||
the current-first pick keeps `0170` (interior-wins beats the outdoor fallback) → no
|
||||
swing to the outdoor render path → no full-world flash.
|
||||
- **Stairs ↔ cellar** (`0175↔0174`, foot Z oscillating ~0.2 m/tick): the ordered pick
|
||||
dampens it while the wobble stays inside the current cell's BSP, but a genuine center
|
||||
crossing will still flip (retail would too — the pick uses `point_in_cell`). The
|
||||
Z-oscillation is a **separate physics bug** (#98 family, handoff §8). Do NOT block
|
||||
Stage 1 on it; if it persists after the pick fix, it is the next target.
|
||||
|
||||
---
|
||||
|
||||
## 7. Conformance tests (extend `CellTransitFindCellSetTests`)
|
||||
|
||||
- **KEEP** `TwoOverlappingCells_CurrentCellWinsTheStraddle` (the `5ca2f44` guard) — passes under the ordered pick.
|
||||
- **ADD** a 3-cell straddle where the current cell wins by *order* (not just containment):
|
||||
current + two overlapping neighbours all contain the center; result == current from each seed.
|
||||
- **ADD** an indoor↔outdoor straddle: vestibule (interior) + outdoor landcell both contain;
|
||||
interior wins while it contains the center; outdoor wins only when it does not.
|
||||
- **ADD** a `CellArray` unit test: ordered append, dedup-by-id, order preserved across re-adds.
|
||||
- The deterministic membership net (`CellTransit|FindEnvCollisions|CellGraph|Doorway|Cellar|DoorBug`)
|
||||
+ the FULL physics suite (breakage vs §10 baseline) + the **visual flap gate**
|
||||
(`ACDREAM_PROBE_CELL`: `[cell-transit]` 59 → ~6-8) are the real verification.
|
||||
```
|
||||
|
|
@ -0,0 +1,636 @@
|
|||
# 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.
|
||||
56
src/AcDream.Core/Physics/CellArray.cs
Normal file
56
src/AcDream.Core/Physics/CellArray.cs
Normal file
|
|
@ -0,0 +1,56 @@
|
|||
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();
|
||||
}
|
||||
59
tests/AcDream.Core.Tests/Physics/CellArrayTests.cs
Normal file
59
tests/AcDream.Core.Tests/Physics/CellArrayTests.cs
Normal file
|
|
@ -0,0 +1,59 @@
|
|||
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)
|
||||
int count = ro.Count; // local avoids xUnit2013 (still exercises .Count)
|
||||
Assert.Equal(1, count);
|
||||
Assert.Equal(new[] { 7u }, ro.ToArray());
|
||||
}
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue