erik/acdream
1
0
Fork 0
Code Issues Pull requests Projects Releases 6 Packages Wiki Activity Actions

fix(vtank): slice 7 round E item D-1 — map legacy JSON recall ordinals

Browse source 
The pre-cutover RouteRecallKind was {Lifestone=0, Marketplace=1,
PrimaryPortal=2, SecondaryPortal=3} — completely different kinds at the
SAME ordinals as today's much larger round-D enum (PrimaryPortalRecall=0,
SecondaryPortalRecall=1, LifestoneRecall=2, LifestoneSending=3, ...).
LegacyWaypointDocument.Recall was still typed RouteRecallKind, so
System.Text.Json deserialized a legacy JSON route's bare "Recall"
integer straight into the new enum — silently remapping every one of
the four old ordinals to the wrong new recall. Enum.IsDefined never
caught it (0..3 are all valid members of the new enum too, just for
different spells).

The field is now a bare int, translated through an explicit
MapLegacyRecall(legacyOrdinal) table in ToWaypoint() that also derives
RecallSpellId/RecallSpellName from the mapped kind (the pre-cutover
schema predates both fields entirely).

Mutation shown to fail first: reverting the source fix (git checkout,
patch saved and reapplied) reproduced the bug for all four legacy
ordinals — ordinal 2 (old PrimaryPortal) even landed on LifestoneRecall
instead of PrimaryPortalRecall, confirming the "still IsDefined, still
wrong" failure mode. The new theory asserts against the migrated
route's own written .af "rcl" line (RecallDisplayName) rather than the
value re-parsed back through LoadCurrent, since the .af format's
own name-only round-trip is a separate, pre-existing limitation
(no spell id, and "Marketplace Recall" cannot resolve through any
catalog because it isn't a real spell) unrelated to this fix.

MossTank suite 716 -> 720; App markup/plugin filter holds 243/243.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
Erik 2026-09-07 17:56:27 +02:00
parent 3178d9202b
commit a9d1d0a823
2 changed files with 132 additions and 36 deletions
Download patch file Download diff file Expand all files Collapse all files

102
src/AcDream.Plugins.MossTank/MossTankRouteProfileStore.cs
View file

@ -574,7 +574,19 @@ internal sealed class MossTankRouteProfileStore
public bool LegacyReferenceValid { get; set; } = true;
public string Text { get; set; } = string.Empty;
public int DurationMilliseconds { get; set; } = 5000;
public RouteRecallKind Recall { get; set; }
// Round E item D-1: this field used to be typed RouteRecallKind,
// which let System.Text.Json deserialize the pre-cutover JSON's
// bare integer straight into TODAY's enum. The pre-cutover enum
// was {Lifestone=0, Marketplace=1, PrimaryPortal=2,
// SecondaryPortal=3} — completely different kinds at the SAME
// ordinals as the round-D enum (PrimaryPortalRecall=0,
// SecondaryPortalRecall=1, LifestoneRecall=2, LifestoneSending=3,
// ...), so every legacy route silently remapped to the WRONG new
// recall and Enum.IsDefined never caught it — 0..3 are all still
// valid members of the new enum, just for different spells. Read
// as a bare ordinal here instead and translate it explicitly via
// MapLegacyRecall in ToWaypoint().
public int Recall { get; set; }
public uint RecallSpellId { get; set; }
public string RecallSpellName { get; set; } = string.Empty;
public float JumpHeadingDegrees { get; set; }
@ -582,42 +594,60 @@ internal sealed class MossTankRouteProfileStore
public int JumpChargeMilliseconds { get; set; } = 1000;
public RouteJumpDirection JumpDirection { get; set; }
public RouteWaypoint ToWaypoint() => new()
public RouteWaypoint ToWaypoint()
{
Type = Enum.IsDefined(Type) ? Type : RouteWaypointType.Point,
Position = new PluginNavigationPosition(
CellId, EastWest, NorthSouth, Elevation, HeadingDegrees, IsOutdoor),
ReferencePosition = new PluginNavigationPosition(
ReferenceCellId,
ReferenceEastWest,
ReferenceNorthSouth,
ReferenceElevation,
ReferenceHeadingDegrees,
ReferenceIsOutdoor),
ObjectId = ObjectId,
ObjectName = ObjectName ?? string.Empty,
LegacyObjectClass = LegacyObjectClass,
LegacyReferenceValid = LegacyReferenceValid,
Text = Text ?? string.Empty,
DurationMilliseconds = Math.Clamp(DurationMilliseconds, 0, 3_600_000),
// Round D item 3 removed RouteRecallKind.Lifestone (the old
// /lifestone-slash-command member, superseded by the real
// LifestoneRecall spell) — a pre-cutover un-migrated JSON
// profile carrying that ordinal now falls back to
// PrimaryPortalRecall instead, the same "still a valid,
// harmless recall kind" sentinel role Lifestone played here.
Recall = Enum.IsDefined(Recall) ? Recall : RouteRecallKind.PrimaryPortalRecall,
RecallSpellId = RecallSpellId,
RecallSpellName = RecallSpellName ?? string.Empty,
JumpHeadingDegrees = float.IsFinite(JumpHeadingDegrees) ? JumpHeadingDegrees : 0f,
JumpRun = JumpRun,
// Round 3 item 5: no load-time clamp here either — the real
// bi.a 2000 ms ceiling is retail's EXECUTION-time behavior
// (NavigationController.TickJump), not a storage-format limit;
// this legacy migration path preserves whatever the pre-cutover
// JSON authored, exactly like the .af load path now does.
JumpChargeMilliseconds = JumpChargeMilliseconds,
JumpDirection = Enum.IsDefined(JumpDirection) ? JumpDirection : RouteJumpDirection.Forward,
RouteRecallKind recall = MapLegacyRecall(Recall);
return new RouteWaypoint
{
Type = Enum.IsDefined(Type) ? Type : RouteWaypointType.Point,
Position = new PluginNavigationPosition(
CellId, EastWest, NorthSouth, Elevation, HeadingDegrees, IsOutdoor),
ReferencePosition = new PluginNavigationPosition(
ReferenceCellId,
ReferenceEastWest,
ReferenceNorthSouth,
ReferenceElevation,
ReferenceHeadingDegrees,
ReferenceIsOutdoor),
ObjectId = ObjectId,
ObjectName = ObjectName ?? string.Empty,
LegacyObjectClass = LegacyObjectClass,
LegacyReferenceValid = LegacyReferenceValid,
Text = Text ?? string.Empty,
DurationMilliseconds = Math.Clamp(DurationMilliseconds, 0, 3_600_000),
Recall = recall,
// The pre-cutover JSON schema predates RecallSpellId/
// RecallSpellName entirely (both are a round-D addition),
// so a legacy document never carries real values for
// them — derive both from the SAME mapped kind, the one
// table this migration path uses end-to-end.
RecallSpellId = RouteWaypoint.SpellIdForRecall(recall),
RecallSpellName = RouteWaypoint.RecallDisplayName(recall),
JumpHeadingDegrees = float.IsFinite(JumpHeadingDegrees) ? JumpHeadingDegrees : 0f,
JumpRun = JumpRun,
// Round 3 item 5: no load-time clamp here either — the real
// bi.a 2000 ms ceiling is retail's EXECUTION-time behavior
// (NavigationController.TickJump), not a storage-format limit;
// this legacy migration path preserves whatever the pre-cutover
// JSON authored, exactly like the .af load path now does.
JumpChargeMilliseconds = JumpChargeMilliseconds,
JumpDirection = Enum.IsDefined(JumpDirection) ? JumpDirection : RouteJumpDirection.Forward,
};
}
// Round E item D-1: the pre-cutover RouteRecallKind ordinals,
// mapped to today's real spell-based kinds. Anything outside that
// 4-value range is impossible (the old enum only ever had 4
// members) so it falls back to PrimaryPortalRecall — the same
// "still a valid, harmless recall kind" sentinel role the old
// Enum.IsDefined guard played before this fix.
private static RouteRecallKind MapLegacyRecall(int legacyOrdinal) => legacyOrdinal switch
{
0 => RouteRecallKind.LifestoneRecall, // old Lifestone
1 => RouteRecallKind.Marketplace, // old Marketplace
2 => RouteRecallKind.PrimaryPortalRecall, // old PrimaryPortal
3 => RouteRecallKind.SecondaryPortalRecall, // old SecondaryPortal
_ => RouteRecallKind.PrimaryPortalRecall,
};
}
}

66
tests/AcDream.Plugins.MossTank.Tests/NavigationTests.cs
View file

@ -567,6 +567,72 @@ public sealed class NavigationTests
Assert.Single(target.Waypoints);
}
/// <summary>
/// D-1 (round E architecture re-check): the pre-cutover
/// RouteRecallKind was {Lifestone=0, Marketplace=1, PrimaryPortal=2,
/// SecondaryPortal=3} — completely different kinds at the SAME
/// ordinals as today's much larger enum (PrimaryPortalRecall=0,
/// SecondaryPortalRecall=1, LifestoneRecall=2, LifestoneSending=3,
/// ...). Before this fix, deserializing a legacy JSON route's bare
/// "Recall" integer straight into the new enum type silently remapped
/// every one of the four old ordinals to the WRONG new recall (and
/// Enum.IsDefined never caught it, since 0..3 are all still valid new
/// members). Each of the four legacy ordinals must migrate to its
/// real VTank recall.
///
/// Migration writes the converted route straight to the real .af file
/// (<c>MigrateLegacyIfNeeded</c>), and <c>LoadCurrent</c> immediately
/// re-parses THAT file to build the returned <see cref="NavigationSettings"/>
/// — so asserting against the re-parsed waypoint would really be
/// testing the .af "rcl" node's own name-only round-trip (a pre-
/// existing, unrelated limitation: it carries no spell id at all, and
/// "Marketplace Recall" can never resolve through ANY catalog because
/// it is not a real spell — see <see cref="RecallWaypointForMarketplaceSubmitsTheSlashCommandNotACast"/>),
/// not this fix. The direct, catalog-independent proof that
/// <c>MapLegacyRecall</c> chose the right kind is the WRITTEN .af
/// text itself: its "rcl" line carries exactly
/// <see cref="RouteWaypoint.RecallDisplayName"/> for that kind.
/// </summary>
[Theory]
[InlineData(0, (int)RouteRecallKind.LifestoneRecall)] // old Lifestone
[InlineData(1, (int)RouteRecallKind.Marketplace)] // old Marketplace
[InlineData(2, (int)RouteRecallKind.PrimaryPortalRecall)] // old PrimaryPortal
[InlineData(3, (int)RouteRecallKind.SecondaryPortalRecall)] // old SecondaryPortal
public void LegacyJsonRouteMigratesOldRecallOrdinalToTheRightNewKind(
int legacyOrdinal,
int expectedKindOrdinal)
{
// xunit only discovers PUBLIC [Theory] methods, and RouteRecallKind
// is internal — a public parameter of that type is a compile error
// (CS0051), so InlineData passes ordinals (public ints) and the
// enum is recovered here instead (matching
// RecallNameAndSpellIdTablesAgree's own pattern above).
var expectedKind = (RouteRecallKind)expectedKindOrdinal;
var storage = new MemoryStorage();
string legacyKey = LegacyRouteByCharacterKey("Barris");
storage.Text[legacyKey] = $$"""
{
"Mode": 1,
"Waypoints": [
{ "Type": 2, "EastWest": 1.0, "NorthSouth": 2.0, "Recall": {{legacyOrdinal}} }
]
}
""";
var store = new MossTankRouteProfileStore(new FakeHost(new FakeAutomation(), storage));
Assert.True(store.BindCharacter("Barris"));
var target = new NavigationSettings();
Assert.True(store.LoadCurrent(target, MetafSerializer.NoOpSpells.Instance));
RouteWaypoint waypoint = Assert.Single(target.Waypoints);
Assert.Equal(RouteWaypointType.Recall, waypoint.Type);
string fileName = "navs/" + VtankProfileDirectory.AutoCharacterFileName(
"Barris", string.Empty, "af");
Assert.True(storage.Text.TryGetValue(fileName, out string? af));
Assert.Contains($"{{{RouteWaypoint.RecallDisplayName(expectedKind)}}}", af);
}
/// <summary>
/// Reproduces MossTankRouteProfileStore's pre-cutover named-profile JSON
/// hash key (its own <c>LegacyProfileKey</c> is private; the format is