9 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Kamron Batman
|
b852bca41e
|
perf(pathing): pool the StepCache strata buffer, then clean up the pathing engine around it (#2523)
Started as an allocation pass over `StepCache` and grew into a cleanup of the surrounding pathing engine. Four commits, each independently reviewable; net **−560 lines**. Build clean (0 warnings). All 122 `Server.Tests.Pathfinding` tests pass. --- ## 1. `perf`: pool the strata buffer, cut a hot-path dictionary lookup **The headline is that `TryGetMask` — the actual hot path — was already allocation-free.** `StepMask` is a readonly struct, `StaticTileEnumerable` is a `ref struct`, `ChunkMissState` is a struct in a `Dictionary`. So most of this is a bake-throughput and GC-churn win, with one exception noted below. `BuildChunk` accumulated packed multi-Z strata into a `List<byte>` that grew by doubling (256 → 512 → 1024 → …) and then paid a final `ToArray()`. A full map bake runs it ~114k times. It now writes into a `byte[]` rented from `STArrayPool<byte>.Shared` through a span writer, and hands the chunk one exact-size copy. **This required fixing a latent out-of-bounds guard.** The record-fit check reserved headroom for **8** strata (`StratumByteLength * 8`) while `ComputeStandableSurfaceZs` can return up to **16** — so a cell could write 305 bytes starting from a 65,383-byte offset. Against a `List` that was benign (it just grew past 64 KB, and emitted offsets stayed under the `NoStrata` sentinel). Against a fixed-size rented buffer it is an out-of-bounds write, so tightening it was a *prerequisite* for the pooling, not a drive-by. The guard is now exact, which additionally proves no emitted offset can collide with `NoStrata == ushort.MaxValue`. **One genuine query-path win:** `ShouldPromoteAfterMiss` did *two* dictionary lookups per miss — a `TryGetValue`, then an indexer assignment that re-hashes and re-probes. It now mutates in place via `CollectionsMarshal.GetValueRefOrNullRef`. This runs on every uncached chunk touch during A* expansion. The window-expiry branch keeps its explicit early return, so `MissPromotionThreshold == 1` still resets rather than promoting. Also dropped `StepProbe.ComputeStrataAt` / `ComputedStratum` (dead code, zero callers) and collapsed six 18-argument `new StepMask(0, 0, …, kind)` blocks into `Fallthrough(kind)`. **Considered and rejected:** pooling the `Direction[]` that `Find` returns. It *escapes* the call — `MovementPath` holds it across ticks while `PathFollower` walks `m_Index` through it — so it cannot be rented-and-returned, and it cannot be borrowed from the shared `BitmapAStarAlgorithm.Instance` without one creature clobbering another's in-flight path. `CheckPath` rate-limits repaths to one per 2s per creature, putting this at roughly 60 KB/sec at 1,000 pathing creatures. Not worth a public API break plus a use-after-return footgun. ## 2. `docs`: rewrite the comments for publication The comments had accumulated as development notes: internal phase jargon (`Tier 4`, `the Phase-2 synthesizer`), change narration aimed at a reviewer (`which the old ComputeStandingZ anchor missed`, `legacy behavior`), benchmark anecdotes (`benchmarked as near-optimal`, `a ~20 ns lookup`), and paragraphs restating the code. Rewritten to keep the rationale you cannot recover by reading the code — why the source-Z guard cannot be widened, why multis fall through with a halo, why the promotion gate counts Finds rather than calls, why `ComputeFingerprint` must hash the *files* and not the live tile tables — and drop the history that got us there. Three comments were **factually wrong**, not just wordy: - `CacheEvictionTimer` and `CacheStats` documented a class called `StaticWalkabilityCache`. No such class exists — it is `StepCache`. - `StepCacheFile` declared `File layout v8` while `FormatVersion` is 9, and called the current record layout "the v6 layout" in four places. The layout descriptions are now unversioned so they cannot drift again. - `StepProbe.ComputeStandingZ` claimed `StepCache` uses it to bake `SourceZ`. It has not since the baker moved to the clearance-aware `ComputeStandableSurfaceZs`; only a parity test calls it. ## 3. `refactor`: simplify `StepCacheFile.Write`, consolidate the format tests `SaveToFile` walked `_keysList` **twice** — once to count the map's chunks, then again through a `ChunkEnumerator` closure to emit them — because `Write` needed the count up front to size its index array. Both loops had the same root cause. Passing a **span** collapses them: the count is just `span.Length`. That deletes the `ChunkEnumerator` delegate, the closure over the list enumerator, and **both `InvalidOperationException` throws**, which existed only to police the delegate's "yield exactly `chunkCount` chunks" contract — a contract a span makes unrepresentable. `Write` now patches the header's `IndexOffset` by seeking back to it rather than reaching into the writer's live buffer with `BinaryPrimitives`. That also retires `IndexOffsetFieldPosition`, a hand-maintained byte offset that had to track the header layout, and sidesteps the stale-array hazard that motivated the manual patch (`BufferWriter` reallocates on growth). **Tests:** `StepCacheFileV6/V7/V8Tests` were named for the format version that introduced each transform — and the format is now **v9**, so all three names described formats the loader rejects outright. Beyond triplicated builders and plumbing, two things were actually broken: - The three near-identical rejection tests each cited a `MinSupportedVersion` that had since moved (`"version 5 < MinSupportedVersion 6"`, `"6 < 7"`, `"7 < 8"`). They passed for the wrong reason. - `AssertBaseEqual` (used by V7 and V8) **silently skipped the swim and strata trailers**. A regression dropping either would not have failed those tests. Now one `StepCacheFileFormatTests`, named for behavior — predictive-Z elision, compression, compact index — with a single `AssertIdentical` that does check both trailers, the three rejection tests folded into one theory that also covers a future version, and a zero-chunk case the delegate-based writer never had coverage for. ## 4. `test`: consolidate the parity and lifecycle tests Three files tested "parity" and none of the names said *which*. They were three different layers, and the seams are the useful part, so they are now one `StepCacheParityTests` that names them: | Test | Compares | Answers | |---|---|---| | `ProbeMatchesSlowPath` | StepProbe vs MovementImpl | Is the bake right? | | `CacheMatchesProbe` | StepCache vs StepProbe | Is it stored and returned intact? | | `CacheServesReachableWalkStates` | StepCache vs MovementImpl | End to end, over the states A* visits | Merging removed a duplicated stub `Mobile`, duplicated region seeds, and a filename/class mismatch (`StepProbeParityTests.cs` declared `StaticWalkabilityParityTests`). `SwimBake_ProducesWetCells` moved with it — it lived in the cache parity file but never touched the cache. Tests reached into `StepCache._chunks` via `GetField` in **9 places**, each rebuilding the key encoding and cell-index arithmetic by hand. `StepCache` now exposes `GetResidentChunk` and `ResidentIndexInSync` alongside the internal test hooks it already had (`LazyReaderHasChunk`, `CurrentFindGeneration`), and the shared arithmetic moved to `PathingTestSupport`. All 9 reflection blocks are gone. `StepCacheLifecycleTests` is regrouped by what it covers — promotion gate, fallthrough routes, strata, swim layer, eviction — with the `Tier4*` names dropped. Removed `Singleton_IsAvailable`, which asserted an inline-initialized static property was not null; that is the entire 123 → 122 test-count delta. --- ## Verification Tests were mutation-checked rather than just run, since round-trip and parity tests can pass while a transform silently no-ops: - Injecting an off-by-one into the `IndexOffset` patch fails **15 of 123** — the format tests are load-bearing. - Offsetting the cache's cell index by one fails **7 of 10** parity cases, and the 3 that stay green are exactly the ones that do not touch the cache. The layering localizes a fault rather than just reporting one. |
||
|
Kamron Batman
|
1c2e114b21
|
feat(pathfinding): multi-aware mask synthesizer + warm interior cache for house/boat cells (#2479)
## Problem Houses and boats (multis) were pathed correctly only by **delegation to the slow path**: `StepCache.TryGetMask` returns `Fallthrough_Multi` for any multi-covered cell, and `GetSuccessors` ran `CheckMovement` **8× per cell** (each re-resolving the tile stack via `GetStaticAndMultiTiles`) — a sustained per-step cost near every house/boat. There was also no automated test pinning multi pathfinding. This branch is the full multi-pathfinding effort in phases on one branch. ## Phase 1 — characterization tests (the oracle) Implementation-agnostic invariants: a cache-on≡cache-off whole-path invariant, a per-cell sweep vs `CheckMovement` over footprint+halo (incl. destination Z), hand-verified routing (around walls, demolish-reopens, foundation-redesign-honored), classic-house / foundation / boat fixtures, non-vacuity guards. These gate every later phase byte-for-byte. ## Phase 2 — live single-pass synthesizer `StepProbe.ComputeMultiMaskAt` synthesizes a covered cell's full 8-direction `StepMask` in one pass (the existing surface/step logic over `GetStaticAndMultiTiles` instead of 8× `CheckMovement`). `GetSuccessors` routes `Fallthrough_Multi` cells through it. No new cache, no `.swb` change. **~1.5×**, zero added allocations. ## Phase 3 / 3.1 — warm per-`multiID` interior cache (airtight) `MultiMaskCache` caches each fixed multi's local-frame `StepMask` for **interior** cells (cell + all 8 neighbours covered → terrain-neighbour-free → position-invariant), keyed by `multiID & 0x3FFF`, built lazily from the MCL. Interior cells become ~20 ns lookups. The cache is gated on a **per-instance footprint-clean flag** (`BaseMulti.PathInteriorCacheState`): an instance whose whole footprint terrain is below its floor (`maxTerrain < minFloor`) serves from the cache; a **dirty** instance (terrain intrudes — a contrived/GM placement) **degrades to live-synth, never a wrong mask**. This closes a cross-instance soundness gap (the cached mask depends on neighbour terrain too) found in a holistic review. The gate resets whenever the footprint's world-terrain relationship can change — **location, map, or ItemID** (a boat's heading swaps the MCL). **Boats are cached too.** Their per-`multiID` deck masks are movement-invariant (built once per heading), so a sailing boat never rebuilds them; only the cheap clean-flag rescan repeats per move (and only when pathed near). Narrow existing boats have little interior; wide galleons (`multi.mul`) would gain Castle-class. `HouseFoundation` (per-instance runtime `DesignState`) is the one type that stays on the live path. ## Verification - `UOContent.Tests` **454/454**, `Server.Tests` **708/708**, 0 failures. - The Phase-1 oracle (`MultiPathInvariantTests`, cache-on ≡ cache-off) stays **byte-identical** with the synthesizer + interior cache active. - Tests pin: footprint-cleanliness (clean vs sunk), dirty/cluttered placement degrades to live-synth while still pathing, clean placement serves, and the gate resets on move/ItemID change. ## Performance (modernuo/ModernUO-Benchmarks#8, full-fixture) Houses at **Green Acres** (flat staff region → clean footprints, the legit-placement case): | Route | Slow path | Phase 3.1 (interior cache) | Speedup | |-------|----------:|---------------------------:|--------:| | `around_a` (29 steps) | 238.3 µs | **49.1 µs** | **4.85×** | | `around_b` (29 steps) | 224.3 µs | **49.5 µs** | **4.53×** | ~130 of ~167 multi cells/route serve from the cache (~20 ns) vs 37 live-synth. Per-cell, the slow path's 8× `CheckMovement` grows with multi complexity (GuildHouse ~857 ns → Castle ~1,194 ns), the synthesizer is a flat ~780 ns, and the cache serve is ~20 ns — so big/tall multis (and wide galleons) gain most. Identical allocations throughout. |
||
|
Kamron Batman
|
30fec7da26
|
fix: Cleans up AI Pathfinding code to make it more portable for custom requirements. (#2474) | ||
|
Kamron Batman
|
c7aaf33de9
|
feat(pathfinding): .swb format v8 compact index (#3b) (#2471)
## Summary Phase #3b (final roadmap item), stacked on #2470. Compacts the index trailer from 20 to 8 bytes/chunk. Trammel: 19.2 MB → 17.9 MB. Roadmap total: 565 MB → 17.9 MB (−96.8%). ## Details - Trailer stores `{ u32 packedKey = (ChunkX << 16) | ChunkY, u32 recordLength }` per chunk, in record write order; the file offset is dropped and reconstructed by cumulative recordLength from HeaderSize. - No record reordering, no varint; fixed-stride, TryReadChunk unchanged. - Also simplifies the accumulated `.swb` code comments across the stack. - Format v8; v7 files rejected and re-baked once. ## Tests v8 multi-chunk round-trip (cumulative offset reconstruction) + the v6/v7 suite; full pathfinding suite green; Release build clean. |
||
|
Kamron Batman
|
9a3d88988c
|
feat(pathfinding): non-eager TryGetMask + second-touch promotion (#2451)
## Summary Closes the Cold-cache regression flagged in PR #2450. `StepCache.TryGetMask` no longer eagerly runs `BuildChunk` on the first miss for a chunk that isn't in a `.swb` lazy reader. Instead it returns `Fallthrough_NotBuilt` and the caller (`BitmapAStarAlgorithm`) takes the per-cell slow path. The chunk is only promoted to the bitmap fast path after the **second** miss within a 30-second window, filtering single-touch pass-throughs. This makes BitmapAStar's worst-case (cold cache + short hops) collapse from **12–47× slower** than FastAStar to **roughly the same**, which is the floor the slow path can deliver. Steady-state warm performance (the actual deliverable) is unchanged from PR-5 — it was always the cache fast path. ## The pet-follow scenario this fixes A mounted player at ~4 tiles/sec with a pet/hireable following will trigger an NPC pathfind every 100–300 ms. Each pathfind is 1–6 tiles. As the player crosses chunk boundaries (~4 sec/chunk), the pet's first pathfind in the new chunk under the previous behavior triggered a full ~700 µs `BuildChunk` for a chunk the player would leave shortly after. At 50–100 mobiles per shard, this exceeded the 8 ms tick budget. PR-5 BDN data showed scenarios 6–9 (2–8 tile NPC perception) at 2,300–3,700 µs Cold vs FastAStar's 80–200 µs. Under the new gate: - First miss → `Fallthrough_NotBuilt` → caller uses slow path (~30–50 µs short path). No `BuildChunk`. No allocation. - Player keeps moving → chunk never gets a second touch within window → never promoted, no rot. - NPC patrolling a fixed territory → repeatedly hits the same chunks → second touch within window → promote → cache fast path on subsequent calls. ## What changed - **`CacheHitKind.Fallthrough_NotBuilt = 6`** + **`CacheStats.FallthroughNotBuilt`** counter. `IsHit=false`, so the caller routes to slow path. - **`StepCache._chunkMissTracker`** — `Dictionary<long, ChunkMissState>` capped at 4096 entries. State is `(byte missCount, uint lastMissTickStamp)` keyed by chunk key. Window-expired entries reset count to 1; capacity overflow prunes window-old entries first. - **`StepCache.MissPromotionThreshold`** (default `2`) and **`StepCache.MissPromotionWindowMs`** (default `30_000`) — tunable, can be wired through `ServerConfiguration` if shards want different policy. Setting threshold to `1` restores legacy eager-build behavior (used by tests that prime chunks via single `TryGetMask` call). - **`StepCache.TryGetMask` miss branch** — try lazy reader first (file-loaded chunks bypass the tracker entirely; an `.swb` represents an explicit prior decision to keep the chunk warm). Otherwise consult the tracker. - **`BitmapAStarAlgorithm.GetSuccessorsSlowPath`** now layers `IsBlockedByDynamic` on top of `CalcMoves.CheckMovement`. Previously the slow path only ran for `CanFly` creatures and rare cache fallthroughs — `CheckMovement` doesn't iterate same-cell mobiles, so the bitmap fast path's `IsBlockedByDynamic` was the only mobile-blocking check. Now first-touch pathfinds run through the slow path, so the gap had to close. ## Tests 50 pathfinding tests pass (was 47). New / updated: - **`TryGetMask_FirstTouchOnUnbuiltChunk_DefersBuildAndReturnsFallthrough`** — single TryGetMask call returns `Fallthrough_NotBuilt`, no chunk built, no allocation. - **`TryGetMask_SecondTouchWithinWindow_PromotesAndBuilds`** — second call inside the 30s window builds + serves. - **`TryGetMask_SecondTouchAfterWindow_RestartsCounterAndDefers`** — second call outside the window restarts the count, returns Fallthrough again. - **`TryGetMask_DistinctChunks_TrackedIndependently`** — counters are per-chunk; one touch on each of two adjacent chunks both stay in fallthrough. - **`LazyReaderHit_BypassesMissTrackerOnFirstTouch`** — open `.swb` + first touch hits without consulting the tracker. Production with `.swb` loaded skips the gate entirely. - **`MultisVersion_Bump_TriggersDirtyRebuild`** — updated to reflect the new 3-step flow (Fallthrough → Miss_NotBuilt → Miss_DirtyRebuild). - Tests that prime chunks via a single `TryGetMask` call (multi-Z, Tier4, lifecycle, parity, BitmapAStar uses-cache) set `MissPromotionThreshold = 1` to opt into eager behavior. ## Expected BDN impact The Cold column from PR-5's BDN should change as follows once the bench's submodule pointer is updated to this branch: | # | Scenario | Cold (PR-5) | Cold (PR-6 expected) | FastAStar Cold | |--:|-----------------|-------------:|---------------------:|---------------:| | 2 | sewer corridor | 1,627 µs | ~36 µs | 36 µs | | 4 | causeway | 1,533 µs | ~39 µs | 39 µs | | 6 | pet 2-tile | 2,364 µs | ~80 µs | 81 µs | | 8 | npc 5-tile | 3,708 µs | ~140 µs | 141 µs | | 9 | npc 8-tile | 2,386 µs | ~200 µs | 197 µs | WarmNoFile and LazyWarm rows should be unchanged — they were always cache-warm. The miss tracker only fires when neither resident chunks nor the lazy reader can satisfy the request. ## Future work (not in this PR) - **Background-thread bake**: builds outside the game thread so even promoted chunks don't pay the 700 µs build cost on the main thread. Rule 10 (no Task.Run) applies, so this needs careful design — the bake is a pure data transform but main-thread synchronization on chunk-state transitions has to be threaded through. Defer to a follow-up. - **Long-traverse BDN scenario**: a multi-Find benchmark simulating 50 pet repaths across chunk transitions. Requires restructuring the bench harness; the existing 10-scenario corpus + Cold provider already exercises the gate. - **Swim sourceZ bake**: scenario 5 (sea serpent) shows 56 B alloc on warm paths because the cache's SourceZ is computed under default-walker rules. Swim creatures fall through to slow path. Independent of this PR. |
||
|
Kamron Batman
|
cff9fbda29
|
fix(ai): creatures pathfind around concave obstacles instead of oscillating (#2461)
## Summary Creatures (pets following, monsters chasing, NPCs approaching) would **oscillate — "pace back and forth really fast"** at concave obstacles (reported at the Britain Inn L-desk: a pet at `(1493,1614,20)` never reaching its master at `(1494,1605,21)`) instead of routing around them. **Root cause** (the A* pathfinder itself was correct): all goal-seeking funnels through `MoveTo` and `WalkMobileRange` → `MoveTowardsOrAwayFrom`, which step greedily via `DoMove(dir, badStateOk:true)`. `DoMove` returns `true` even when the direct step was blocked and the creature merely **auto-turned and sidestepped** (`MoveResult.SuccessAutoTurn`), and the caller then set `Path = null`, discarding the `PathFollower`. So at a concave obstacle a non-progressing sidestep was mistaken for progress and the creature never committed to a route. (AOS pet-follow runs at `CurrentSpeed = 0.1`, hence the "really fast" shuffle.) ## What changed - **New centralized `BaseAI.ApproachTarget(target, run, range)` primitive.** A greedy step is committed only when it **fully succeeds (`MoveResult.Success`) and actually gets closer**; otherwise the creature commits to a **persistent `PathFollower`** that routes around the obstacle and is never discarded by a greedy step. The open-terrain fast path (one greedy step, no pathfinding) is preserved. `MoveTo`, `MoveTowardsOrAwayFrom`, and `MoveToWithCollisionAvoidance` all delegate to it — public signatures unchanged, so no AI-class call site changes. - **Best-distance give-up + idle.** A creature that cannot reach a **stationary** in-range goal stops shuffling and idles after `ApproachGiveUpTicks` (40) ticks without lowering its closest-ever distance; a **moving** goal (active chase) never gives up. It resumes the moment the goal moves. - **Pathfinder fix (required):** `BitmapAStarAlgorithm.IsBlockedByDynamic` now skips the dynamic mobile-block check **at the goal cell only** (`MoveImpl.Goal`). Previously A* returned `null` whenever the target mobile stood on the goal cell, so creatures could never pathfind *toward* another mobile — only toward empty ground. The follower stops within `range` short of it. Static/item blocking and all non-goal mobile blocking are unchanged. ## Tests New AI-loop integration tests in `ApproachTargetTests.cs` drive the real `BaseAI` primitives against live Britain Inn map statics: exact-repro pet follow, open-terrain (asserts zero pathfinding), `MoveTo` chase (static + walking-away target), route-around-a-dynamic-wall, and walled-off give-up-and-idle. - Pathfinding + AI subset: **52/52** pass. - Full `UOContent.Tests`: **301/301** pass. (Note: the test host lingers on shutdown — a pre-existing infra quirk unrelated to this change; all tests complete and pass.) - Full solution build: clean (0 warnings / 0 errors). ## Notes - Branched off `main`; independent of the in-flight step-cache work. - Out of scope (future work): proactive "SmartAI" look-ahead pathfinding so clever creatures plan a route before walking into the obstacle, rather than reacting after they hit it. ## Test Plan - [X] In-game: order a pet to `follow`/`come` across the Britain Inn L-desk; confirm it routes around and reaches you instead of pacing. - [X] Aggro a monster and kite it around a building/treeline; confirm it chases around obstacles. - [X] Confirm open-terrain following/chasing feels unchanged (no extra latency). - [X] Confirm a creature with a genuinely unreachable target idles rather than shuffling forever. |
||
|
Kamron Batman
|
a8ca82738d
|
feat(pathfinding): JSONL recorder + public bake helpers (#2449)
## Summary Adds two pieces of pathfinding tooling on top of PR #2448's lazy `.swb` infrastructure: - **`PathfindRecorder`** — admin-toggled JSONL telemetry capture; one record per `BitmapAStarAlgorithm.Find` call. Output format matches the BDN harness corpus, so production traffic can be captured and replayed in benchmarks without an adapter. - **Public bake helpers on `StepCache`** — `ComputeLiveTileDataHash`, `TryReadTileDataHashFromFile`, `BakeMap`, `ClearResidentChunks`. Lets the benchmark project (and any future bake utility) drive cache fill + persist without exposing internal types. The companion BDN harness update lives in [ModernUO-Benchmarks#kb/pathfinding-pr4-bench](https://github.com/modernuo/ModernUO-Benchmarks/tree/kb/pathfinding-pr4-bench): porting `Benchmarks/PathfindInGame/` from the `kb/ai_pathfinding` branch to the API shipped in #2446–#2448. ## What's in this PR ### `PathfindRecorder` (`PathfindRecorder.cs`) - Holds a single `StreamWriter` open while recording; its internal buffer absorbs per-record writes without per-call `File.AppendAllText`. - Single `bool` check on the hot path; cheap when disabled. - Disabling flushes + disposes; an IO failure during write also disables the recorder. - Server config: - `pathfinding.recorder.enable` — bool, default `false`. Read on boot via `GetOrUpdateSetting`. - `pathfinding.recorder.path` — default `<basedir>/Data/Pathfinding/recordings/pathfinds.jsonl`. - Hooked into `BitmapAStarAlgorithm.Find` — runs once per call, does nothing when disabled. - Admin command: `[PathRecord [on|off|flush|status]` (default `status`). ### Public cache helpers - `static ulong StepCache.ComputeLiveTileDataHash()` — wraps the file module's hash function for staleness checks. - `static bool StepCache.TryReadTileDataHashFromFile(string, out ulong)` — peeks at a `.swb` file's hash field (20 bytes). - `int StepCache.BakeMap(int, string)` — walks every chunk in the map, populates resident set, saves. Offline / fixture use; blocks for many seconds on a full-map walk. - `void StepCache.ClearResidentChunks()` — drops chunks + zeros counters but keeps lazy readers open. Lets benchmark loops measure "first query after boot" cost across iterations without the lazy-reader reopen overhead. |
||
|
Kamron Batman
|
9066e8fd00
|
feat: expand cache to nearly all mobiles + dynamic-obstacle pass (#2447)
## Summary Builds on PR #2446's cache-direct A*. The previous PR conservatively routed players + creatures with capability flags entirely through the slow path. This PR pushes that line: most mobile classes now use the cache, with the right rule set layered on top per-mobile, and the cache fast-path now does the dynamic items / mobiles check that PR #2446 had silently skipped. ## What changed - **Non-GM players** now use the cache. Diagonal corner-cut applies the strict AND-rule (BOTH cardinal partners walkable) by reading the same source-cell mask byte the creature OR-rule reads — both rules are evaluable from one byte. - **Creatures with `CanOpenDoors` / `CanMoveOverObstacles`** now use the cache. Reading `MovementImpl` confirmed those flags only affect dynamic items, never static tiles, so they were over-conservatively excluded before. - **Swim creatures** now use the cache via a capability overlay. `StepProbe` bakes a second rule set (`canSwim=true, cantWalk=true`) producing `WetMask` + `SwimZ_*`. The algorithm composes `effectiveMask = (walkMask & !cantWalk) | (wetMask & canSwim)` per direction; walk Z preferred when both apply. - **Dynamic-obstacle pass.** Cache fast-path now mirrors `MovementImpl`'s per-cell items + mobiles collision check (`GetItemsAt` / `GetMobilesAt` at the target cell, with `CanOpenDoors` / `CanMoveOverObstacles` / spell-field overrides). This closes a correctness gap from PR #2446 — the cache fast-path was silently skipping dynamic obstacles entirely. - **`StepCache.TryGetMask` returns `StepMask` struct** instead of 11 out parameters. `HitKind` rolls into the struct with an `IsHit` accessor. Sets up wet/swim without ballooning the call site. - **`StepChunk.MultiZCells` is lazy-init.** Most chunks are entirely single-Z; allocating the 32-byte bitmap up-front wasted ~256KB at full cap. - **Admin commands.** `[PathCacheStats` (resident chunks + hit/miss/eviction counters) and `[PathCacheClear` (drop everything, zero counters). - **Feature flag.** `bitmap_pathfinding_cache` (default true) gates the cache fast-path. Flipped off, every cell expansion routes to `MovementImpl` — equivalent to PR #2446's slow-path-only behavior. Safety net for shipping the new behavior. `RequiresSlowPath` shrinks to just `CanFly` — flying creatures Z-jump arbitrarily, which the cache's static-Z model can't accommodate. |
||
|
Kamron Batman
|
6a3804addc
|
feat: Replace FastAStarAlgorithm with BitmapAStarAlgorithm (#2446)
## Summary Replaces `FastAStarAlgorithm` with `BitmapAStarAlgorithm`: one cache lookup per cell expansion (8-direction mask + per-direction destination Z) instead of 8 separate `MovementImpl.CheckMovement` calls. Adds the supporting cache infrastructure to back it. Public API unchanged — `MovementPath` / `Mobile.Move` / `CalcMoves.Find` return the same shapes; the algorithm swap is internal. ## What's in this PR - **`BitmapAStarAlgorithm`** — A* that issues one `StepCache.TryGetMask` call per cell expansion. Inline fallthrough to the per-cell slow path for multi-Z, off-map, source-Z mismatch, and non-default walkers. - **`StepCache`** — singleton chunk store keyed by `(mapId, chunkX, chunkY)`. Lazily built on first query, invalidated by `Sector.MultisVersion` mismatch, memory-bounded by sampled probabilistic LRU. - **`StepProbe`** — computes static-only walkability for a single cell, mirroring `MovementImpl.Check` minus the item / mobile collision phases. - **`StepMask` / `StepChunk`** — value / storage types for the per-cell results. - **`CacheEvictionTimer`** — periodic cap backstop (60s interval; early-returns when not over cap). - **`Map.Sector.MultisVersion`** promoted to `public` so the cache can detect dynamic-static invalidations cheaply. ## Eviction strategy Sampled probabilistic LRU (Redis-style). Per eviction, sample 5 random keys from a parallel `List<long>` kept in lockstep with the chunk dictionary; evict the oldest of the sample via swap-and-pop. O(1) per eviction regardless of resident count, so sustained cap pressure has no perpetual perf hit. ## Capability handling (interim) Non-default walkers (non-GM players, creatures with `CanSwim` / `CanFly` / `CanOpenDoors` / `CanMoveOverObstacles`) route entirely through the per-cell slow path via `BitmapAStarAlgorithm.GetSuccessorsSlowPath`. The 2-pass design (cache + capability overlay + dynamic-obstacle pass) lands in the follow-up PR. |
Renamed from Projects/UOContent/Engines/Pathing/FastAStarAlgorithm.cs (Browse further)