fix: Fixes pathfinding prebake and pathfinding multi-fallthrough. (#2478)
## Summary
Fixes the pathfinding step-cache (`.swb`) prebake so it bakes **once** and skips when a valid cache already exists, instead of re-baking on every boot. The root cause was the staleness fingerprint hashing mutable in-memory tile data rather than the on-disk files. This PR makes the fingerprint a pure function of the client data files and separates dynamic multis (houses/boats) from the static cache.
> This branch builds on the `ConfigurePrompts` first-boot-prompt unification (commit `5df8d0bd`, also included here) — that commit accounts for the `ServerConfiguration.cs` and `dev-docs/server-lifecycle.md` changes in the diff.
## The bug
With `pathfinding.prebakeMaps` set, the cache re-baked on **every** boot. The `.swb` staleness fingerprint hashed the live `TileData.LandTable`/`ItemTable` flags, which the server patches at runtime (`ItemFixes`, `LOSBlocker`, `PotionKeg`, `CTF`) at nondeterministic lifecycle points (Initialize-phase methods share a priority; static ctors fire lazily). So a fingerprint stamped at runtime (`[PathBake`) never matched the one recomputed during startup `Initialize()`, and the cache rebaked every time.
## Changes
**1. Fingerprint the files, not the in-memory tables** (`fix`)
Hash `tiledata.mul` (cached, computed once) plus the per-map `.mul`/`.uop` files — never the runtime-mutated `TileData` tables. The fingerprint is now lifecycle-stable. Existing `.swb` files rebake once after deploy, then stay stable.
**2. Compute the fingerprint once per boot** (`refactor`)
`Configure()`'s `AutoLoadAtStartup()` already opens and fingerprint-validates a reader for every up-to-date `.swb`. `Initialize()` now skips baking any map that already has an open reader (`StepCache.HasLazyReader`) instead of recomputing the fingerprint a second time.
**3. Bake static-only; route multis to the live path** (`refactor`)
Multis (houses/boats) are dynamic, so they're no longer baked into the static chunk cache — they were tagged with `BuiltMultisVersion`, a non-persisted session counter, which made persisting them unsafe (false matches / wasted re-bakes).
- Chunks bake land + `statics.mul` only.
- At query time, any cell whose sector (or its 1-cell halo) contains a multi routes to `Fallthrough_Multi` → the existing live, multi-aware `CheckMovement` path. The halo prevents a cell proposing a walkable edge into a neighbouring wall; interior (multi-free) cells pay one sector lookup.
- Adds `Sector.HasMultis` (one engine accessor); `.swb` format → v9 (rejects old multi-baked files); new `Fallthrough_Multi` telemetry.
- Behaviour-preserving: multis use the same live path the engine used before the cache existed.
**4. Comment polish** (`style`) — no behaviour change.
## Testing
All green: **92** pathfinding (incl. a new fingerprint-stability test and a multi-halo fallthrough test), **423** UOContent, **708** Server.
## Follow-ups (not in this PR)
- **Background bake worker** — make `[PathBake` and the boot prebake non-blocking (game thread serves tile reads to an off-thread worker).
- **Per-multi MCL cache** — cache walkability in each multi's own frame (keyed by multiID, movement-invariant) so houses/boats get a fast path instead of the live fallback.
- **House-pathfinding equivalence tests** — the one area not yet covered by a dedicated automated test; multi pathing is currently correct by delegation to the live path.
This commit is contained in:
Kamron Batman
GitHub
parent
eec37edd67
commit
9d26a44a28
11 changed files with 281 additions and 107 deletions
|
|
@ -114,6 +114,49 @@ public class StepCacheFileTests
|
|||
Assert.Equal(0, cache.OpenLazyReaderCount);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// HasLazyReader is the boot prebake's skip predicate (PathCacheCommands.Initialize): a map
|
||||
/// with an open, fingerprint-valid reader needs no bake. Lock the open/clear contract.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void HasLazyReader_TracksOpenAndClear()
|
||||
{
|
||||
var cache = StepCache.Instance;
|
||||
cache.Clear();
|
||||
|
||||
var map = Map.Maps[1];
|
||||
Assert.NotNull(map);
|
||||
Assert.False(cache.HasLazyReader(map.MapID));
|
||||
|
||||
// Build + save a chunk so there's a valid .swb to open.
|
||||
cache.MissPromotionThreshold = 1;
|
||||
Span<sbyte> surfZ = stackalloc sbyte[16];
|
||||
Assert.True(StepProbe.ComputeStandableSurfaceZs(map, 1500, 1600, surfZ) > 0);
|
||||
cache.TryGetMask(map, 1500, 1600, surfZ[0]);
|
||||
|
||||
var path = Path.Combine(Path.GetTempPath(), $"step-cache-haslazy-{Guid.NewGuid():N}.swb");
|
||||
try
|
||||
{
|
||||
Assert.True(cache.SaveToFile(path, map.MapID) > 0);
|
||||
cache.Clear();
|
||||
Assert.False(cache.HasLazyReader(map.MapID));
|
||||
|
||||
Assert.True(cache.TryOpenLazyReader(path, map.MapID));
|
||||
Assert.True(cache.HasLazyReader(map.MapID)); // open → true
|
||||
|
||||
cache.Clear();
|
||||
Assert.False(cache.HasLazyReader(map.MapID)); // clear closes the reader → false
|
||||
}
|
||||
finally
|
||||
{
|
||||
cache.Clear();
|
||||
if (File.Exists(path))
|
||||
{
|
||||
File.Delete(path);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void TryOpenLazyReader_BadMagic_ReturnsFalse()
|
||||
{
|
||||
|
|
|
|||
|
|
@ -0,0 +1,43 @@
|
|||
using Server.Engines.Pathing.Cache;
|
||||
using Xunit;
|
||||
|
||||
namespace Server.Tests.Pathfinding;
|
||||
|
||||
[Collection("Sequential Pathfinding Tests")]
|
||||
public class StepCacheFingerprintTests
|
||||
{
|
||||
/// <summary>
|
||||
/// Regression: the cache fingerprint must hash the on-disk tiledata.mul, NOT the mutable
|
||||
/// in-memory <see cref="TileData"/> tables. The server patches item flags/heights at runtime
|
||||
/// (ItemFixes, LOSBlocker, PotionKeg, CTF, ...) at nondeterministic lifecycle points, so a
|
||||
/// fingerprint taken over the live tables depended on WHEN it was computed: a runtime
|
||||
/// [PathBake stamped one value into the .swb and the next startup's Initialize() recomputed a
|
||||
/// different one, marking the bake stale and re-baking on every boot. Hashing the file makes
|
||||
/// the fingerprint a pure function of the client's tile data, immune to those mutations.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void Fingerprint_IgnoresRuntimeTileDataMutation()
|
||||
{
|
||||
const int mapId = 1; // Trammel — loaded by the test bootstrap.
|
||||
|
||||
var before = StepCacheFile.ComputeFingerprint(mapId);
|
||||
|
||||
const int probeId = 0x2A0;
|
||||
var original = TileData.ItemTable[probeId].Flags;
|
||||
try
|
||||
{
|
||||
// Mutate an in-memory item flag the way ItemFixes/CTF/etc. do at runtime. XOR
|
||||
// guarantees the value actually changes regardless of the current flag state.
|
||||
TileData.ItemTable[probeId].Flags ^= TileFlag.NoShoot;
|
||||
Assert.NotEqual(original, TileData.ItemTable[probeId].Flags); // sanity: mutation took
|
||||
|
||||
var after = StepCacheFile.ComputeFingerprint(mapId);
|
||||
|
||||
Assert.Equal(before, after);
|
||||
}
|
||||
finally
|
||||
{
|
||||
TileData.ItemTable[probeId].Flags = original;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -1,4 +1,7 @@
|
|||
using System.Collections.Generic;
|
||||
using System.Reflection;
|
||||
using Server.Engines.Pathing.Cache;
|
||||
using Server.Items;
|
||||
using Xunit;
|
||||
|
||||
namespace Server.Tests.Pathfinding;
|
||||
|
|
@ -205,42 +208,49 @@ public class StepCacheLifecycleTests
|
|||
}
|
||||
|
||||
[Fact]
|
||||
public void MultisVersion_Bump_TriggersDirtyRebuild()
|
||||
public void MultiCoveredCell_AndHalo_RouteToFallthrough()
|
||||
{
|
||||
var cache = StepCache.Instance;
|
||||
cache.Clear();
|
||||
cache.MissPromotionThreshold = 2;
|
||||
cache.MissPromotionThreshold = 1; // eager build so a multi-free cell serves immediately
|
||||
|
||||
var map = Map.Maps[1];
|
||||
var sector = map.GetRealSector(1500 >> 4, 1600 >> 4);
|
||||
|
||||
// First touch defers (Fallthrough_NotBuilt); second touch promotes and builds.
|
||||
Assert.Equal(CacheHitKind.Fallthrough_NotBuilt, cache.TryGetMask(map, 1500, 1600, 10).HitKind);
|
||||
Assert.Equal(CacheHitKind.Miss_NotBuilt, cache.TryGetMask(map, 1500, 1600, 10).HitKind);
|
||||
// A cell far from any multi serves from the static cache.
|
||||
Assert.True(cache.TryGetMask(map, 1500, 1600, 10).IsHit);
|
||||
|
||||
// Bump _multisVersion via reflection.
|
||||
var versionField = typeof(Map.Sector).GetField(
|
||||
"_multisVersion",
|
||||
System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance
|
||||
);
|
||||
Assert.NotNull(versionField);
|
||||
var current = (int)versionField.GetValue(sector);
|
||||
versionField.SetValue(sector, current + 1);
|
||||
// Inject a multi into an isolated sector. Sector.HasMultis only checks Count > 0, so a
|
||||
// single-entry list is enough to mark the sector as multi-bearing — the fallthrough
|
||||
// decision never dereferences the multi, so no real BaseMulti instance is needed.
|
||||
const int mx = 2000;
|
||||
const int my = 2000;
|
||||
var sx = mx >> 4;
|
||||
var sy = my >> 4;
|
||||
var sector = map.GetRealSector(sx, sy);
|
||||
var multisField = typeof(Map.Sector).GetField("_multis", BindingFlags.NonPublic | BindingFlags.Instance);
|
||||
Assert.NotNull(multisField);
|
||||
var original = multisField.GetValue(sector);
|
||||
try
|
||||
{
|
||||
multisField.SetValue(sector, new List<BaseMulti> { null });
|
||||
|
||||
// Third query: detects version mismatch, rebuilds.
|
||||
Assert.Equal(CacheHitKind.Miss_DirtyRebuild, cache.TryGetMask(map, 1500, 1600, 10).HitKind);
|
||||
// Cell inside the multi sector → routed to the live path.
|
||||
Assert.Equal(CacheHitKind.Fallthrough_Multi, cache.TryGetMask(map, mx, my, 0).HitKind);
|
||||
|
||||
var stats = cache.GetStats();
|
||||
Assert.Equal(1L, stats.MissesDirtyRebuild);
|
||||
Assert.Equal(2L, stats.BuildsTotal);
|
||||
// Cell in the adjacent sector but on the shared boundary → caught by the 1-cell halo
|
||||
// (its mask would otherwise propose an edge into the multi sector).
|
||||
var boundaryX = sx * 16 - 1; // last tile of sector sx-1; halo (x+1) reaches into sx
|
||||
Assert.Equal(CacheHitKind.Fallthrough_Multi, cache.TryGetMask(map, boundaryX, my, 0).HitKind);
|
||||
|
||||
// Mutual-exclusivity invariant: hits + miss-builds + dirty-rebuilds = served-result count.
|
||||
// Three calls returned an answer; two were "served from a build" (Miss_NotBuilt + Miss_DirtyRebuild),
|
||||
// and the first was a Fallthrough_NotBuilt (no build, slow-path signal).
|
||||
Assert.Equal(2L, stats.MissesNotBuilt + stats.MissesDirtyRebuild + stats.Hits);
|
||||
Assert.Equal(1L, stats.FallthroughNotBuilt);
|
||||
Assert.Equal(0L, stats.FallthroughMultiZ);
|
||||
Assert.Equal(0L, stats.FallthroughOffMap);
|
||||
// Two tiles out → interior of the multi-free sector, unaffected.
|
||||
Assert.NotEqual(CacheHitKind.Fallthrough_Multi, cache.TryGetMask(map, sx * 16 - 2, my, 0).HitKind);
|
||||
|
||||
Assert.True(cache.GetStats().FallthroughMulti >= 2);
|
||||
}
|
||||
finally
|
||||
{
|
||||
multisField.SetValue(sector, original);
|
||||
}
|
||||
}
|
||||
|
||||
[Fact]
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue