Campaign VT slice 1 Part A: the .usd document model + 137-setting
serializer with declared type tags and exact compare, metaf .af reader/
writer for metas and nav routes with real byte identity against the
owner's fixtures, .utl gate fixes, the VtankProfiles host storage
(ACDREAM_VTANK_PROFILE_DIR), and the cutover of all four profile stores
to real VTank files with one-time JSON migration. Two Opus lenses, three
fix rounds, two narrow re-reviews, final re-check: MERGE-READY.
Contract-doc ledger conflict resolved by keeping the campaign branch.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Five small fixes bundled per the round's cleanup item:
- VtankNavRouteSerializer.cs's doc comment cited a "WriteBinaryNavBlob"
method that no longer exists anywhere in the codebase (MetaEngine's
embedded-navigation contract moved to the typed MetaAction.EmbeddedRoute
NavigationSettings, saved/loaded through MetafSerializer.SaveNav/
TryLoadNav, back at round 2 step B) — corrected to name the real
mechanism.
- MossTankCommands.cs:274's comment referenced an "exports/nav/" mirror
directory that stopped existing when route profiles cut over to writing
their real .af file directly (round 2 steps 2-3) — corrected.
- docs/research/vtank-kb/07-meta-and-expressions.md section 5.2 row 6
described the pre-cutover "MossTankMetaProfileStore.WriteLegacyExport
convenience mirror" design; .af is now the SOLE authoritative Meta
store, so a disabled rule's save refusal now blocks the profile itself
— the row now says a disabled rule makes the profile file genuinely
unsaveable, not that a mirror goes stale.
- The two bare `catch (FormatException) { }` blocks that silently dropped
a corrupt monster-rule expression (one in SideCarDocument.Apply, reached
from a corrupt side-car; one in LegacyCombatProfileDocument.Apply,
reached during legacy-JSON migration) now log a warning via the host's
IPluginLogger, threaded through as an optional parameter from every call
site.
- VtankDatabase.Render()'s table-sort doc comment now states explicitly
that StringComparer.Ordinal matching .NET Framework's SortedDictionary
default order is confirmed only for the plain-ASCII table names VTank
ships (AntiExtraBuffSpells, MyMonsters, Settings, …), not as a general
claim for any string — comment only, no behavior change.
Added CorruptSideCarMonsterRuleIsLoggedNotSilentlySwallowed (FakeLogger
now captures Warn() calls via a new FakeHost.Logger property) pinning the
swallow-to-log fix.
Mutation: reverted MossTankProfileStore.cs to HEAD (keeping only the new
test) and ran it — failed with an empty Warnings collection, confirming
the silent-swallow bug before the fix.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Round 2 step 5 gave the settings Profiles tab a Delete action; Meta, Route,
and Loot never got the same verb. Added MossTankMetaProfileStore.Delete,
MossTankRouteProfileStore.Delete, and MossTankLootProfileStore.Delete
(same contract as Settings: remove the selected named profile's real file,
fall back to By char; refuse for By char itself, which has nothing to
delete — see each store's ClearCurrent for that case), wired through
MossTankPanel.DeleteMetaProfile/DeleteRouteProfile/DeleteLootProfile to
three new "Delete" buttons in mosstank.xml (Route tab row, the Meta tab's
button row, and the Loot rule editor's button row).
MossTankMarkupContractTests' interactive-control count moves 191 -> 194
for the three new buttons.
Mutation: reverted all four .cs files and mosstank.xml to HEAD (keeping
only the new/changed tests) — the test project failed to even COMPILE
(DeleteRouteProfile/DeleteMetaProfile/DeleteLootProfile do not exist on
MossTankPanel), confirming the six new behavioral tests
(DeleteRouteProfile/DeleteMetaProfile/DeleteLootProfile, each with a
successful-delete and a refuse-by-char case) and the markup-count update
all depend on this commit's code.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
MossTankLootProfileStore now reads/writes real .utl files through
VtankLootProfileSerializer.TryRead/Write in the VtankProfiles storage,
matching the Settings/.usd, Route/.af, and Meta/.af cutovers already
landed: directory-backed listing (new VtankProfileDirectory.ListLootProfiles,
*.utl, "--" hidden rule), .cdf participation (LootFileName, already present
in VtankCharacterBinding but never populated by this store), and a one-time
JSON migration (SweepLegacyRosterIfNeeded) that converts BOTH this
character's own "By char" document and every other named profile the
pre-cutover roster still lists — unlike Meta/Route (whose rosters were
already abandoned pre-round-2), loot's roster was still the LIVE mechanism
right up to this commit, so there is no separate "selected vs the rest"
split the way Settings/Meta/Route each have. WriteLegacyExport is deleted;
exports/ has no remaining loot writer.
Found and fixed a real representational-loss bug the cutover would
otherwise have introduced: VtankLootProfileSerializer.ExportRequirements
replaces an empty VtankRequirements list with a "safely disabled"
VTClassic placeholder requirement — correct when .utl was only ever a
courtesy export mirror alongside the authoritative JSON store, but
silently destructive once .utl becomes the SOLE store, since every
MossTank-authored rule's Expression text would be permanently discarded on
its first save/reload cycle. Added a MossTank-owned length-delimited
"MossTankRuleExpressions" block (using the serializer's own existing
UnknownBlocks round-trip contract — a real VTClassic reader just ignores it
as an unrecognized block, the same as any other extension) that restores
each affected rule's exact Expression text on load.
Filed AD-123: MossTank's own ByCharacter auto-.utl-file convention for loot
(kept for consistency with the other three stores) versus retail's real
loot picker, which seeds only [None] and has no per-character auto file at
all (docs/research/vtank-kb/01-settings-and-profiles.md section 3).
Updated four pre-existing tests for the new storage shape/behavior:
ProfileGiveControllerTests' FakeHost now wires VtankProfiles (the loot
store no longer uses Storage at all); LootProfilesAreIndependentNamedDocuments
and LootCommandsImportAndExportExactVtclassicUtlFiles now assert against
the real file/no-exports-mirror shape instead of the deleted hashed-JSON/
exports-mirror one. Added LootRosterSweepConvertsByCharacterAndEveryNamedLegacyProfileOnce
pinning the migration + its idempotence.
Mutation: reverted MossTankLootProfileStore.cs/VtankProfileDirectory.cs to
HEAD (keeping only the new/changed tests) and ran the three
cutover-dependent tests — all three failed (no real .utl file, no
ListLootProfiles, exports/ mirror still expected) — confirming they
exercise the bug/gap before the fix. LootingUsesVtankDefaultsAndPersistsTheOrderedRuleEditor
(pre-existing, unchanged) independently caught the representational-loss
bug during development before the MossTankRuleExpressions block was added.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
bk.a (refs/vtank/decompiled/bk.cs:6-35) appends a brand-new Settings row
([0]=name, [1]=value, every other column a void gy() cell) when a .usd
file's Settings table has no row for that name at all, and reports
"Done saving setting X to all profiles. (Changed N profiles)" with N =
every .usd file scanned. MossTankProfileStore.SetOptionInAll instead
silently skipped any file missing the row, and MossTankPanel's message
("Set option X in N profile(s) = value") matched neither retail text nor
count semantics.
Fixed SetOptionInAll to append (VtankCell { Tag = "0" } for every column,
then overwrite [nameColumn]/[valueColumn]) and to return the total scanned
file count instead of a per-row "changed" tally; changed the setinall
success message to retail's exact text.
Also added real type validation to /vt opt set|setinall: TryParseOptionValue
now takes the catalog's declared VtankSettingValueType and fails with
retail's exact "Option set: Invalid value specified. Proper type of X is
Y." text (refs/vtank/decompiled/uTank2/PluginCore.cs:5501,5508,5612) when
the typed value doesn't parse as that type — previously any non-empty
string silently succeeded regardless of the option's real type. The
Advanced Options editor keeps the original lax free-form parse (no
catalog type to check against there).
Mutation: reverted MossTankProfileStore.cs/MossTankCommands.cs to HEAD
(keeping only the new tests) and ran all four new/changed tests — all
four failed (old "Set option..." message text, no append, no type
validation) — confirming they exercise the bug before the fix.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Retail's real 2000 ms jump-charge ceiling (refs/vtank/decompiled/bi.cs:502-505,
bi.a) is enforced at the moment a jump STARTS charging, not by the storage
format — metaf's own NJump (metaf_monolithic.py:11708-11820) and VTank's
.nav both round-trip the authored value unclamped. The prior slice-1 port
misread this as a LOAD-time clamp: MetafSerializer's .af "jmp" parser and
the legacy-JSON route migration both clamped JumpChargeMilliseconds on
read, silently rewriting an authored 5000 ms waypoint down to 2000 ms even
when the route is never executed.
Removed both load-time clamps; NavigationController.TickJump now clamps
the EFFECTIVE charge duration (Math.Clamp(..., 0, 2000)) only at the one
place retail actually enforces it — the charge-hold comparison during
execution — leaving the stored/authored value untouched.
Renamed MetafSerializerTests.JumpNodeClampsChargeMillisecondsTo2000 to
JumpNodeLoadPreservesAuthoredChargeMillisecondsAboveRetailCeiling (now
asserts the 5000 ms value survives the .af load) and added a save+load
round-trip test and a Navigation execution test asserting the jump
releases at ~2000 ms of in-game charging despite a 5000 ms authored value.
Mutation: reverted MetafSerializer.cs/MossTankRouteProfileStore.cs/
Navigation.cs to HEAD (keeping only the new/changed tests) and ran the
three new/renamed tests — all three failed (load clamped to 2000,
execution never released before 5000 ms) — confirming they exercise the
bug before the fix.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Item 3: MossTankRouteProfileStore's per-character auto route file was named
nav_--Name_Server.af (NavPrefix + AutoCharacterFileName) — the "nav_" marker
came BEFORE the "--" hidden prefix, so the whole filename does not start
with "--" and defeats every StartsWith("--") hidden-file check in
VtankProfileDirectory, leaking another character's private route binding
into both the nav and meta pickers. Chose hidden-prefix-first naming
(--nav_Name_Server.af, matching VTank's own "--" convention with the nav_
kind marker second) via a new AutoCharacterFileName(name, server, ext,
marker) overload; applied only to the per-character auto file — named
routes keep their existing nav_Name.af (shared/visible) shape.
Item 4: ListNavigationProfiles and ListMetaProfiles shared the same flat
.af directory with no marker check at all, so each picker returned the
other's files too (a Meta profile appeared in the nav picker and vice
versa). ListNavigationProfiles now requires the nav_ marker;
ListMetaProfiles now excludes it.
Mutation: reverted VtankProfileDirectory.cs and MossTankRouteProfileStore.cs
to HEAD (keeping only the new/changed tests) — the build failed outright
(VtankProfileDirectory has no NavMarker/marker-overload for the new tests to
call), and the two pre-existing tests this round updated
(ListNavigationProfilesFiltersBothReservedPrefixes,
RouteStoreLeavesLegacyJsonUntouchedWhenAfCounterpartExists) independently
failed at runtime against their OLD un-marked/mis-ordered fixtures once
this round's marker/ordering requirement was pinned, confirming both are
exercising real, fixed behavior.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The pre-cutover roster documents (profiles/{index,meta/index,route/index}.json)
carried EVERY named profile a character had, but each store's cutover only
ever converted the ONE currently-selected profile (MigrateLegacyIfNeeded) —
every other named profile was silently orphaned: never converted to the new
.usd/.af format, never listed again. Worse, MossTankProfileStore actively
reused "profiles/index.json" for a MineOnly-only shape, so the very next
SetMineOnly() save would have permanently discarded the roster.
Added a one-time SweepLegacyRosterIfNeeded() to all three stores
(settings/meta/route), guarded by a per-instance flag so repeat LoadCurrent
calls are a no-op:
- Settings: entries owned by the currently-bound character convert to their
real per-(character,server) sub-profile .usd + side-car; entries owned by
a different character are written back to the roster for that character's
own future session. MineOnly now lives at a NEW dedicated key
(profiles/macro/preferences.json) so the old roster key is never again
flattened/overwritten.
- Meta/route: named profiles were never owner-scoped (one shared, globally-
hashed key per name), so every roster entry converts unconditionally.
Mutation: reverted all three store .cs files to HEAD (keeping only the new
tests) and ran SettingsRosterSweepConvertsEveryNamedLegacyProfileOnce,
MetaRosterSweepConvertsEveryNamedLegacyProfileOnce, and
RouteRosterSweepConvertsEveryNamedLegacyProfileOnce — all three failed
(missing converted .usd/.af files) — confirming the tests exercise the bug
before the fix.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
MossTankProfileStore's SideCarDocument.InventoryEnableLooting duplicated
the real "EnableLooting" .usd Settings row, and the side-car's own Apply()
ran AFTER the .usd was applied in LoadCurrent — so a drop-in profile with
looting on loaded with it silently off, clobbered by whatever the side-car
happened to hold (default false for a profile with no side-car at all).
Deleted the duplicate field; settings.Inventory.Loot.Enabled is now owned
exclusively by the .usd row. Added a reflection test proving no SideCarDocument
field (typed or with its Combat/Buff/Vitals/Inventory group prefix stripped)
collides with a VtankOptionCatalog name, and a drop-in regression test:
a real .usd fixture with EnableLooting=True and NO side-car now loads with
panel.LootEnabled == true.
Mutation: reverted MossTankProfileStore.cs to HEAD (keeping only the new
tests) and ran both new tests — SideCarDocumentHasNoFieldNamedForARealVtankSetting
failed with `["InventoryEnableLooting"]`, DropInUsdWithLootingEnabledAndNoSideCarLoadsLootingEnabled
failed with Expected True/Actual False — confirming both tests exercise the bug
before the fix.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Campaign VT slice 1 Part A round 2 step 5: the Profiles tab's action set
(Create/Select/Save/Delete/name field/mine-only) was missing Delete
entirely — every profile family only ever had Create/Copy/Clear. Adds
MossTankProfileStore.Delete (removes the selected named profile's real
.usd file and its side-car, falls back to "By char"; refuses for "By
char" itself, which has nothing to delete — see ClearCurrent for that
case) and wires it through MossTankPanel.DeleteProfile to a new "Delete"
button in mosstank.xml, next to "Clear profile!".
Create/Select/the name field/the mine-only toggle already bind to the
directory-backed store from steps 1-4; this closes the one missing verb.
Mutation shown to fail: Delete short-circuited to always refuse made
DeleteProfileRemovesTheRealFileAndFallsBackToByCharacter fail (selection
stayed on the named file instead of falling back); restored, it passes
along with the By-char refusal companion test. The markup contract's
interactive-control count was updated for the new button (190 -> 191).
595 MossTank tests passing (was 593).
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
MossTankMetaProfileStore and MossTankRouteProfileStore now store their
profiles as real .af files through IPluginHost.VtankProfiles, named and
listed by VtankProfileDirectory's rules, instead of a hashed JSON
document plus a stale "exports/" mirror (WriteLegacyExport deleted from
both). Named Meta profiles are plain shared files (matching VTank's own
ac() picker, which has no per-character sub-profile carve-out); named
route profiles carry a "nav_" prefix (metaf's own observed convention
for a stand-alone nav .af, confirmed against the committed nav_*.af
fixtures) so a route and a Meta profile sharing a user-typed name never
collide in the shared VtankProfiles directory. Both stores' Selected/
AvailableNames strip the file extension (and the route store's "nav_"
prefix) for display, matching the bare names users type at /vt meta|nav.
The route store now persists only the fields metaf's NAV: grammar
actually carries (Mode, Waypoints, FollowTarget) — Enabled/Priority/
MinimumDistanceMeters/FollowAroundCorners/OpenDoors/Door* are real VTank
Settings-table rows already owned end-to-end by MossTankProfileStore's
.usd profile (round 2 step 1), matching real VTank's own split between
global nav prefs and the per-route file; ClearCurrent and LoadCurrent
were narrowed to match. LoadCurrent gained an ISpellCatalog parameter
(TryLoadNav's own requirement); both call sites now pass
host.Automation.Spells.
Both stores gained the same one-time legacy-JSON migration as the
Settings store: first load converts a not-yet-migrated JSON profile to
.af and deletes the JSON key, leaving an existing .af counterpart (and
its stale JSON) untouched. The Meta store's SaveCurrent/Create now
refuse (return false, set SaveNotice, leave the prior .af content in
place) rather than silently drop a disabled rule that metaf/.af cannot
represent — MossTankPanel's four rule-editing call sites were updated to
prefer that refusal notice over their own generic success message.
Mutations shown to fail: MossTankMetaProfileStore.MigrateLegacyIfNeeded
stubbed to a no-op made MetaStoreMigratesLegacyJsonProfileToAfAndDeletesTheJsonKey
fail (legacy key was not deleted); SaveCurrent's SaveMeta call switched to
dropDisabledRules:true made MetaStoreRefusesToSaveADisabledRuleAndKeepsThePriorAfContent
fail (the disabled rule was silently written). Both restored and green.
NavigationTests.RouteProfilesRoundTripEveryWaypointField (asserting the
pre-cutover JSON-carries-everything behavior) was split into
RouteProfilesRoundTripWaypointFieldsButLeaveSettingsOwnedFieldsAlone and
FollowModeRouteRoundTripsTheFollowTargetThroughAf, and now also
documents two pre-existing, already-recorded metaf representational
gaps discovered by routing this path through .af for the first time:
JumpDirection has no .af representation at all (MetafSerializer.cs:924)
and a "jmp" node carries no cell id (six bare fields, no hex component).
MossTankPanelTests' nav/meta export-path tests were updated from the
retired "exports/meta|nav/" mirror to the real file locations.
591 MossTank tests passing (was 588 after step 1's commit, +3 new tests
net of the two renamed/retired ones).
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
MossTankProfileStore now stores the VTank-catalog Settings (Combat/Buffs/
Vitals/Inventory/Navigation) as a real .usd database read/written through
VtankSettingsProfileSerializer, listed via VtankProfileDirectory's real
naming rules (auto "By char" file, "--Name_Server_" sub-profiles, .cdf
per-character binding) instead of a hashed JSON document. Only state with
no VTank setting name (item/consumable lists, the monster-rule table, and
a handful of MossTank-only knobs) still lives in a small JSON side-car
keyed by the real .usd file name.
A one-time migration converts a not-yet-migrated legacy JSON profile into
its .usd + side-car pair on first load and deletes the JSON key, leaving
an existing .usd counterpart (and its stale JSON) untouched. `opt setinall`
now patches every known .usd file's Settings row directly, keeping each
file's side-car DynamicSettings mirror in sync so MossTankPanel's existing
option-override replay doesn't clobber the freshly patched value on the
next load.
VtankProfileDirectory gained WriteCharacterBinding (the .cdf writer
counterpart to the existing reader). VtankSettingsProfileSerializer.Apply
is now internal so the store can seed live settings directly from a
parsed database without a text round-trip.
Mutation shown to fail: MigrateLegacyIfNeeded stubbed to a no-op made
FirstLoadMigratesLegacyJsonMacroProfileToUsdAndDeletesTheJsonKey fail
(legacy key was not deleted); restored, it passes along with the
ExistingUsdCounterpartLeavesLegacyJsonUntouchedAndUnread companion test.
585 -> 587 MossTank tests passing (net +2 after adjusting three existing
tests to the new, more retail-faithful defaults/identity: a brand-new
profile now seeds from VTank's own shipped defaultsettings.usd rather
than a MossTank-guessed CLR default, and Selected/MacroProfileNames now
surface the real VTank file name instead of a bare invented name).
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Item J (slice-1 fix round), five sub-parts (item 12's sixth, the
VtankCellBuilder/VtankCellFactory note, required no change — both are
genuinely in use):
1. MobsInDist_Priority's regex-table entry gained an argument-order
comment (count, distance, priority — cross-referenced against Meta.cs's
runtime evaluation, CountMonstersByPriority(priority, distance) >=
count) plus a new synthesized round-trip test
(MobsInDistPriorityRoundTripsAllThreeNumbersDistinctly): this condition
is never exercised by any committed real fixture (only its name
appears, in the auto-completion header banner text), so nothing
previously caught an accidental swap of any two of its three numeric
fields.
2. VtankDatabase.Render() now explicitly sorts tables by name
(StringComparer.Ordinal) before writing — VTank's own `y` class holds
tables in a SortedDictionary, so a real .usd/.ast always emits
table-name order. This port's own Tables is an insertion-ordered list,
so every committed fixture happened to round-trip in order today
purely because it was already sorted the last time real VTank wrote it
(confirmed: defaultsettings.usd's own first five tables are already
alphabetical). New RenderEmitsTablesInNameOrderRegardlessOfInsertionOrder
adds three tables in deliberately reversed order to prove the sort,
not just re-check an already-sorted fixture.
3. SaveMeta's NAV: block write order now comes from a List<(MetaAction,
string)> populated in AssignEmbedTags's own traversal order, not
Dictionary<MetaAction,string> enumeration — Dictionary enumeration
order happens to match insertion order in the current runtime absent
removals, but that is an implementation detail, never a documented BCL
contract. embedTags stays a Dictionary purely for WriteAction's O(1)
lookup; embedOrder is the sole source of write order.
4. "/vt nav save Foo.af" and "/vt meta save Foo.af" used to keep the
".af" suffix (only ".nav"/".met" were stripped from the argument),
producing a doubled "exports/nav/Foo.af.af" / "exports/meta/Foo.af.af"
export instead of "exports/nav/Foo.af" / "exports/meta/Foo.af" — .af
is the only VTank-compatible storage format now, so both commands
strip it too. NavCommandsImportAndExportExactVtankNavFiles/
MetaCommandsImportAndExportExactVtankMetFiles renamed to
NavCommandsImportLegacyAndExportAf/MetaCommandsImportLegacyAndExportAf
(the writer is real metaf output now, not a byte-exact pass-through of
the imported .nav/.met, so the old names overstated what they prove);
new NavSaveAcceptsAnAfSuffixedNameWithoutDoublingIt/
MetaSaveAcceptsAnAfSuffixedNameWithoutDoublingIt pin the fix.
5. VtankLootRequirementEvaluator's IntKeyExists/DoubleKeyExists were each
an independently hand-maintained duplicate of IntValue/DoubleValue's
own named-field key list — a key added to one switch and forgotten in
the other would silently make BuffedInt/BuffedDouble's KeyExists gate
treat a real, always-present field as "raw property bag only". New
TryIntValue/TryDoubleValue are the single source of truth for both
"what is this key's value" and "does it exist at all"; IntValue/
DoubleValue and IntKeyExists/DoubleKeyExists are now both thin wrappers
over them. The two double-side "virtual field" keys (VtankDoubleBase+12/+14,
which always exist regardless of whether their remapped raw key is
present) needed an explicit comment to preserve that exact semantic
through the consolidation.
Full MossTank suite: 583 -> 585 (2 new tests from item 4; items 1-3 and 5
added/renamed tests without net new count beyond that). App.Tests
(Plugin|LaunchOptions filter): 84/84.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Item I (slice-1 fix round, finishing item 1). CaptureMatchesDeclaredSettingTypeAndValue's
~1.5e-7 float-round-trip tolerance was masking a real cause, not a real
value change: every tDouble-declared distance setting whose live field
was actually a float (CombatSettings.MaximumRange/MinimumRange/
ApproachDistance/RingDistance/ArcRange/TargetSelectAngleRange/
PetCustomRange/CollisionProjectileRadius/CollisionStepDistance/
SpellRangeFudge, Looting.CorpseApproachRange/CorpseMinimumApproachRange,
VitalPlan.HelperHealthDistance/HelperStaminaDistance/HelperManaDistance)
lost precision below float's ~7-significant-digit guarantee on every
load, since a loaded .usd's real value is a double.
- All 15 fields widened from float to double, matching their declared
tDouble type. Every physics/combat call site that genuinely needs a
float (IProjectileAutomation.EvaluatePath/EvaluatePathWithDiagnostics,
ICombatAutomation.CaptureHostileTargets/CaptureCorpses, the
fellow-distance Lowest() helper) now casts explicitly at that one use
site (CombatController.cs, Looting.cs, VitalRecharge.cs,
MossTankCommands.cs, PetAutomation.cs) instead of the field itself
being narrowed everywhere it's stored.
- MossTankProfileStore's JSON DTOs (InventoryProfileDocument.
CorpseApproachRange, CombatProfileDocument.MaximumRange/
ApproachDistance/TargetSelectAngleRange/ArcRange/RingDistance/
PetCustomRange) widened to match, so MossTank's own persisted profiles
keep full precision too — their Apply()-side Math.Clamp calls needed no
changes (the float literal bounds like 2f/100f already widen to the
double overload implicitly).
- VtankSettingsProfileSerializer.Apply's `(float)(cell.AsDouble() * 240d)`
casts and `cell.AsFloat()` calls for these 15 settings are now plain
`cell.AsDouble()` / `cell.AsDouble() * 240d` — no narrowing at all.
- ValuesEqual's "d"/"f" branch dropped DoublesEqual/FloatRoundTripTolerance
entirely: `a.AsDouble() == b.AsDouble()`, exact, matching every other
tag. CaptureMatchesDeclaredSettingTypeAndValue's own test-side tolerance
(a second, independently-tolerant comparison) removed the same way —
all 135 catalog names now pass under Assert.Equal(exact) with zero
special-casing.
Full MossTank suite: 581/581 (no count change — this is a precision fix,
not new coverage; CaptureMatchesDeclaredSettingTypeAndValue's own 135
cases already existed and now pass exactly instead of within tolerance).
App.Tests (Plugin|LaunchOptions filter): 84/84.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Item H (slice-1 fix round), six sub-parts:
1. Restored VtankMetaProfileSerializerTests.LoadsKnownTypedCondActRecord
and SignedHighBitLandblockIdRoundTripsExactly (the latter adapted from
the deleted RoundTripPreservesEveryVtankConditionActionAndEmbeddedNav's
LandblockEquals/LandcellEquals coverage of unchecked((int)0x8B370000u)),
both deleted as collateral damage of an unrelated file move in commit
0d10399e0. Neither calls the deleted VtankMetaProfileSerializer.Save
writer (demoted to import-only in 3ff9461ef) — both are pure reader
assertions.
2. MetafSerializer.SaveMeta now refuses (throws InvalidOperationException
naming the count) to silently drop a disabled MetaRule: real VTank/
metaf has zero concept of "disabled" (confirmed: metaf_monolithic.py
has no "enabled"/"disabled" occurrences anywhere), so
MetaRule.Enabled is a MossTank-only extension with no metaf-compatible
marker. A new SaveMeta(profile, dropDisabledRules: true) overload lets
a caller accept the loss explicitly.
MossTankMetaProfileStore.WriteLegacyExport (the .af convenience mirror
beside MossTank's own fully-fidelity JSON storage) deliberately does
NOT opt in — it leaves that mirror stale and logs a warning via its
existing try/catch rather than losing the rule. Recorded as gap 6 in
docs/research/vtank-kb/07-meta-and-expressions.md section 5.
3. New VtankProfilesDefault (src/AcDream.App/Plugins/): the graphical
host's default VtankProfiles root (<DataDirectory>/vtank), extracted
out of Program.cs's inline Path.Combine call into its own pure,
injectable-root function specifically so the "Path.Combine only, never
a hard-coded Windows path" guarantee is a real, failable Linux-path
unit test (VtankProfilesDefaultTests.ResolveIsBuiltWithPathCombineOnly)
rather than something only checkable by reading the source — the
pattern item F's VtankProfileDirectory rewrite removed when
Resolve/PortableDefault moved out of the plugin.
4. New BuffedDoubleRequirementDoesNotApplyBonusWhenBaseKeyIsAbsent: the
KeyExistsDouble gate (ComputedItemInfo.cs:234) already existed in
BuffedDouble, but only the int side
(BuffedIntRequirementDoesNotApplyBonusWhenBaseKeyIsAbsent) had a
pinning test.
5/6. DoubleSpellBonuses gained an explicit Change field (KB doc 05
section 2.2: "additive unless the static table's Change==1, in which
case multiplicative", ComputedItemInfo.cs:244), replacing
BuffedDouble's prior `(int)bonus.Bonus == 1` magnitude-based proxy —
that proxy only worked because every multiplicative bonus in the
current 19 rows happens to fall in [1.0, 2.0) and every additive one
happens to be under 1.0; it would have silently mis-branched on a
future row like an additive 1.5 or a multiplicative 2.0+. Every
existing row's Change value was derived mechanically from its old
proxy result (no behavior change for the current table), and
BuffedDoubleRequirementAppliesAdditiveBonusWhenBaseKeyExists/
AppliesMultiplicativeBonusWhenChangeIsSet pin both branches through
the real named field.
Full MossTank suite: 574 -> 581. App.Tests
(Plugin|LaunchOptions|RuntimeOptions filter): 135 -> 137. Core.Tests
(Plugin filter): 50/50 (no change, no Core-side edits this item).
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Item G (slice-1 fix round).
- ListSettingsProfiles's "Mine only" filter previously hid EVERY shared
(non-sub-profile) file when checked. Real VTank's predicate
(uTank2/PluginCore.cs:7020-7024, cSettingsShowAll/field a9) is
"!checked || file == current" — a shared file is hidden only when the
box is checked AND it is not the file currently assigned to the
character, so the profile actually in use never disappears out from
under the user just because they ticked the box. New optional
currentFileName parameter carries that exemption.
- New VtankProfileDirectory.CdfFileName/TryReadCharacterBinding: the
real per-character binding file (da class, da.cs:15,105-164) —
filename Server_CharacterName.cdf (Server-then-Name order), literal
"uTank2 CDF 1.0" version-header line 1 (a mismatch is treated
identically to a missing file, per da.cs:113-121, and the method
returns null in both cases rather than fabricating a default binding),
lines 2-4 the settings/loot/nav filenames currently assigned to that
character, optional line 5 the meta filename (present only when the
stream wasn't already at EOF — an older .cdf predating meta support
has no line 5 at all). The legacy .uts->.usd settings-filename
rewrite (da.cs:130-141) is applied here so callers never see a stale
extension.
- New VtankProfileDirectory.AstFileName: the per-character spell-
tracking cache name (dm class, dm.cs:391) — CharacterName_Server.ast,
no "--" prefix, not user-selectable. Note the concatenation order is
the REVERSE of CdfFileName's (Name-then-Server vs Server-then-Name) —
both are pinned by dedicated tests so a future edit can't silently
swap one for the other.
- New RealAstFixturesParseAsTheSpellsTable theory over the three
committed owner-{a,b,c}.ast fixtures: parses via the existing
VtankDatabase.Parse (same "y" grammar as .usd) and asserts the real
Spells table's four columns (SpellID/EndTime/Target/CastTime), per
live inspection of +Horan_sawato.ast recorded in
docs/research/vtank-kb/01-settings-and-profiles.md section 3.
No production caller of these new members exists yet — same as item F,
this is contract/rule-implementation work per A2's "foundation only, not
yet wired into the profile stores" scope; wiring belongs to the store
cutover explicitly deferred to round 2.
Full MossTank suite: 562 -> 574 (12 new tests, no regressions). App.Tests
(Plugin|LaunchOptions filter): 82/82.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Item F (slice-1 fix round). IPluginHost.VtankProfileDirectory handed the
plugin a raw string path and told it to fall back to its own
System.IO-based portable default when null — a plugin reading and
resolving filesystem paths itself, which is exactly the seam the rest of
IPluginHost.Storage deliberately avoids (Core.Plugins.ScopedPluginHost
scopes/validates every key; the plugin never sees a path).
- IPluginHost: VtankProfileDirectory (string?) deleted; new VtankProfiles
(IPluginStorage, defaults to NoOpPluginStorage) added — a second,
UNSCOPED storage instance (unlike Storage, which Core scopes per
plugin manifest id) rooted at a host-composed VTank-compatible
directory.
- ScopedPluginHost.VtankProfiles forwards _inner.VtankProfiles directly
(no scoping — it names one shared external location, not per-plugin
data). New PluginSessionTests.ScopedHostForwardsVtankProfilesUnscoped
proves the forwarded instance is the exact same object (Assert.Same),
not a wrapper.
- AppPluginHost/Program.cs: new vtankProfiles constructor parameter,
composed as FilePluginStorage(runtimeOptions.VtankProfileDirectoryOverride
?? Path.Combine(applicationPaths.DataDirectory, "vtank")).
- RuntimeOptions.VtankProfileDirectoryOverride: new init-only property
parsed from ACDREAM_VTANK_PROFILE_DIR (row added to
docs/launch-options.md, side-effects column states the redirect is the
only effect and documents the NullIfEmpty whitespace-not-special-cased
quirk it shares with every other path-override flag). New
RuntimeOptionsTests.VtankProfileDirectoryOverrideIsNullUnlessSet.
- FilePluginStorage.List(prefix): empty prefix now means "the storage
root itself" instead of throwing (Resolve() rejects empty/whitespace
keys, which is correct for every OTHER caller but wrong for "list
everything" — VtankProfileDirectory needs exactly that).
- Headless: HeadlessPluginHost gained the same VtankProfiles
property/constructor param, threaded through HeadlessPluginSession.Create
-> HeadlessSessionHost -> HeadlessProcessHost, composed from the new
HeadlessPathSet.VtankProfilesDirectory (<DataDirectory>/vtank, no
ACDREAM_VTANK_PROFILE_DIR-equivalent override — Headless path overrides
are HeadlessPathOverrides/CLI flags, not env vars). A small
AcDream.Headless.Plugins.FilePluginStorage duplicates the App
implementation byte-for-byte (Headless does not reference AcDream.App
and no shared "platform plugins" library exists yet to host one copy;
documented as a reasonable future consolidation, not required here).
- VtankProfileDirectory.cs rewritten: Resolve/PortableDefault deleted
outright (no more System.IO, no plugin-owned portable-default fallback);
ListSettingsProfiles/ListNavigationProfiles/ListMetaProfiles now take
IPluginStorage and enumerate through EnumerateFileNames, which calls
storage.List(string.Empty) and skips any key containing '/' (VTank's
profile directory is flat; a nested key from some other IPluginStorage
implementation is not a profile file). VtankProfileDirectoryTests
rewritten against an in-memory IPluginStorage fake instead of real
temp directories; new NestedPathKeysAreNotTreatedAsProfileFiles pins
that skip. The prior Resolve/PortableDefault-specific tests (Linux-path
guarantee, host-override-vs-portable-default) are superseded by
RuntimeOptionsTests.VtankProfileDirectoryOverrideIsNullUnlessSet plus
the RuntimeOptions.FromEnvironment Path.Combine-only composition in
Program.cs.
- docs/architecture/acdream-architecture.md: one sentence in the
Storage/List(prefix) paragraph naming VtankProfiles as the second,
unscoped storage.
No production caller of VtankProfileDirectory's listing methods exists
yet (A2's foundation is not wired into MossTankProfileStore/
MossTankMetaProfileStore/MossTankRouteProfileStore's own selection —
per that slice's own ledger note), so this is a contract + plumbing
change with no MossTank runtime behavior change.
MossTank suite: 562/562. Core.Tests (Plugin filter): 50/50. App.Tests
(Plugin|LaunchOptions|RuntimeOptions filter): 135/135. Headless.Tests:
173/174 (the one failure, HeadlessCredentialResolverTests.
LinuxRejectsGroupOrOtherCredentialPermissions, is a pre-existing
Linux-only lane gate that throws PlatformNotSupportedException on this
Windows host — unrelated to this change).
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Item E (slice-1 fix round). SaveMeta/SaveNav emitted no header at all
and no per-STATE/per-NAV editor-fold comment pair, so proof (4)
(byte-identity against metaf's own canonical emission) could only ever
pass after stripping every "~~" line — which hid that metaf's own
ExportToMetAF DOES mechanically emit both:
- OutputText.metaHeader/navHeader (metaf_monolithic.py:365-434): a fixed
auto-completion-assistance banner, prepended to every meta/nav-only
file respectively.
- State.ExportToMetAF / Nav.ExportToMetAF (py:12050-12054,12463-12467):
every STATE:/NAV: block wrapped in "~~ {" ... "~~ }", unconditionally
(including the single-node Target/follow NAV case).
- Meta.ExportToMetAF (py:12775-12794): when any NAV exists, a blank
line, the exact separator
"~~========================= ONLY NAVS APPEAR BELOW THIS LINE =========================~~"
(no space beside either "~~"), and another blank line, before the
first NAV: block.
Changes:
- MetafSerializer: MetaHeaderLines/NavOutputHeaderLines — the header
text copied byte-for-byte from Fixtures/vtank/af/bella.af (meta) and
nav_ab.af (nav-only) rather than retyped from the Python source, per
the slice-1 contract. Joined with bare "\n" (matching metaf's own
multi-line string constant, written as ONE f.line entry) plus a
trailing "\n" — combined with this writer's own "\r\n" join separator,
reproduces the single blank line real output has between the header
and the first STATE:/NAV: line exactly.
- SaveMeta: prepends MetaHeader; wraps every STATE: block in "~~ {"/
"~~ }"; emits the "ONLY NAVS APPEAR BELOW..." separator only when at
least one embedded Nav tag exists.
- WriteNavBlock: wraps every NAV: block (both waypoint-list and
Target/follow modes) in "~~ {"/"~~ }".
- SaveNav: prepends NavOutputHeader; writes the sole nav under tag
"nav0" (matching metaf's Meta.GenerateUniqueNavTag counter starting at
0 — every committed nav_*.af fixture's tag is literally "nav0";
the prior tag "route" was a MossTank invention).
Proof (4) is now REAL byte identity (Assert.Equal(original, rewritten),
no comment stripping, no blank-line normalization) plus a new nav-only
case (WriterOutputMatchesMetafCanonicalEmissionNavOnly, against
nav_ab.af). Investigating the raw bytes of every candidate fixture
found real, PRE-EXISTING header divergence unrelated to this writer:
aphus.af/neftet.af/follower.af open "~~ {\r\n~~ " (CRLF) where a fresh
metaf conversion's header is bare-LF internally ("~~ {\n~~ ",
confirmed against augments.af/bella.af/gauntlet_leader.af/
empyrean_facility.af/example_sort_meta.af) — evidence of a re-save by
something other than metaf itself (e.g. a text editor normalizing every
line ending). hunting.af and lockandkey.af carry a wholly custom
hand-written banner instead of metaf's own. ByteIdenticalFixtureData is
narrowed to the five fixtures whose header IS metaf's fresh canonical
form (bella, gauntlet_leader, empyrean_facility, augments,
example_sort_meta — still five, per the slice-1 contract's floor);
aphus/neftet/hunting/follower/lockandkey keep exercising every other
proof (parse, parse-write-parse, and the ptl/tlk direct assertions from
the item-A fix) normally, with the header divergence documented at the
exclusion site rather than asserted away.
Three MossTankPanelTests assertions changed from StartsWith to Contains
("STATE: "/"NAV: ") since exported .af content is no longer the first
thing in the file.
Full MossTank suite: 570 -> 566 (5 fixtures dropped from the
byte-identity theory, replaced by 1 new nav-only fact: -4 net).
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Item D (slice-1 fix round). CreateNew built ONLY a bespoke 4-column
Settings table (empty Description, hardcoded SettingType=1/Bool for
every row regardless of the setting's real declared type) and had no
representation of the other nine tables VTank ships at all (MyMonsters,
GemFoodItems, ExtraBuffSpells, AntiExtraBuffSpells, ItemUseSpecifiers,
SettingsCategories, SettingsEnumInfo, AssistItems, BuffedItems).
Separately, VitalSettings.RechargeHandlerRows defaulted to an empty
list, so a freshly-constructed VitalSettings (not loaded from a real
.usd) always fell back to VitalRechargePlanner's own hand-ported
handler-order switch instead of VTank's real 26-row table.
- VtankDefaultSettings.usd: the real uTank2.Resources.defaultsettings.usd
(already committed as a test fixture) copied into the plugin project as
an EmbeddedResource, following the existing VtankCraftRecipes.tsv/
VtankAmmunitionOptions.tsv pattern.
- New VtankDefaultSettingsDatabase: loads the embedded text once (Lazy),
exposes Parse() (a fresh, independently mutable VtankDatabase per call)
and DefaultRechargeHandlerRows (the real RechargeHandlerSet table,
parsed once via VtankSettingsProfileSerializer.ParseRechargeHandlerSet).
- VtankSettingsProfileSerializer.CreateNew now parses the embedded
document and calls the existing Save(document, source) against it —
every table, every row's real Description/SettingType survive
untouched; only the Settings table's Value cells get overwritten from
the live source, exactly as re-saving an existing profile would.
DefaultCell (the hand-typed per-catalog-kind cell builder CreateNew
used before) is deleted as dead code.
- VitalSettings.RechargeHandlerRows now defaults to
VtankDefaultSettingsDatabase.DefaultRechargeHandlerRows instead of [].
VitalRechargePlanner.Handlers' hardcoded switch-based defaults are kept
(not deleted) as a documented, genuinely defensive fallback for the
case where a caller explicitly clears the row list or an
(vital, stance, percent) combination the real table happens not to
cover — no longer the primary source of truth it used to be, and the
doc comment says so.
- RenderRechargeHandlerSet (a write-back helper for the RechargeHandlerSet
table, never called anywhere) is deleted rather than wired into
Capture(): docs/research/vtank-kb/01-settings-and-profiles.md section 2
row 137 confirms real VTank has NO save path for this table at all
(tCustom has no case in the Advanced Options editor or "/vt opt set";
it's owned by a dedicated cRechargeManager object). Wiring a write-back
would be a MossTank invention, not a retail port, so Capture() keeps
deliberately leaving this row untouched.
New CreateNewHasTheSameTableSetAsTheDefaultFixture test: asserts
CreateNew's table set matches defaultsettings.usd's own table set
exactly, and that Settings rows carry real (non-empty) Description and a
real (non-Bool-for-everything) SettingType. Verified failing against the
prior 4-column-only implementation (temporarily restored, ran, reverted):
"Expected: [AntiExtraBuffSpells, AssistItems, ...] / Actual: [Settings]".
Full MossTank suite: 570/570 passing (569 -> 570, one new test).
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Item C (slice-1 fix round). MossTankMetaProfileStore and
MossTankRouteProfileStore both wrote their .af exports into the same
flat "exports/" directory keyed only by profile name — a Meta profile
and a route (Navigation) profile sharing a name (e.g. both named
"Same") would silently clobber each other's .af file on save, with no
error and no warning.
- MossTankMetaProfileStore.WriteLegacyExport now writes to
"exports/meta/{name}.af".
- MossTankRouteProfileStore.WriteLegacyExport now writes to
"exports/nav/{name}.af".
- Both class docs updated to name the collision this avoids and point at
the sibling store's subdirectory.
- MossTankPanelTests: existing NavCommandsImportAndExportExactVtankNavFiles
and MetaCommandsImportAndExportExactVtankMetFiles updated to the new
paths. New MetaAndRouteExportsWithTheSameNameDoNotCollide saves a Meta
and a route profile both named "Same" and asserts both .af files exist
with their own correct content — verified failing before the fix
(asserted false on the meta file's existence once both paths were
reverted to the flat "exports/" root, confirming the collision is real
and this test catches it).
Full MossTank suite: 569/569 passing (568 -> 569, one new test).
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Item B (slice-1 fix round). A LoadEmbeddedNavigationRoute action carried
a synthesized "uTank2 NAV 1.2" TEXT blob in MetaAction.Text that both
MetaEngine and the runtime consumer (MossTankPanel) had to re-parse on
every load — an unnecessary re-parse of already-typed data, and the
reason proof (3)'s AssertActionsEqual skipped comparing embedded routes
entirely (the blob's exact byte shape wasn't a meaningful comparison
target).
- MetaAction: new EmbeddedRoute (NavigationSettings?) property replaces
the blob. Null only for a genuinely unresolved/never-defined Nav tag.
- MetaServices.LoadEmbeddedNavigationRoute: Action<string> ->
Action<NavigationSettings?>; MetaEngine's dispatch passes
action.EmbeddedRoute directly.
- MetafSerializer.ResolveEmbeddedNavs (.af importer) builds the
NavigationSettings directly via the existing ApplyNavBody helper
(renamed SynthesizeNavBlob -> BuildNavSettings) instead of serializing
it back into text. WriteBinaryNavBlob/WriteBinaryWaypoint/
FormatBinaryDouble are deleted outright (the only caller was the
now-removed blob synthesis).
- SaveMeta had its own now-dead re-parse of action.Text via
VtankNavRouteSerializer.TryLoad to rebuild each embedded NAV: block on
save; this silently started producing EMPTY NAV: blocks once Text
stopped carrying the blob (Text is now always cleared for this action
kind), caught immediately by the full suite: MetaParseWriteParseIsIdentical
started failing "Expected: Once / Actual: Circular" (a re-parsed route
falling back to NavigationSettings' default Mode because its NAV: block
vanished). Fixed by writing straight from action.EmbeddedRoute.
- VtankMetaProfileSerializer (.met importer): TryLoad gained an
ISpellCatalog overload (threaded through ReadAction/ReadEmbeddedNavigation);
ReadEmbeddedNavigation now parses its reassembled blob text through
VtankNavRouteSerializer.TryLoad into a real NavigationSettings instead
of handing the raw text to the caller. The existing 2-arg TryLoad
overload defers to MetafSerializer.NoOpSpells.Instance (promoted from
private to internal) so both test call sites and MossTankMetaProfileStore
(which now passes _host.Automation.Spells) keep working.
- VtankNavRouteSerializer.Apply promoted from private to internal so
MossTankPanel.LoadEmbeddedNavigationRoute can copy an already-typed
NavigationSettings into the live _navigationSettings instance directly,
replacing its own VtankNavRouteSerializer.TryLoad(string, ...) re-parse.
- MetafSerializerTests: AssertActionsEqual now asserts
EmbeddedRoute is non-null on both sides and calls AssertNavigationEqual
on them (waypoint-by-waypoint) for LoadEmbeddedNavigationRoute actions,
instead of skipping the comparison. PtlNodeKeepsBothCoordinateTriplesDistinct's
FindNavWaypoint helper reads action.EmbeddedRoute directly instead of
re-parsing action.Text (which is now empty).
Full MossTank suite: 568/568 passing (net zero change in count — this
is a representation change, not new coverage, though the AssertActionsEqual
tightening now exercises real waypoint comparisons on every fixture with
an EmbedNav action that it previously skipped).
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Item A (slice-1 fix round). VTank/metaf's Portal2/UseNPC nav nodes carry
TWO coordinate triples (metaf_monolithic.py:356-357,11482,11618 —
"FORMAT: ptl/tlk myx myy myz tgtx tgty tgtz tgtObjectClass tgtName"): the
outer header ("myxyz", retail's own dead-weight last-save player position
per docs/research/vtank-kb/06-navigation-and-nav.md section 1.2) and the
embedded d-record ("tgtxyz", the real target coordinate used to match a
live world object by name+class+proximity). The prior port's
RouteWaypoint had a single Position field, so both the .af reader
(MetafSerializer.ReadNavNode) and the binary .nav reader
(VtankNavRouteSerializer.ReadWaypoint, case 6/7) overwrote "myxyz" with
"tgtxyz" on load, and the .af writer echoed the same Position value for
BOTH triples on save — a real .af round trip of the same waypoint was
lossy, which is why aphus/augments/lockandkey/neftet were excluded from
the byte-identity proof.
- RouteWaypoint: new ReferencePosition field (Position stays "myxyz",
ReferencePosition is "tgtxyz"); included in Clone().
- MetafSerializer.ReadNavNode/RenderNavNode: ptl/tlk read/write both
triples distinctly. WriteBinaryNavBlob's embedded-route writer (the
MossTank runtime blob EmbedNav actions carry) fixed the same way — it
was echoing Position for the reference triple too.
- VtankNavRouteSerializer.ReadWaypoint case 6/7: keep the header triple in
Position, read the trailing triple into ReferencePosition instead of
overwriting Position.
- Navigation.TickUse: TryFindObject now searches near ReferencePosition
(the real target coordinate) instead of Position, preserving the
correct runtime search behavior now that Position no longer aliases it.
- MossTankPanel.AddSelectedObjectWaypoint: new Portal2/UseNPC waypoints
now set Position from the live snapshot (matching retail's own
"wherever the character stood") and ReferencePosition from the selected
object's live position (the real search anchor) — previously both were
set from the object's position.
- MossTankRouteProfileStore's WaypointDocument DTO carries the reference
triple too, so MossTank's own JSON-persisted routes round-trip it.
- MetafSerializerTests: un-excluded aphus/augments/lockandkey/neftet.af
from the byte-identity proof (they all embed a ptl/tlk node and now
round-trip correctly) and added example_sort_meta.af, which also
passes. bore_quest.af was NOT added despite the slice-1 contract's
ask: it is hand-edited the same way as the already-excluded
bore_enhanced.af (space instead of tab between "IF:"/"DO:" and the
following keyword, confirmed at bore_quest.af line 9 — metaf's own
Rule.ExportToMetAF always joins with a tab, metaf_monolithic.py:12371),
so it can never byte-match; documented alongside bore_enhanced's
existing exclusion note instead. New PtlNodeKeepsBothCoordinateTriplesDistinct
test pins the two-triple split directly (failed before this change:
Position held the second triple with nowhere to read the first triple
back from). VtankNavRouteSerializerTests updated to assert the split
instead of the old collapsed value.
- jmp direction: metaf's NJump class has no strafe-direction field at all
(metaf_monolithic.py:11708-11821, confirmed reading ImportFromMetAF/
ExportToMetAF end to end) — the .af format cannot represent
RouteWaypoint.JumpDirection, full stop. ReadNavNode no longer assigns
JumpDirection = Forward explicitly (the model's own default), and the
loss is now recorded as gap 9 in docs/research/vtank-kb/
06-navigation-and-nav.md section 6.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Fidelity blocker 6: VtankLineCursor pre-normalized every "\r\n" to "\n"
across the WHOLE document before splitting into lines, then ReadBlob
re-joined consumed lines with a single '\n'. VTank's real reader
(f6.cs:10-17) is `TextReader.Read(array, 0, num)` — it reads exactly N
raw characters straight off the stream, so an embedded CRLF inside a
"ba" blob costs 2 characters toward that length, not 1. The prior
cursor silently dropped that extra character, corrupting any blob
whose content used CRLF line breaks and desyncing the parse position
for everything that follows it in the same row/table.
Separately, VtankDatabaseReader.ReadCell's default case unconditionally
consumed a "value" line for ANY unrecognized tag. VTank's own reader
(gy.cs:50-55) and writer (gy.cs:98-101) treat every tag outside
{d,i,u,f,s,b,TABLE,ba} as void/unrecognized (y.cs:28-41 registers only
TABLE and ba as named custom types) and consume/emit ONLY the tag line
— no value line either way. The prior default case would misread the
next cell's own tag as this cell's bogus value, corrupting the rest of
the row exactly like the blob bug above.
- VtankLineCursor now indexes directly into the original (unnormalized)
text: ReadLine() scans for '\n' and strips one trailing '\r' per line
(matching StreamReader.ReadLine() semantics); ReadBlob(length) takes
exactly `length` raw characters from the current position with zero
reinterpretation.
- VtankCell.WriteTo (and Row/Table/Database) now build a single
StringBuilder instead of a `List<string> lines` that assumed one
entry == one line: a "ba" blob writes its length line then the RAW
blob content with NO added line terminator (matching f6.cs's
WriteLine(length) + Write(content), not WriteLine(content)) — the
next structural token continues immediately after the blob's last
character, exactly like real VTank output.
- ReadCell's default case now builds a bare unknown-tag cell (no
ScalarText/BlobText, consuming nothing further); WriteTo mirrors this
by emitting nothing after such a tag's own line.
- Bonus (adjacent, from the same gy.cs read): VtankCell.String now
strips embedded '\n' on write (gy.cs:84's `text.Replace("\n", "")`),
matching VTank's own string-cell writer exactly — a partial down
payment on item 12's "strip \n from string cells on write" nit.
New tests (tests/AcDream.Plugins.MossTank.Tests/VtankUsdDocumentTests.cs):
BaBlobWithEmbeddedCrlfRoundTripsExactCharacterCount,
BaBlobLengthCountsEmbeddedCrAndLfAsSeparateCharacters, and
UnrecognizedTagConsumesOnlyItsOwnLineNotTheNextCellsValue. All three
were verified failing against the pre-fix VtankUsdDocument.cs (restored
from HEAD, tests re-run, then reverted back) before this change:
the first two threw FormatException "unexpected end of file", the
third read "s" (the next cell's own tag) as ScalarText instead of null.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Fidelity blocker 2: VTank's UseArcs is 1=No / 2=AtRange / 3=Yes
(refs/vtank/decompiled/hi.cs:515-538, switch on f3.f("UseArcs")): No
always picks the direct-shape spell, AtRange picks arc only once the
target reaches ArcRange, and Yes always picks arc regardless of
distance. The prior port collapsed this onto CombatSettings.UseArcs
(bool), which can represent No and (an approximation of) AtRange but
has no way to express Yes — a profile with UseArcs=3 could never
actually always-arc, and Capture() had to fudge 1<->3 on save,
which is why UntouchedRoundTripIsByteIdentical needed a NormalizeUseArcs
special case.
- CombatSettings: new UseArcsMode enum (No=1, AtRange=2, Yes=3);
UseArcs is now UseArcsMode (default AtRange, matching the previous
bool default's runtime behavior).
- AttackSpellCatalog.ShouldUseArc implements the real 3-way switch,
replacing the `settings.UseArcs && target.Distance >= settings.ArcRange`
expression at both call sites (Yes now genuinely always arcs).
- VtankSettingsProfileSerializer Apply/Capture "usearcs" cases now
cast directly to/from UseArcsMode instead of the `!= 0` / `? 3 : 1`
bool collapse.
- MossTankPanel GetMetaOption/SetMetaOption "usearcs" now exposes the
raw 1-3 value (ExpressionValue.Number / AsInt32 clamped 1-3),
matching the existing pattern for DebuffEachFirst/DebuffSelectionMethod,
instead of ExpressionValue.Boolean/IsTruthy.
- MossTankProfileStore's JSON DTO field type follows suit (System.Text.Json
already serializes CombatSettings' other enum settings the same way).
- Deleted NormalizeUseArcs from UntouchedRoundTripIsByteIdentical —
the test now asserts full byte-identity with no special case, and
CaptureMatchesDeclaredSettingTypeAndValue (added in the previous
commit) no longer needs to skip "UseArcs".
Verification: reverting to the bool model reproduces exactly one
theory failure (CaptureMatchesDeclaredSettingTypeAndValue("UseArcs"):
expected 1, actual 3) confirming this is the only affected setting;
after this change the full 559-test suite passes with zero special
cases.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Fidelity blocker 1: 17 tInt Settings rows (Recharge-*,
IdleBuffTopoffTimeSeconds, RebuffTimeRemainingSeconds,
BlacklistMonsterTimeoutSeconds, GhostDeleteHPTrackerSeconds,
DebuffPrecastSeconds, BuffCastRecast(Reset)_Seconds,
BlacklistCorpseOpenTimeoutSeconds) were written as VtankCell "d" and 3
tSingle rows (CollisionProjectileRadius, CollisionStepDistance,
SpellRangeFudge) were written as "f", while VTank's shipped
defaultsettings.usd Settings.SettingType column declares them tInt(3)
and tDouble(2) respectively (refs/vtank/decompiled/uTank2/eSettingValueType.cs).
VTank's gy.e/gy.f unbox helpers (refs/vtank/decompiled/gy.cs) throw
InvalidCastException reading a mistagged cell, so a profile MossTank
wrote back would corrupt the next VTank load.
Verified against the real fixture with a small parser
(defaultsettings.usd's own SettingType column) rather than trusting a
second hand-written table: exactly 20 mismatches, matching the review's
count precisely.
- VtankOptionCatalog: added VtankSettingValueType (VTank's
eSettingValueType) and a 137-row DeclaredType lookup transcribed
verbatim from the fixture's SettingType column.
- VtankSettingsProfileSerializer.Capture: every numeric arm now routes
through Num(name, value), which wraps the value using
VtankOptionCatalog.DeclaredType(name) instead of a hardcoded
VtankCell.Int/Double/Float call.
- VtankCell.FormatDouble now formats "G15" (VTank's own
Convert.ToString(double) under classic .NET Framework), not .NET's
shortest-round-trippable default — verified against every "d" value
in the real fixture.
- ValuesEqual is now an exact per-tag compare (bool/int/uint/string
exact; double/single bounded by a named float-round-trip epsilon
documented as such, not an arbitrary tolerance) instead of lumping
every numeric tag into one loose comparison.
New test CaptureMatchesDeclaredSettingTypeAndValue is a theory over all
135 catalog names with a live write path (skipping EnableMeta,
RechargeHandlerSet, and UseArcs — pre-existing exclusions), asserting
Capture()'s tag and value against defaultsettings.usd's own Settings
row rather than a second hand-authored expectation table. It failed on
the 20 known-bad names before this fix (see conversation record) and
passes now.
Deliberate, documented deviation from the review's literal "delete the
1e-6 tolerance" instruction: several distance settings (AttackDistance,
ArcRange, …) are declared tDouble but still round-trip their live value
through a CombatSettings `float` field for the physics/combat math that
consumes it; a fully exact compare would make Save() rewrite those rows
on every untouched load due to sub-15-significant-digit float noise,
regressing the class's own byte-identity goal. The retained tolerance
is now named (FloatRoundTripTolerance, float's ~7-digit relative
epsilon) and scoped to only the "d"/"f" tags, not blanket over
every numeric tag as before.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
New internal static UiCheckLamp (LampSize=11f, the four checked/
unchecked colors, Draw) is the ONE definition of the five-band lamp
glyph, promoted out of UiMarkupToggle's private/internal fields —
UiMarkupToggle.OnDraw and UiMarkupList.DrawCheckCell both call
UiCheckLamp.Draw now instead of each carrying (or one exposing to the
other) its own copy of the same five DrawFill calls and four colors.
DrawCheckCell also centers the lamp horizontally in its cell instead of
drawing it flush left at cellX+1 — matching DrawIconCell, which already
centers its sprite. A check column declared wider than the glyph itself
(routine under fix item 11's PITCH-based authoring convention) no longer
strands the glyph in the cell's left edge.
UiMarkupListColumn's settable members are now internal init (Kind/Width
lost their `required` modifier — C# forbids `required` pairing with a
setter less visible than the containing public type, CS9032 — every
factory already sets both unconditionally, so this is a compiler-level
demotion, not a behavior change) — the type is constructible only
through its Text/Check/Icon factories from any external assembly (a
plugin) with no InternalsVisibleTo grant, so it can never assemble an
inconsistent instance via object-initializer syntax.
Replaced the inert draw-offset assertion in
Columns_CheckThenIcon_EachCellDrawsInsideItsOwnColumnBounds (check
column FIRST, so its own-cell assertion held trivially even with
completely broken column offsets) with
Columns_IconThenCheck_EachCellDrawsInsideItsOwnColumnBounds — check
column now SECOND, so the assertion can only pass if the glyph actually
moved into its own [20,100) cell.
New test: a single 50px-wide check column's glyph lands near the cell's
midpoint (~22.5) rather than the old flush-left x=4 — shown to fail
first before the centering change.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
DrawCheckCell drew nothing for a row past its own column's bound data —
inconsistent with docs/plugin-ui-markup.md's own description of VVS's
HudList, which materializes every cell in a row regardless of which
columns actually have data for it. Text/icon cells rightly stay
no-draw past their own count (no sensible default string or icon), but
a check cell always has one: unchecked.
DrawCheckCell now computes isChecked defensively (false when the flags
array is null or too short) instead of returning early, so a short
check column draws the unchecked lamp for every row past its own data
just like every other row.
New test: a 3-row list (driven by a longer text column) with a check
column bound to only 1 row now shows the unchecked lamp for rows 1 and
2 — shown to fail first (0 unchecked lamp quads, since DrawCheckCell
returned early past index 0).
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
DrawColumns allocated four fresh IReadOnlyList<T>?[] sidecar arrays and
ColumnLayout allocated a fresh (float,float)[] EVERY draw call; worse,
OnEventColumns re-invoked every column's source Func a SECOND time (via
UiMarkupListColumn.RowCount()) just to recompute the same row count
DrawColumns had already materialized, and called ColumnLayout a second
time to recompute the same layout — on every single click/scroll event.
Columns is now a property with a custom setter that (re)sizes seven
instance-field caches to Columns.Count exactly once per assignment:
_cachedTextRows/_cachedColorRows/_cachedCheckRows/_cachedIconRows (the
per-column sidecar arrays), _cachedLayout (the column x/width array),
and _scratchIsAuto/_scratchFixedWidth (ComputeColumnLayout's own working
arrays, previously freshly allocated on every layout computation too).
DrawColumns writes into these caches instead of local arrays and records
_cachedRowCount; ComputeColumnLayout (the renamed, now-instance
ColumnLayout) writes into _cachedLayout in place instead of returning a
new array. OnEventColumns reads _cachedRowCount/_cachedLayout/the
per-column cached row arrays instead of re-invoking anything — this also
completes fix item 3's row-bound guard without a second RowCount() call.
UiMarkupListColumn.RowCount() stays as public API (still asserted by an
existing test) but is no longer called from UiMarkupList internally,
which is the "single row-count definition" the fix round asked for.
This does make OnEvent depend on at least one prior Draw call for
correct row/layout data (mirrors real frame order: draw every frame,
then handle input) — the four hit-test click tests that previously
fired OnEvent with no preceding Draw now draw once first, matching what
Scroll_OffsetIsRespectedBySubsequentHitTests already did.
New test: each column source Func increments its own call counter; one
Draw call invokes each exactly once, and five rounds of subsequent
click events invoke none of them again — shown to fail first (16 calls
instead of 1, from OnEventColumns's old RowCount()-per-column-per-event
re-invocation) before the caching change.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
BuildListColumn's per-attribute binder calls used the generic "column
items"/"column values"/"column onchange"/"column onclick" context
strings from the initial slice — indistinguishable when a list has
several columns of the same type. Route every column-attribute binder
call (text items/colors/onclick, check values/onchange, icon
values/onclick) through the ColumnContext helper item 2 introduced, so
every throw message reads column[N] type="..." attr. ValidateIconKind
gains an optional context parameter (default "iconkind" for the
existing non-column call sites — <icon>, <button icon>, <list icons>,
none of which changed message-wise beyond wording) so the column
iconkind check can identify its own column too.
Updated all twelve column-throw tests in MarkupListColumnsTests to
assert Assert.Contains on the distinguishing phrase: the ten
attribute-specific ones now check column[0] type="..." attr; the
unknown-<column type> and combined-with-legacy-attribute tests keep
(and, for the type one, add to) their existing distinguishing
assertions. Confirmed each attribute-specific assertion is meaningful
by first running the malformed-colors case against the pre-fix
"column colors must be..." message (mismatch), then landing the
production change alongside the rest.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Campaign VT slice 1 Part A, deliverable 2 (foundation only - see the
closeout note in the final report for what is not yet wired up).
VtankProfileDirectory.cs resolves the on-disk VTank profile directory
through IPluginHost.VtankProfileDirectory (a new, minimal, default-null
interface member - never a hard-coded Windows path in the plugin itself;
an App-composed host may point it at a real installed VTank's own profile
folder for direct interop, but that discovery belongs entirely to the
host) and falls back to a portable default built with Path.Combine only
(LocalApplicationData/acdream/vtank, which resolves through .NET's
XDG-aware base-directory logic on Linux). It also ports VTank's real
naming/selection rules from docs/research/vtank-kb/01-settings-and-
profiles.md section 3: the per-character auto file (--Name_Server.ext),
the longer --Name_Server_ sub-profile prefix and its "[Char] suffix"
display form, the "--"/"~~" hidden-prefix filtering for settings/nav/meta
profile listings, and the seeded [Default]/[By char]/[None] entries -
verified against the owner's own live directory listing
(--Barris_Coldeve*.usd family).
Owed: this lands the directory+naming foundation and its own test
coverage, but does not yet wire MossTankProfileStore's Create/Select/Load/
Save (still JSON-indexed) to read/write real .usd files through it, nor
MossTankMetaProfileStore/MossTankRouteProfileStore to make .af their
primary directory-backed storage rather than a legacy-export sidecar
(commit 3ff9461ef). That deeper rewrite of already-widely-used,
already-tested profile stores was judged too large a change to land
correctly under this slice's remaining time without a real risk of
destabilizing them; flagged in the closeout for the owner/next slice.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Matches UiButton.cs:896-906 / UiElement.cs:683-720's own clip
discipline: every per-cell PushClip in DrawColumns now has its matching
PopClip in a finally, so a cell draw that throws (e.g. a plugin's icon
resolver misbehaving) doesn't leak that PushClip onto the context's
clip stack. Without this, the leaked entry combines badly with
UiElement.DrawSelfAndChildren's own outer clip pop on the way out
(it pops the wrong stack entry), permanently corrupting the shared
UiRenderContext's clip state for every draw that follows in the frame.
Added UiRenderContext.ClipStackDepth (internal, InternalsVisibleTo
AcDream.App.Tests) purely to make this provable from a test — the
number of PushClip calls not yet matched by PopClip.
New test: a column whose icon resolver throws mid-draw still leaves
the clip stack at its pre-draw depth after the exception propagates —
shown to fail first (leaked to depth 1 instead of 0, reproducing
exactly the "outer pop consumes the wrong stack entry" mechanism
described above) before the try/finally was added.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Campaign VT slice 1 Part A, per the owner's 2026-09-06 "MossTank does not
implement .met and does not author .nav" direction: .af (MetafSerializer)
is now the only storage/authoring format for meta profiles and navigation
routes. Deletes both classes' Save() writers and every writer-only helper
(WriteCondition/WriteAction/WriteEmbeddedNavigation/ConditionType/ActionType/
Number/IntValue/LineWriter in VtankMetaProfileSerializer; WriteWaypoint/
WriteDouble in VtankNavRouteSerializer) - TryLoad and its read-path helpers
are untouched, so a real binary .met/.nav still imports one-shot into the
in-memory model.
MetaEngine's LoadEmbeddedNavigationRoute still needs the "uTank2 NAV 1.2"
in-memory blob shape for a resolved EmbedNav action (that's a MossTank
runtime contract, not a VTank file on disk), so MetafSerializer gained its
own small private WriteBinaryNavBlob - a deliberate, scoped duplicate of
what used to be VtankNavRouteSerializer.Save's WriteWaypoint, kept
independent of the now-import-only class.
MossTankMetaProfileStore/MossTankRouteProfileStore's WriteLegacyExport
(the "/vt meta save"/"/vt nav save" sidecar) now writes .af via
MetafSerializer.SaveMeta/SaveNav instead of the deleted binary writers.
This is a real, if partial, step toward the contract's ".af is the only
storage/authoring format" goal - full profile-directory-backed .af storage
(A2's VTank-naming-scheme directory) is separate follow-up work, noted in
the closeout.
Deletes VtankMetaProfileSerializerTests.cs entirely (it only tested the
now-deleted Save/round-trip behavior); trims the two writer-only tests out
of VtankNavRouteSerializerTests.cs, keeping every reader test intact
(LoadsEveryOfficialNav12WaypointPayload's read assertions,
LoadsEmbeddedWrapperAndDoesNotMutateOnFailure). Updates
MossTankPanelTests.cs's two "/vt meta|nav save" integration tests for the
new .af export path (the meta test's synthetic import fixture is now a
hand-authored CondAct payload instead of a call to the deleted Save).
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A short column (fewer bound rows than the list's own max-across-columns
row count) already draws nothing past its own row — DrawCheckCell/
DrawIconCell/DrawTextCell all no-op once index >= their own array's
Count. OnEventColumns's click routing didn't share that bound: a click
landing on a row the OVERALL list considers valid (row < the longest
column) but past a SHORTER column's own data would still invoke that
column's callback with a row index it never bound anything for.
Add the same per-column bound (index < columns[c].RowCount()) to the
check-column, icon-column, and text-column-with-onclick click paths —
text-without-onclick's plain SelectionChanged fallback is unaffected,
since row selection is a list-level concept already bounded by the
outer rowCount check.
New tests: a click past a short check/icon/onclick-bearing-text
column's own row count fires nothing (each shown to fail first — the
callback fired with the out-of-bound row index before this guard).
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
VVS's HudList shares remaining width equally among every 0-width column
(docs/research/vtank-kb/08-ui-views.md's HudList row, Decal3XMLParser.cs
:409-418) after fixed-width columns are subtracted — the initial slice
only let the LAST column auto-size. Port the real rule:
- A non-last <column width> that is missing, unparseable, or non-positive
throws FormatException at Build naming the column index and declared
type (column[N] type="..." width) — UNLESS it is width="*".
- width="*" (any column, including the last) marks it auto: every auto
column shares the list's remaining width equally, with the LAST column
in the list (always implicitly auto, regardless of its own width or
IsAutoWidth) absorbing the integer-division rounding slack. With no
"*" anywhere this reduces to exactly the original "last column gets
100% of the remainder" contract — zero behavior change for every
existing column-less/single-"*" list.
- A declared width overflowing the list's total width is clamped to
whatever room is left, walked left to right; every column after the
overflow point gets 0 width (and never draws — DrawColumns already
skips a <=0-width cell).
UiMarkupListColumn gains IsAutoWidth (internal to the width contract,
set via each factory's new optional isAutoWidth parameter); ColumnLayout
in UiMarkupList is rewritten around an explicit auto/fixed classification
pass instead of the old "only index n-1 is special" one-liner.
MarkupDocument's new ParseColumnWidth owns the Build-time validation,
shared through the same ColumnContext helper item 1 introduced.
New tests: non-last missing/unparseable/non-positive width throws
(each naming column[N] type="..." width), width="*" parses without
throwing on both a non-last and the last column, two auto columns
share the remainder with the last absorbing the odd pixel, and an
overflowing declared width clamps with later columns drawing nothing
— each shown to fail first (compile error for the missing isAutoWidth
parameter, then a wrong split before the ColumnLayout rewrite).
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Campaign VT slice 1 Part A. Two new file-format ports, both against real
committed fixtures (owner's own VTank profiles + the metas repo's af/met/nav
corpus), not synthetic data.
A1 — VtankUsdDocument.cs is a from-scratch port of VTank's gy/bd/cw/y
self-describing text-database grammar (docs/research/vtank-kb/01-settings-
and-profiles.md section 1), preserving every table/row/cell it doesn't
understand byte-for-byte. VtankSettingsProfileSerializer.cs maps all 137
Settings rows (VtankOptionCatalog.Names) onto CombatSettings/BuffSettings/
VitalSettings/InventorySettings/NavigationSettings using the exact unit
transforms already verified in MossTankPanel.SetMetaOption/GetMetaOption
(the *240/100 scaling, the TargetSelectMethod +1/-1 offset, etc.) — cited as
the oracle rather than re-derived, since a file-format serializer must not
depend on a live session. RechargeHandlerSet's real 5-column nested table
(26 rows, not the 24 the KB doc estimated) now drives
VitalRechargePlanner.Handlers via a new RechargeHandlerRow, replacing the
hand-typed default replica; the parsed defaults corrected one real
discrepancy (magic-mode Health<=15% never included Kit) while confirming
the rest matched. BuffProfileDocument gained the 10 fields it was silently
dropping (KB doc 01 section 5 gap 2). Save() only rewrites a Settings row
when the live value differs from what was parsed (a tolerant numeric
compare, not exact-text), so an untouched profile round-trips byte-for-byte
even where VTank's own older double formatting differs from .NET 10's.
A3 — MetafSerializer.cs ports metaf's STATE:/IF:/DO:/NAV: text grammar
(github.com/JJEII/metaf, metaf_monolithic.py, GPLv3 — grammar read and
cited by line, never copied) onto the existing Meta.cs/Navigation.cs
models. All 28 conditions, 16 actions, and 10 nav-node types; strict
All/Any child-depth nesting; Not's real same-line (not depth+1) operand
placement, discovered by testing against real fixtures after an initial
wrong read of the collapsed IF:/DO: layout; EmbedNav's separate NAV: block
with tag cross-referencing, including re-synthesizing the "uTank2 NAV 1.2"
blob MetaEngine already expects. The jump-charge 2000ms clamp (KB doc 06
row 5) is applied at .af load. All four contract proofs pass: every real
.af parses; parse-write-parse is model-identical; our binary-.met and
.nav import matches the same content loaded from metaf's own .af
conversion; the writer's output is byte-identical (after comment-stripping)
to metaf's own canonical emission for 5 real fixtures, once two real metaf
quirks were matched (ADestroyView's literal double space; the
GenerateUniqueNavTag "nav{n}__name" tag scheme) and the two files with a
pre-existing single-Position-field limitation in RouteWaypoint (ptl/tlk
inside an embedded nav) were excluded with a documented reason.
Also: the .utl BuffedInt/BuffedDouble base-key-exists gate (KB doc 05
section 2.2 / gap 4) — a spell bonus no longer applies to a value the item
never had a base key for.
Fixtures: the owner's own defaultsettings.usd/owner-{a,b,c}.usd+.ast,
4 .utl loot profiles, and a hand-picked set of real metas-repo .af/.met/.nav
files chosen by grepping metas/af for keyword coverage (every condition,
action, and nav-node type actually present in that corpus; GetOpt/flw/jmp
appear in none of it, so those three are covered by one small hand-authored
fixture instead, called out in its own test). Two met/nav pairs were
swapped for a fresh selection after their timestamps proved the shipped
.af had drifted from a since-re-recorded .nav (a real data-consistency
issue in the source repo, not a port bug).
Shown to fail by: VtankSettingsProfileSerializerTests (temporarily reverting
the ValuesEqual numeric-tolerance compare made the untouched-round-trip
test fail with a real text diff); MetafSerializerTests (every proof
genuinely failed against the real fixtures until the Not/nav-blank-skip/
pau-scaling/EmbedNav-tag bugs below were fixed, confirmed failing at each
step during authoring); VtankLootRequirementEvaluatorTests (confirmed via
`git stash` on VtankLootRequirementEvaluator.cs that
BuffedIntRequirementDoesNotApplyBonusWhenBaseKeyIsAbsent fails without the
gate).
Deviation: added -text entries to the root .gitattributes, scoped to only
the new tests/AcDream.Plugins.MossTank.Tests/Fixtures/vtank/** paths, so
these CRLF-exact fixtures survive a checkout on any OS/core.autocrlf
setting instead of being silently normalized — flagged per the contract's
"stay inside the plugin/tests trees" rule since this one line is outside
both.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
None of VTank's eight HudList instances relies on row selection
(PluginCore.cs Monsters/Items/Meta/Route/Buffs/Consumables cell
handlers) — every real text cell is an action target. Add an optional
onclick="{Action<int>}" to <column type="text">: when present, a click
fires it with the row index INSTEAD of selecting; when absent, the
original select-the-row behavior is unchanged. BuildListColumn now
takes the column's own index (used for its onclick throw message and
threaded through for the rest of the fix round's column-context
messages); the unknown-<column type> throw also picked up the index
as a side effect of that signature change.
New tests (MarkupListColumnsTests): the onclick attribute binds and
falls back correctly when absent, a malformed onclick throws at Build,
and a click actually reaches the bound callback instead of firing
SelectionChanged — each shown to fail first (missing TextClicked
member / no 4-arg Text() overload before the production change).
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
VVS HudList parity (docs/research/vtank-kb/08-ui-views.md 2-3): <list>
accepts <column type="text|check|icon" width=... [iconkind] [colors]/>
children, each binding its own per-row source parallel to the row count.
Row count is the longest bound column; a click in a text column selects
(the list's own selected/onchange, unchanged), a click in a check/icon
column fires that column's own required onchange/onclick with the row
index and does NOT change selection. Last column always absorbs the
remaining width, recomputed live off the list's own Width rather than
baked in at Build. Per-cell horizontal clipping via UiRenderContext's
existing PushClip/PopClip. The check-column glyph reuses <toggle>'s own
five-band lamp primitive (UiMarkupToggle.DrawLamp/colors promoted from
private to internal) so it looks like every other checkbox. Unknown
column type, a missing required column binding, <column> combined with
the legacy items/icons/colors list attributes, or any non-<column> child
of <list> all throw FormatException at Build. A column-less <list> is
byte-for-byte the original single-text-column widget (new if/Columns
branch in OnDraw/OnEvent; the legacy branch's code is untouched).
New UiMarkupListColumn model (src/AcDream.App/UI/UiMarkupListColumn.cs)
carries each column's kind/width/bindings; MarkupDocument's `list` case
now builds either the legacy single-column fields or a Columns list,
never both. MarkupListColumnsTests (27 new tests) cover parse/binding
validation, draw-level column-offset/clipping/check-glyph/icon pins
against the recording renderer, hit-test routing, and a backward-
compatibility proof (a column-less list built through MarkupDocument
produces a byte-identical draw record to a hand-built UiMarkupList with
equivalent fields). Every new assertion was verified to fail first via
targeted temporary mutations (hit-test isolation, clip removal, last-
column-absorbs-remainder, required onchange/onclick, row-count = max),
each reverted after confirming failure.
docs/plugin-ui-markup.md gets a full Columns section (attribute grammar,
a Monsters-tab-style example, the no-header-row idiom, backward
compatibility) and the old single-text-column LIMITATION note is
retired; the bindings truth table gains the six new column-attribute
rows.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Implements docs/plans/2026-09-06-mosstank-mode-arbitration.md.
The buff pass used to fire TryCast with no regard for combat mode or
which caster was wielded. ACE's Player_Magic.cs:84-95 drops any cast
that arrives while CombatMode != Magic, and Player_Combat.cs:778+
(GetEquippedWand) requires a wielded caster before Magic mode can be
entered at all — so a buff pass started from Peace or Melee silently
cast nothing.
Design A: BuffCasterPreparer (new) is the single owner of "which
caster do we buff with and how do we get into Magic mode". It resolves
the wielded caster first, else the first profiled caster (same
membership predicate as VitalRecharge), wields it through Peace when
needed, then requests Magic — gating the buff queue on Ready. A
missing caster stops the pass with VTank's own notice, posted once
per Reset. A stuck mode request retries on a 2s cadence up to
VitalSettings.DropToPeaceModeRetryCount before stopping and naming the
stuck stage. Hosts that don't model combat-mode automation at all
(EnterMode returns Unavailable) bypass the gate rather than deadlock,
matching the existing TickEquipment convention for older/no-window
hosts.
Design B: MacroIdleModeArbiter (new) is the single owner of "Peace
Mode When Idle", deleting CombatController's own idle-peace branch.
The old branch only ran from CombatController's own no-target state,
which a disabled combat policy never reaches — so a running macro
with combat disabled never dropped to peace. The arbiter ticks after
every controller in MossTankPanel.OnTick and covers that case.
Tests 1-7 of the plan: BuffCasterPreparer (wielded-caster fast path,
wield-then-magic ordering, no-caster notice latch, exhausted retry
budget) and MacroIdleModeArbiter (retry gate, suppression, IdlePeaceMode
off, the disabled-combat case) are added to CombatControllerTests.cs,
reusing its FakeAutomation extended with deferred mode/equip
confirmation and a call log. A full buff-then-fight panel scenario is
added to MossTankPanelTests.cs via a new CombatCapableFakeAutomation.
CombatControllerTests' IdlePeaceIsTheNoTargetFallback is deleted and
re-pinned on the arbiter. Every new test was confirmed to fail (by
compile error or by runtime assertion) against the unmodified
production code via a temporary git stash before its fix landed.
337 -> 345 AcDream.Plugins.MossTank.Tests (336 baseline sans the moved
test, plus 9 new); AcDream.App.Tests MossTank/Plugin filter (79 tests,
including MossTankMarkupContractTests) stays green with no markup
changes.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Owner report 2026-09-06: MossTank's shelf icon (0x06002C41) drew with a white
ring. DAT icon art reserves pure-white-opaque pixels as the recolor key that
retail IconData::RenderIcons (0x0058d180) replaces per pixel through
SurfaceWindow::ReplaceColor (0x004415b0) from the effect tile — the solid-black
0x21 tile when there are no effects. The inventory already does this through
IconComposer; the plugin did sink (markup <icon did>, <button icon>, <list icons>
and the shelf button) blitted the art raw.
RetailMarkupIconResolver.ResolveDid now hands out IconComposer.GetKeyedIcon —
the drag-icon composite (base art + effects==0 recolor, no overlay, no
underlay), sharing that cache — so did icons look like a mundane inventory item
does. The resolver no longer needs a TextureCache. KeyedIconInstalledDatTests
pins both halves against the real DAT: the raw art carries the key, the
composite carries none, and ResolveDid returns exactly the keyed composite.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
- Split the two hermetic RetailMarkupIconResolver memoization tests (and
their counting fakes) out of the Lane=InstalledDat class into a new
untagged RetailMarkupIconResolverMemoizationTests.cs so CI's portable
filter (Lane!=InstalledDat) actually runs them.
- PluginSidePanel: move the entry button's Anchors = AnchorEdges.None from
the Add() call site into PluginShelfButton's own constructor (same
comment carried over) so a second construction path cannot miss it.
- UiRectOutlinePainterOrderTests: assert the back panel's border segment
carries exactly 4 quads (24 vertices, FloatsPerVertex each) so a partial
outline cannot pass the painter-order check.
- RetailMarkupIconResolver: document the type as UI-thread-only (every
caller is a draw-time icon source) and bound the MISS cache to 256
entries with FIFO eviction — HIT entries stay unbounded (bounded by the
DAT's own surface count already). New test proves the 257th distinct
miss evicts the first (re-probe count rises); verified failing first
against the un-bounded code (Expected 258, Actual 257) before restoring
the fix.
- docs/plugin-ui-markup.md: split the icon-binding row's failure mode into
Build-time (missing property only — the binder never checks CLR type)
vs. draw-time (a resolved value that cannot convert to a number throws
from the draw, not from Build).
- docs/ISSUES.md: filed #486 (credits picture scroll frozen by the
per-draw anchor pass) and #487 (radar compass tokens candidate, same
mechanism, unconfirmed); corrected #461's causality — the graceful
logout/reveal-cancel log lines are printed by LiveSessionController.Tick's
catch -> StopAfterFailure -> StopCore AFTER the motion-update exception,
then it rethrows, so the logout is a consequence of the crash, not its
cause; real chain is the #462 stalled login-reveal materialization
leaving PlayerMovementController in RuntimeOwnedDormant outside its
SetPosition ground phase when an inbound 0xF74C arrives.
- Plan doc: recorded the three fix-round commits' verdicts (all PASS) and
the Smoke-plugin cleanup commit SHA in the Review ledger, plus a pointer
to the two newly filed issues.
Verified: dotnet build AcDream.slnx -c Release (0/0), targeted filter
85/0/0, full App suite 7364 passed / 97 skipped / 36 failed (36 pre-existing
InstalledDat/Manual/Linux-only failures, unchanged by name from baseline;
net +1 passed test from the new eviction test).
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Owner direction 2026-09-06: the owner visually accepted all three connected
gates from the plugin-shelf/DAT-icon work — the movable plugin shelf
(Slice A), the DAT icon markup (Slice B), and the retained-UI outline-order
fix (761a7519f). With that gate passed, the Smoke plugin's job as the gate
artifact is done, so it is deleted outright rather than merely hidden:
src/AcDream.Plugins.Smoke/ (SmokePlugin.cs, SmokeIconPanel.cs, csproj, lock
files).
Reference sites cleaned:
- AcDream.slnx: removed the project entry.
- src/AcDream.App/AcDream.App.csproj: removed the Smoke ProjectReference and
the CopySmokePluginToBuildOutput/CopySmokePluginToPublishOutput targets;
the MossTank equivalents are untouched.
- .github/workflows/headless-portability.yml: the Linux package-contract
step no longer asserts a Smoke plugin.dll/plugin.json pair — repointed at
MossTank's, since the step's job was to prove the plugin-copy packaging
mechanism works end to end, not specifically to prove Smoke.
- tests/AcDream.Core.Tests/Plugins/PluginManifestTests.cs: the inline JSON
fixture used Smoke's manifest values as arbitrary test data; swapped for
MossTank's so the parser test still proves the same thing.
- tests/AcDream.App.Tests/Rendering/LinuxPlatformBoundaryTests.cs: the
shipped-plugin-copy shape test counted 4 GetTargetPath targets (Smoke +
MossTank, build + publish); now 2 (MossTank only).
- tests/AcDream.App.Tests/Plugins/AppAutomationSurfaceIconInstalledDatTests.cs:
reworded a doc comment that named the now-deleted SmokeIconPanel.
- README.md, docs/plugin-ui-markup.md: dropped Smoke-specific mentions,
kept the icon markup example/grammar (now citing MossTank's own real
IconSurfaceId).
- docs/plans/2026-09-06-plugin-shelf-and-dat-icons.md: recorded the owner's
2026-09-06 acceptance and the Smoke removal in the review ledger; deleted
the now-moot "before shipment" Smoke-in-release-zip warning.
- docs/reviews/coverage-ledger.md, docs/reviews/findings-ledger.md: left
untouched — both are frozen audit snapshots ("complete for baseline
<hash>"), so their Smoke rows are historical record, not live claims.
- docs/ISSUES.md: left untouched — its Smoke mentions are inside closed
issue #193's historical write-up of a past investigation.
MossTank plugin shelf icon: MossTankPlugin.cs's PluginPanelDescriptor now
sets IconSurfaceId = 0x06002C41 (IconText = "MT" remains the fallback).
Verified against the installed retail DAT with a new InstalledDat-lane test,
tests/AcDream.App.Tests/UI/MossTankIconInstalledDatTests.cs, mirroring
RetailMarkupIconResolverInstalledDatTests's convention: confirms the id is a
real Portal/HighRes RenderSurface and that RetailMarkupIconResolver.ResolveDid
returns a non-zero texture for it.
Verified: dotnet build AcDream.slnx -c Release green; a stale
plugins/AcDream.Plugins.Smoke output folder from a prior build was deleted
and a fresh build does not recreate it. Full App suite: 7,363 passed / 97
skipped / 36 failed (was 7,362/97/36) — the failing set is unchanged and
none are plugin-related; the one new pass is the MossTank DAT-icon test.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Bounded residual round on the Slice B review fix commit (466272ec5),
docs/plans/2026-09-06-plugin-shelf-and-dat-icons.md.
N1 (BLOCKING): PluginSidePanel's shelf entries resolved icons through
_bindings.Assets.ResolveSprite (= ResolveChrome =
TextureCache.GetOrUploadRenderSurface), which returns a non-zero 1x1
magenta placeholder for a missing id - so PluginShelfButton's initials
fallback could never fire in production. RetailUiRuntime.MountPlugins
now passes the same iconResolver.ResolveDid every markup icon sink
already uses, which returns (0,0,0) for an unresolvable id. Re-pointed
the existing initials-fallback unit test at a resolver matching
ResolveDid's real contract, and added an InstalledDat-lane test
(ShelfButton_BogusDescriptorId_OnTheRealResolver_FallsBackToInitials)
proving a bogus descriptor id on the REAL resolver yields initials.
N2: MarkupDocument's <button icon>/<list icons> handling only built the
uint reader (BindUintLiteralOrBinding/BindUintList) when an
IMarkupIconResolver was wired, so a malformed icon="{Typo}" or
icons="notabinding" silently loaded instead of throwing at Build on a
resolver-less host. Both readers now build unconditionally (same rule
ValidateIconKind already followed); only the IconSource/IconIdsSource
assignment stays gated on icons is not null.
N3: a negative bound icon id threw OverflowException out of
Convert.ToUInt32 every frame from inside UiSimpleButton.OnDraw (scalar
path), while the list's IEnumerable<int> path silently wrapped -1 to
0xFFFFFFFF (unchecked reinterpret). Both paths now map any
out-of-range value (negative, or above uint.MaxValue) to 0u instead -
Decal's own "no icon" convention - via a shared ToUintOrZero helper
that catches exactly the OverflowException Convert.ToUInt32 already
throws for both cases.
N4 (perf): RetailMarkupIconResolver.ResolveDid probed Portal/HighRes
(two cache misses + two B-tree lookups under the database lock) on
EVERY call for an unresolvable id, and re-entered the DAT lock on
every resolve of a hit too. Memoizes the resolved (tex,w,h) tuple per
DID, including the (0,0,0) miss, in a plain Dictionary.
N5 (nit): documented in TextureCache.GetOrUploadRenderSurface that the
(id, nearest) cache key uploads the same RenderSurface twice when both
samplers are wanted (chrome via ResolveChrome, plugin icons via
ResolveDid's nearest:true) - GetOrCreateLinearUiTwin exists but only
shares in the nearest-registered-first direction, so wiring it through
here is left as a documented nit rather than a behavior change.
N6 (nit): corrected stale SampleData.cs:64 citations to :69 (Melee
Defense's real line after the file grew) across SmokeIconPanel.cs,
PluginSidePanelTests.cs, RetailMarkupIconResolverInstalledDatTests.cs,
and docs/plugin-ui-markup.md (including the 64-82 range, now 69-83).
N7 (nit): <icon iconkind="..."> was silently ignored (icon derives its
kind from which of did/spell/item is set, unlike button/list). Now
throws FormatException at Build with a message naming the correct
surfaces; documented in plugin-ui-markup.md.
N8 (nit): the truth-table's `list colors` row now says IEnumerable<uint>
or IEnumerable<int> (shared BindUintList), matching `list icons`.
N9 (ship check): added a "Before shipment" line to the plan's Review
ledger. Confirmed the Smoke plugin (including its auto-open Icon Smoke
panel) IS included in the launcher's client-<rid>.zip release payload:
tools/publish-bin.ps1's New-PayloadZip zips App's entire publish
directory unfiltered, and AcDream.App.csproj's
CopySmokePluginToPublishOutput target runs unconditionally
AfterTargets="Publish". No behavior changed per instruction - flagged
for follow-up after the owner's connected gate.
Verification: dotnet build AcDream.slnx -c Release green; filtered
test command 108/108 passed (0 skipped) including the InstalledDat
lane; full AcDream.App.Tests suite 7362 passed / 97 skipped / 36
failed (all 36 pre-existing, same names, none touching
Markup/PluginSidePanel/RetailMarkupIconResolver/TextureCache/PluginIcons);
AcDream.Plugins.MossTank.Tests 337/337 passed.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Owner report: with the MossTank plugin window BEHIND the inventory window,
the plugin's "Force Buff"/"Cancel Force Buff" button border outlines drew on
top of the inventory paperdoll. Only outlines leaked; fills did not.
Root cause: TextRenderer composited three buckets per layer — submission-
ordered sprite segments, then ALL untextured DrawRect quads (_rectBuf), then
debug text (Flush/DrawLayer). UiRenderContext.DrawRect forwarded into that
separate rect bucket, which always flushed AFTER every sprite segment
regardless of submission order. UiRenderContext.DrawRectOutline is four
DrawRect calls, so every BorderColor outline in the retained UI (UiPanel,
UiMarkupList) composited above every window's sprite content drawn after it,
instead of only the windows actually painted before it.
Fix: UiRenderContext.DrawRect now forwards to DrawFill — the same untextured
SPRITE-bucket segment DrawFill already used for panel backgrounds — so
DrawRectOutline inherits real painter/submission order. Audited the only
other DrawRect caller (UiMeter's bg-then-bar fill, which already relied on
same-call submission order and is unaffected) and the only other
DrawRectOutline callers (UiPanel, UiMarkupList, both routed through the same
fixed chokepoint). TextRenderer.DrawRect/DrawRectOutline and the _rectBuf
bucket are left in place (not deleted) with updated doc comments noting no
caller in src/ outside TextRenderer itself reaches them anymore; a future
non-retained-UI caller (e.g. a debug overlay) could still want that
"always on top of sprites" behavior. Added TextRenderer.DebugRectVertexCount
(test-only) and a failing-first regression test
(UiRectOutlinePainterOrderTests) that builds a real UiPanel border behind a
later-added opaque sprite and asserts submission order.
No retail-divergence register row: this is a renderer ordering bug, not a
documented retail behavior deviation.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
EVIDENCE (live UI probe dump at 1280x720, build 2ebcc0164): after collapsing
the plugin shelf, PluginSidePanel rect=(1240,233,24,28) was correct, but
ShelfGripPanel rect=(1240,233,20,18) and the toggle rect=(1260,233,16,18)
stayed UNCHANGED from the expanded geometry. The toggle then sat outside the
24px shelf and the ancestor clip removed it, so the owner saw the tab with
no "<".
ROOT CAUSE: UiElement.ApplyAnchor (src/AcDream.App/UI/UiElement.cs ~829-856)
runs for every child on every draw (called at :699). For any child whose
Anchors != AnchorEdges.None, it captures the Left/Top/Width/Height margins
ONCE (_anchorCaptured) on the first draw and re-applies that snapshot every
subsequent draw, overwriting whatever PluginSidePanel.LayoutChrome/Reflow had
just set. The grip, the toggle, and each PluginShelfButton entry were
constructed with the default Anchors (Left|Top), so their first-draw
geometry froze. The shelf itself already used AnchorEdges.None for exactly
this reason. Unit tests never caught it because UiRoot.Tick does not draw —
the anchor snapshot only exists after a real Draw pass, and the prior
draw-level toggle tests only ever drew once, before any collapse.
FIX: set Anchors = AnchorEdges.None on _grip, _toggle (PluginSidePanel
constructor) and each PluginShelfButton entry (PluginSidePanel.Add) — the
shelf is the sole layout owner of these children and anchoring is the wrong
mechanism for them, not a per-reflow patch via ResetAnchorCapture().
PluginMinimizeButton is untouched (it is a child of the plugin window and
deliberately anchors Top|Right).
TESTS (tests/AcDream.App.Tests/UI/PluginSidePanelToggleGlyphClipTests.cs):
- Collapse_AfterADraw_RepositionsGripAndToggle_NotFrozenAtExpandedGeometry:
draws the shelf, collapses via a real UiRoot press/release, draws again,
and asserts the grip/toggle geometry actually reflects the collapsed
Width/Height rather than the frozen expanded snapshot. Failed-first
(pre-fix) at line 364 with "Expected: 8, Actual: 20" (grip.Width frozen at
the pre-collapse value instead of the new collapsed Width - ToggleWidth).
- MultiColumnReflow_AfterADraw_EveryRemainingButtonMatchesAFreshSinglePassLayout:
12 entries, draw, unregister one window (a real removal), draw again, and
compares every surviving button's geometry against an independent
reference shelf built directly with the same final 11-entry set. Failed-
first with "Expected: 16, Actual: 48" (a surviving button's Left frozen at
its stale 12-entry column/row instead of the fresh 11-entry reflow).
Both tests use font: null (bitmap fallback) so they run in every CI lane
without an installed retail DAT, unlike the Lane=InstalledDat tests above.
VERIFY: dotnet build (Release) green for src/AcDream.App and the test
project. Targeted filter (PluginSidePanel|Markup|UiRootInput): 130/130
passed. Full tests/AcDream.App.Tests suite: 7353 passed / 97 skipped / 36
failed - matching the stated baseline (7351/97/36) plus the two new tests;
the 36 failures are the pre-existing environment-gated set (installed-DAT
version mismatch, Linux-only waiter, Lane=Manual live-mount probes) and are
unrelated to this change.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Two Opus reviews of commit 8217a349e (Slice B: DAT icons in plugin
markup) found one BLOCKING defect and 14 SHOULD-FIX findings. All 15
fixed here in one commit per the review contract.
BLOCKING (finding 1): an unresolvable did painted a magenta square.
TextureCache.GetOrUploadRenderSurface's 1x1 magenta placeholder for a
missing RenderSurface is load-bearing for authored chrome, but
RetailMarkupIconResolver.ResolveDid only short-circuited did==0, so any
other unresolvable id fell through to that placeholder and got scaled
up by UiMarkupIcon/UiMarkupList/UiSimpleButton -- the classic
resolve(0)-style footgun (claude-memory/feedback_ui_resolve_zero_magenta.md),
just triggered by a missing id instead of a literal 0. Fixed by probing
Portal/HighRes existence via IDatReaderWriter.TryGet<RenderSurface>
BEFORE ever calling GetOrUploadRenderSurface -- that TryGet already
serializes concurrent DAT access internally (DatDatabaseWrapper's own
_databaseLock), the same synchronization IconComposer.TryDecode relies
on, so no additional lock was needed. RetailMarkupIconResolver now
takes IDatReaderWriter + TextureCache directly (RetailUiAssets gained a
TextureCache field, its one construction site in
InteractionRetainedUiComposition.cs updated) instead of the old
resolveSprite delegate, since it also needs the nearest-sampled upload
path for finding 6 below.
Finding 2 -- Smoke panel wiring bugs: its list fed iconkind="spell"
raw art DIDs (PluginSpellInfo.IconId) instead of spell ids, so
IMarkupIconResolver.ResolveSpell composited the wrong (or no) badge
every row. SmokeIconPanel.Binding.SpellIds now yields SpellId (the
printed text still shows IconId alongside). The bare-index demo and the
descriptor both moved from the unverified literal 7735 to 0x165 --
retail's real Melee Defense skill icon (SampleData.cs:64,
0x06000165) -- so the owner's visual gate proves real art, not a guess.
StartVisible flipped true, and a character with no self-buffs known
falls back to spell 1's real catalog entry (or an honest "no spells
known" row with icon 0 if even that fails) rather than fabricating art.
Finding 3 -- PluginIcons.Normalize's threshold was silently rewritten
from the contract's 0x01000000 to 0x06000000 during Slice B. Restored
to 0x01000000; the class/method XML docs now state the number directly
(no cref to the private const); the test table adds 0x02000000 (a value
that only distinguishes the two thresholds) and 0x01000000 itself
(passes through unchanged).
Finding 4 -- an unknown iconkind (e.g. "spel") only threw when a
resolver happened to be wired, because BuildIconSource/
BuildRowIconResolve validated inside their own null-icons early return.
A new ValidateIconKind helper runs UNCONDITIONALLY before that branch,
so a malformed iconkind is a Build-time author error on every host.
Finding 5 -- BindUintLiteralOrBinding required an exact uint property
type, rejecting the int-typed bindings Decal-facing code commonly uses
(MosswartMassacre's HudPictureBox.Image is int end to end). It now
matches BindUint's existing leniency: any property, converted via
Convert.ToUInt32 at read time. BindUintList likewise now accepts
IEnumerable<int> alongside IEnumerable<uint> (unchecked per-element
reinterpret -- icon ids never go negative in practice).
Finding 6 -- TextureCache._renderSurfaceGpuTextures was keyed by id
alone, so whichever caller asked for a given RenderSurface id FIRST won
the sampler for every later caller of the same id -- UiDatFont's glyph
atlases already request nearest:true while ResolveChrome's background
art requests nearest:false, so this was a real, reachable collision,
not hypothetical. Rekeyed to (id, nearest); RetailMarkupIconResolver.
ResolveDid now requests nearest:true (pixel-exact 32x32 icon art);
ResolveChrome is untouched (still nearest:false/linear). Audited every
other _renderSurfaceGpuTextures use site (TryGetValue/set/Dispose
iteration+Clear) plus the separate _nearestUiTextureSources/
_linearUiTwinHandles/_uploadMetadata dictionaries (all keyed by handle
or accounting name, unaffected) -- no other eviction/accounting path
assumed id-only keying.
Finding 7 -- column-reservation semantics, per the DECIDED shape:
MarkupDocument now sets button.IconSource / list.IconIdsSource +
IconResolve ONLY when a resolver (icons parameter) is actually wired --
previously button.IconSource was always assigned (even to an
always-empty func on an icons:null host); combined with this finding's
other half -- UiSimpleButton.OnDraw now reserves its icon column
whenever IconSource is non-null, regardless of a per-frame resolve miss,
so a bound id that goes briefly to 0 no longer slides the caption back
and forth -- would have permanently reserved a blank column on such a
host. UiMarkupList already reserved its column whenever IconIdsSource
was set; no draw-side change needed there.
Finding 8 -- added a with/without-icons comparison test for
UiMarkupList (mirroring the existing UiSimpleButton one): asserts the
row text quad's x is strictly greater with an icon column present, and
the icon quad itself has non-zero width.
Finding 9 -- <icon tooltip=""> (empty string) was still treated as
"has a tooltip" by a bare attribute-presence check, making the icon
swallow clicks with no visible tooltip ever appearing. Now uses
!string.IsNullOrWhiteSpace, matching ApplyCommon's own predicate for
every other element's tooltip.
Finding 10 -- PluginShelfButton.OnDraw drew nothing when a non-zero
descriptor icon id resolved to no texture (a bad Decal index, a DAT id
from a different install), rather than falling back to Initials the
way a zero id already did. Now decides once, on the first draw
(memoized, so Initials' string work never repeats every frame): a
failed resolve permanently switches Text to the initials fallback,
computed and assigned BEFORE base.OnDraw actually paints the caption.
Finding 11 -- MarkupDocument.AddElement's switch had no default arm, so
an unknown or miscased element name (<Icon>, <butotn>) silently
vanished from the built tree instead of failing loudly like every
other malformed-markup case. Added a default arm that throws
FormatException. Ran AcDream.Plugins.MossTank.Tests (337/337,
unchanged) and the full App markup suite to confirm no existing markup
relies on an unknown element.
Finding 12 -- PluginPanelDescriptor.IconSurfaceId's XML doc now states
that a bare Decal index is accepted and normalized, citing
PluginIcons.Normalize.
Finding 13 -- docs/plugin-ui-markup.md: replaced the blanket "wrong
type/missing property throws at Build" sentence with the per-attribute
truth table the review produced (which attributes are silent at
runtime vs. throw at Build, and each one's bound CLR/delegate type);
restated the icon-id boundary as 0x01000000; added the "do NOT add
0x06000000 to the four already-full IconId records" warning (citing
SkillBase._iconID / UIRegion::SetImageByDID @0x004f150e); documented
that 0x-prefixed hex is required (an unprefixed all-digit literal
parses as decimal); noted unknown element names now throw; called out
list colors (0xRRGGBB) vs. color=/background=/border= (#AARRGGBB) as
non-interchangeable grammars; documented the root <panel visible>
binding-only exception; corrected the shelf's collapse toggle glyphs
(</>, not the old doc's arrows) and the 28px collapsed-tab size; added
the IconId record-equality API-v1 note; and called out iconkind as
per-<list> (mixed id spaces need pre-normalized DIDs; the composited
spell badge has no did-space escape hatch) plus the existing
one-text-column LIMITATION being deferred to MossTank.
Coverage added for finding 14: a PluginSidePanelTests case proving the
shelf button normalizes a bare descriptor index before resolving, and a
reflection-based unit on AppAutomationSurface.ProjectWorldObject (its
public callers gate on IsAvailable, which needs a fully connected
session heavier than this mapping needs -- the plan's own documented
fallback) proving PluginWorldObject.IconId carries ClientObject.IconId
through unchanged; PluginInventoryItem.IconId uses the identical
one-line pattern inline in CaptureOwnedItems, reviewed by inspection.
Finding 15: recorded a "Review ledger" section in the plan doc with
both slices' commits, both review verdicts, and the two items
explicitly deferred to the MossTank plugin work (multi-column list,
root literal visible).
Verification: full solution builds green. Targeted filter
(Markup|PluginSidePanel|PluginIcons|AppAutomation|TextureCache|
UiDatFont) passes 131/131, including the two InstalledDat-lane tests
(RetailMarkupIconResolverInstalledDatTests,
AppAutomationSurfaceIconInstalledDatTests) actually resolving against
the real installed DAT, not skipping. AcDream.Plugins.MossTank.Tests
passes 337/337 unchanged. Full AcDream.App.Tests suite: 7351 passed /
97 skipped / 36 failed -- identical failure set/count to the
7334/97/36 baseline (the +17 passes are exactly the new/expanded
tests: 2 new PluginIconsTests.Normalize theory rows, 10 new
MarkupIconTests cases, 2 new PluginSidePanelTests cases, 1 new
AppAutomationSurfaceTests case, and the 2 new standalone test files).
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Owner report: "I dont see the < after I minimize the window" — after
collapsing the plugin shelf with the > toggle, the < (expand) glyph was not
visible.
Root cause, established with a real-DAT draw probe (font 0x40000000) before
changing anything: '<' and '>' share IDENTICAL glyph metrics (OffsetY=4,
Width=5, Height=7, VerticalOffsetBefore=5), so there is no per-glyph
asymmetry to explain "I see one but not the other." Against the pre-fix 12px
grip band, the toggle's FILL glyph plane measured fully INSIDE the band in
both states (local y=[3,10] of [0,12]) — the "16px line box overhangs a 12px
band" theory alone does not erase the glyph, so a bare clip fix would not
have addressed the report. What IS true: the border-inflated OUTLINE
(background/shadow) plane, drawn first per retail's UIElement_Text::DrawSelf,
spans y=[-1,14] before clipping and was cropped by the band's self-clip
(UiElement.ClipsChildren) to exactly [0,12] — a real but minor defect. The
actual explanation for the report is discoverability: the collapsed shelf
shrank to a bare 24x12 near-black sliver at the screen edge, several times
smaller than any other clickable affordance in the UI — easy to overlook even
though its pixels were, in fact, being drawn.
Fix (src/AcDream.App/UI/PluginSidePanel.cs):
- ExpandedGripBandHeight (new internal property) derives the EXPANDED
grip/toggle band from the real font metrics — max(12, font.LineHeight + 2)
— so neither the fill nor the border-inflated outline plane can clip for
any font; the 12px constant remains only as the bitmap-font fallback.
Threaded through the ctor's initial Height, OnTick's row-wrap height calc,
LayoutChrome, and Reflow's entry-Top/expanded-Height math.
- The COLLAPSED tab is now ButtonExtent (28px) tall instead of the 12px grip
band — the same size as an ordinary entry button — with the toggle glyph
filling and centering in the taller band. This is the actual fix for the
report: the collapsed affordance is now button-sized and findable, not a
bug-for-bug-identical-but-larger clip fix.
Tests:
- tests/AcDream.App.Tests/UI/PluginSidePanelToggleGlyphClipTests.cs (new,
Lane=InstalledDat): loads the real DAT font, builds a live shelf, and
proves via TextRenderer.DebugSpriteSegmentVerts that the toggle's ink (fill
+ outline) is fully contained in its own clip band in BOTH the expanded
('>') and real-click-collapsed ('<') states, plus that the collapsed tab is
button-sized. Verified failing against the pre-fix code (git stash of just
this file) with concrete numbers: computed unclipped span [-1,14] does not
fit inside the 12px band; collapsed height measured 12 (not button-sized).
Passes after the fix.
- tests/AcDream.App.Tests/UI/PluginSidePanelTests.cs: added
CollapseThenExpand_WhileStillDocked_ReturnsToTheIdenticalLeftAndTop (the
dock/anchor invariant survives the collapsed-height change), and derived
Drag_StartingOnShelfPadding_DoesNotMoveTheShelf's press-below-the-grip-band
Y coordinate from the new ExpandedGripBandHeight accessor instead of a
re-hard-coded literal.
Verification: dotnet build (App + tests) green. Filtered run
(PluginSidePanel|UiDatFont|Markup|UiRootInput) 131/131 passed, 0 skipped —
the InstalledDat lane tests actually ran (DAT dir resolved). Full
AcDream.App.Tests suite: 7334 passed / 97 skipped / 36 failed (baseline was
7331/97/36) — the 3 new tests are the only delta; the 36 failed test names
are byte-identical to the pre-existing set (cathedral collision installed-dat
gates, alpha-flush conformance, layout live-mount probes, Linux frame-pacing,
credential resolver — all unrelated to this change).
No retail-divergence register change needed: IA-27 already covers the
plugin shelf's non-retail collapse-toggle glyphs/behavior in general; this
is a bug fix within that already-declared deviation, not a new one.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Owner request: plugin panels (Decal/VirindiViewService-class, per the
MosswartMassacre reference usage) need to embed real DAT icons the way
FlagTrackerView.SafeSetListImage does — spell/skill art, raw portal
indices, and a window icon. This is Slice B of
docs/plans/2026-09-06-plugin-shelf-and-dat-icons.md (Slice A, the
movable/collapsible shelf, landed in 01b98ca30/4fada238e/718005b21).
What shipped:
- AcDream.Plugin.Abstractions.PluginIcons.Normalize: the one Decal-style
bare-index -> 0x06xxxxxx RenderSurface DID grammar, applied at every
icon SINK (descriptor IconSurfaceId in PluginShelfButton, and markup
<icon did>/<button icon>/<list icons> did-kind ids) rather than on the
plugin-facing records, which already carry real DIDs read straight
from the client's tables.
- PluginSpellInfo.IconId / PluginSkillInfo.IconId /
PluginInventoryItem.IconId / PluginWorldObject.IconId: additive init
properties (default 0), filled in AppAutomationSurface from
SpellMetadata.IconId (already projected from SpellBase.Icon by
RetailSpellMetadataProjector — no gap there), a new BindSkillIcons
parallel to BindSkillNames (GameWindow reads
DatReaderWriter.Types.SkillBase.IconId — confirmed via reflection over
the installed Chorizite.DatReaderWriter package, since its XML docs
don't cover Pack/Unpack-generated public fields: Description, Name,
IconId (uint), TrainedCost, SpecializedCost, Category, ChargenUse,
MinLevel, Formula, UpperBound, LowerBound, LearnMod), and
ClientObject.IconId in CaptureOwnedItems/ProjectWorldObject.
- IMarkupIconResolver (AcDream.App.UI): ResolveDid/ResolveSpell/
ResolveItem. MarkupDocument.Build gains an optional parameter (null by
default -> every icon sink resolves to nothing rather than throwing,
so pre-Slice-B callers/tests are unaffected). RetailUiRuntime.
MountPlugins builds ONE RetailMarkupIconResolver per pass from
RetailUiAssets.ResolveSprite + RetailUiAssets.Icons (the shared
IconComposer) + Toolbar.Objects (the SAME ClientObjectTable
Magic/Toolbar bindings already borrow for their own icon resolution —
no second object lookup introduced).
- New UiMarkupIcon widget (<icon x y w h did|spell|item tooltip>):
exactly one source required (FormatException at Build otherwise,
matching every other malformed-attribute rule), aspect-preserved,
centered, click-through unless a tooltip makes it a real hit-test
target.
- UiSimpleButton.IconSource and UiMarkupList.IconIdsSource/IconResolve:
additive, default null/no-op, so every existing button/list caller
(including the plugin shelf's own toggle/minimize buttons) is
unaffected. Button icon draws flush left and shifts the caption's
centering region right; list icons reserve a leading RowHeight-2
column (Decal's IconColumn) and skip rows whose id is 0 or
unresolvable.
- MarkupDocument centralizes the did/spell/item dispatch (including
PluginIcons.Normalize for did) in two small helpers (BuildIconSource
for <icon>/<button>, BuildRowIconResolve for <list>) so all three
markup surfaces share one resolver call path.
- AcDream.Plugins.Smoke ships a RegisterPanelContent (in-memory KSML,
no plugin-side .xml file) proof panel exercising every new surface:
a bare-index <icon>, a literal-hex <icon>, a composited <icon
spell=...>, a <button icon=...>, and a <list icons=... iconkind=
spell> of the first five known self-buffs with their IconId printed
alongside. Descriptor IconSurfaceId reuses the same bare index to
prove the shelf button and the panel's own icon normalize identically.
- docs/plugin-ui-markup.md is the new SSOT for the full markup
vocabulary + icon grammar + the Slice A shelf; linked from
docs/README.md and docs/plans/2026-04-24-ui-framework.md.
Design decisions where the plan left room:
- Normalize runs inside the resolver dispatch (BuildIconSource/
BuildRowIconResolve), not scattered at each markup call site, so
every did-kind sink shares one choke point.
- did/spell/item all accept either a literal (decimal or 0x-hex) or a
{Binding}, via one BindUintLiteralOrBinding helper, for symmetry —
the plan only showed spell/item as bindings but didn't forbid a
literal.
- <icon> requires exactly one source INCLUDING zero (not just two);
an icon with no source is not a coherent element.
- The button/list icon draw math (icon column extent, padding) lives
in the widgets themselves (UiSimpleButton/UiMarkupList), not in
MarkupDocument, keeping the parser only responsible for wiring
Func<(tex,w,h)> sources.
Tests: PluginIconsTests (Normalize table), MarkupIconTests (icon/button/
list resolver dispatch via a fake IMarkupIconResolver, plus draw-level
pins via the RecordingGpuDevice/TextRenderer apparatus already used by
UiAncestorClipTests/UiRenderContextDrawStringDatOutlineTests — "draws
nothing when unresolvable" and "button/list icon shifts the text"),
and AppAutomationSurfaceIconInstalledDatTests (Lane=InstalledDat: a
known spell's IconId matches the real installed SpellTable's own Icon
field exactly). Verified every new test fails to COMPILE without this
change (git-stashed the src/ changes, rebuilt the test project: CS0246
on IMarkupIconResolver) before restoring. Full App suite: 7331 passed /
97 skipped / 36 failed (identical pre-existing failure set/count to the
7306/97/36 baseline; the +25 passes are exactly the new tests).
AcDream.Plugins.MossTank.Tests (the main consumer of the touched
Plugin.Abstractions records) passes 337/337 unchanged, confirming
API-v1 binary/source compatibility. Full solution builds green.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Opus re-review of 4fada238e (Slice A review-fix round) left seven residuals.
All seven addressed here, in one commit per the residual-round contract:
NEW-1 (test-coverage): no test pinned the Draggable:true->false change
itself. Added Drag_StartingOnShelfPadding_DoesNotMoveTheShelf, pressing the
shelf's own PADDING (left of the first button's Left=4, below the 12px grip)
and dragging 100px through the real UiRoot press/move/release path. Proven
to fail against the pre-4fada238e shelf: temporarily setting Draggable=true
in the ctor and re-running this test moved the shelf from Left=760 to 764
(clamped to parent.Width-Width) instead of leaving it in place — reverted
after confirming.
NEW-2 (correctness): RetailWindowLayoutPersistence.ClampAllToScreen clamps
generically to screen.Width-handle.Width, 4px (OuterPadding) off what the
shelf's own dock formula (parent.Width-Width-OuterPadding) would produce for
the same new parent size. A screen shrink that triggers that clamp used to
flip _userPositioned permanently even though nothing a user would call
"dragging" happened. PluginSidePanel.OnHandleMoved now re-derives "still
docked" at comparison time as a function of the CURRENT parent size: either
the live dock formula's own result, or what ClampAllToScreen's clamp would
produce from the PREVIOUS docked position — only a position matching
NEITHER flips _userPositioned. New test
ClampAllToScreen_AfterShrinkingTheRoot_DoesNotFlipAnchoring_ButARealDragStillDoes
shrinks the root, runs ClampAllToScreen, confirms the shelf still anchors
top-right through a later reflow and a collapse, then confirms a genuine
grip drag afterward still flips anchoring.
NEW-3 (decision, documented): KeepWindowReachable's per-tick clamp and
ClampAllToScreen's screen-resize sweep both touch plugin windows. Evidence
gathered and recorded as a doc comment on KeepWindowReachable:
RetailWindowManager.MoveTo (line 178) already short-circuits an unchanged
position before ever raising Moved; persistence's ScreenSize and Host.Root's
size derive from the same d.Window.Size and are reconciled every frame
(UiHost.Draw sets Root.Width/Height from screenSize); and because
RetainedGameplayUiFrame.Render ticks BEFORE it draws in the same frame,
ClampAllToScreen always resolves a screen-resize's clamp before
KeepWindowReachable ever observes the new size next tick, making
KeepWindowReachable's own pass a structural no-op for that case (never a
second write). KeepWindowReachable is NOT dropped, though: it is the only
reachability guarantee for (a) a plugin window whose geometry is mutated
directly rather than through MoveTo — pinned by the pre-existing
FullWidthPluginWindowStartsAndStaysReachableAtMinimumCanvas test, which
exercises exactly that with no screen resize at all — and (b) a host wired
with no RetailWindowLayoutPersistence at all. Decision: keep both; they do
not race.
NEW-4 (robustness): RetailWindowLayoutPersistence._attached is mutated
mid-session by WindowRegistered/WindowUnregistered (a callback invoked from
inside one of these loops — e.g. Apply -> Show()/Hide() -> a controller
unregistering another window — could otherwise mutate the list being
enumerated). All five bare `foreach (... in _attached)` loops
(RestoreAllCore, ClampAllToScreen, SaveAll, SaveNamed, RestoreNamed) now
snapshot with .ToArray(), matching Dispose's existing pattern.
NEW-5 (retail-faithfulness): the plugin-shelf hide message hard-coded
"Shift+Ctrl+F1" even though InputAction.TogglePluginManager is rebindable
through Configure Keyboard. RetailUiRuntime.PluginShelfHiddenMessage now
looks up the CURRENT binding via the live InputDispatcher and formats it
through Layout.RetailKeyNames.Describe — the exact formatter Configure
Keyboard's own row captions already use
(Layout/KeyboardConfigController.cs:284) — falling back to an honest
"bind it in Configure Keyboard" message when the action is unbound or no
dispatcher is wired.
NEW-6 (bookkeeping): added divergence-register row IA-27 for the plugin
shelf's repurposing of retail's plugin-manager chord (Shift+Ctrl+F1),
its ASCII </> toggle glyphs, its two acdream-authored system messages, and
its Draggable=false grip-only drag model — citing PluginSidePanel.cs and
the RetailUiRuntime.cs TogglePluginManager case. Header count bumped
23->24 active IA rows.
NEW-7 (correctness): the one-time dock wrote Left directly, so the
first-run docked position went unsaved until some later, unrelated event
happened to trigger a save. PluginSidePanel.OnTick now routes the one-time
dock through the retained-window handle's MoveTo when registered (falling
back to the direct field write when unregistered, as before) —
_dockLeft/_dockTop are set to the TARGET position BEFORE calling MoveTo so
the synchronous OnHandleMoved re-entry it triggers (NEW-2's logic) reads the
new dock position and does not flip _userPositioned. New test
FreshShelf_OneTick_SavesTheDockedPositionImmediately confirms the store has
the docked X/Y after exactly one tick and that a subsequent collapse still
preserves the right (not left) edge.
Verification: dotnet build src/AcDream.App (Release) green; targeted filter
(PluginSidePanel|RetailWindowLayout|Markup|UiRootInput|KeyboardConfig) 145
passed / 2 pre-existing Lane=Manual failures (unrelated, gated on
ACDREAM_PROBE_LIVE_MOUNT=1); full tests/AcDream.App.Tests suite: 7306
passed / 97 skipped / 36 failed — same 36 pre-existing failures by name
(installed-DAT/Cathedral/Linux/Lane=Manual probe tests), +3 passed vs the
7303 baseline (exactly the three new tests this round added).
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Two independent Opus reviews of 01b98ca30 (Slice A - movable, collapsible
plugin shelf) found blocking design and behavior issues plus several
should-fix gaps. All addressed in this commit:
1. Grip/toggle are now REAL children instead of a drawn band + a
cursor-position-dependent HandlesClick override + an OnEvent toggle
hack: ShelfGripPanel (WindowMoveHandle=true) spans the top band minus
the toggle width; a UiSimpleButton toggle sits beside it (HandlesClick
already wins at UiRoot.OnMouseDown before the Draggable-window
fallback). The shelf's own Draggable is now FALSE - verified against
UiRoot.FindDragHandleWindow, which never reads a window's own
Draggable flag at all (it walks for a WindowMoveHandle ancestor-or-self
then climbs to the nearest child of UiRoot), so Draggable=true was
never required for the grip to work and only armed the whole-window-
drag fallback for clicks on the shelf's own padding - exactly the
behavior the review asked NOT to have. The two Assert.Single(shelf.
Children) test sites now filter by the (now internal) PluginShelfButton
type instead of asserting child count.
2. Deleted the per-tick "always highest ZOrder" raise in OnTick. It fought
RetailDialogFactory.Tick's own per-frame dialog re-raise, so a dialog
opened while the shelf was visible could never end up on top of it.
Registration's ordinary press-to-raise (a grip press calls
BringToFront before the drag starts) remains.
3. Collapse and hide/show intent are now persisted through their own
channel. RetainedWindowState gained a nullable RequestedVisible;
RetailWindowHandle gained an internal StateChanged event that
RetailWindowLayoutPersistence subscribes to (alongside Moved/Resized/
Shown/Hidden) and that the shelf raises after a collapse toggle or
Show/Hide. Capture() now persists state.RequestedVisible (the panel's
own intent) instead of the derived IsVisible, so an availability hide
(last plugin window unregistered) is never mistaken for a user hide.
WindowNames.PluginShelf is now one of RetailUiRuntime's state-managed
visibility windows, so Apply() restores the intent through
RestoreWindowState directly rather than via Show()/Hide().
4. RetailWindowLayoutPersistence now subscribes to
RetailWindowManager.WindowRegistered/WindowUnregistered so a window
(a plugin window loaded after startup, or the shelf on any path that
constructs persistence first) attaches even when it registers after
persistence already exists.
5. Reflow()'s default height argument is now nullable and falls back to
the last height OnTick actually measured (or unbounded if none yet),
instead of always forcing a single-column layout - every call site
OTHER than OnTick's own row-wrap (Add, unregister, Show/Hide, the
collapse toggle, RestoreWindowState) used to collapse a wrapped
multi-column layout to one column for a frame.
6. _userPositioned is now flipped only when the handle's position differs
from the recorded dock placement, not on every RetailWindowHandle.
Moved (which fires unconditionally on any completed window-drag
release, including a zero-movement grip click, and on any
ClampAllToScreen reachable-clamp).
7. New tests cover: a press+drag starting on an entry button does not
move the shelf; the removed per-tick raise (a sibling with higher
ZOrder keeps it after a tick); TogglePluginManager's hidden-and-
collapsed -> shown-and-expanded / visible -> hidden transitions at the
shelf API (no RetailUiRuntime construction harness exists in this test
suite to exercise the action-routing switch itself - the "no plugin
windows registered" message branch is therefore not covered here).
8. The hide branch of TogglePluginManager now displays "Plugin shelf
hidden. Press Shift+Ctrl+F1 to show it again."; the show branch stays
silent.
9. The collapse toggle now draws ASCII '<'/'>' instead of the DAT-font-
dependent '«'/'»' glyphs (the only use of those code points in the App
UI, silently dropped by UiDatFont when absent), and gets the same
DatFont + bitmap fallback the shelf's entry buttons already have
through UiSimpleButton. A new installed-DAT test pins that the default
font actually carries both ASCII glyphs.
10. ResizeX/ResizeY are false on the shelf so a restored layout's saved
dimensions can never stomp the derived Width/Height via ResizeTo.
11. WindowNames.PluginShelf replaces the "plugin-shelf" literal at every
site (RetailUiRuntime, docs comments, tests).
12. The grip dims to half opacity while RetailWindowManager.IsLocked, the
same visual cue every other retail window gets (the shelf's grip has
no DatElementId, so RetailWindowLockPresentationController's authored-
chrome dimming does not reach it on its own).
Every new test was verified to fail against the pre-fix source: reverting
src/AcDream.App/UI/PluginSidePanel.cs, IRetainedWindowStateController.cs,
RetailWindowHandle.cs, WindowNames.cs, and RetailUiRuntime.cs to their
01b98ca30 state makes the whole PluginSidePanelTests.cs file fail to even
compile (missing WindowNames.PluginShelf, the now-internal
PluginShelfButton type, and RetainedWindowState.RequestedVisible);
reverting RetailWindowLayoutPersistence.cs alone (fixed source elsewhere)
makes WindowRegisteredAfterConstruction_StillRoundTrips fail at runtime
with a null saved layout, confirming finding 4 in isolation.
Verified: dotnet build src/AcDream.App (Release) green; dotnet test
tests/AcDream.App.Tests (Release) targeted filter
(PluginSidePanel|RetailWindowLayout|Markup|UiRootInput) 105/105 green;
full suite 7303 passed / 97 skipped / 36 failed (identical failure set to
the pre-fix-round baseline - installed-DAT live-mount probes, Linux-only
pacing/credential tests, and alpha-flush COUNT-only conformance
divergences, none touching plugin UI; +9 tests, all passing, over the
prior 7294/97/36 baseline).
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Owner request (docs/plans/2026-09-06-plugin-shelf-and-dat-icons.md, Slice A):
the plugin shelf was pinned to the right screen edge every tick, had no drag
or hide affordance, and never persisted position. It is now a registered
retained window ("plugin-shelf") that gets drag, the UI lock, and
RetailWindowLayoutPersistence position/visibility/collapsed persistence for
free, the same way every other retained window does.
Design decisions where the plan left room:
- Grip + collapse toggle are drawn, not child elements. A real child for the
toggle would have to live outside any WindowMoveHandle grip subtree (nesting
it inside lets UiRoot's drag-handle promotion swallow the press before the
button ever sees a click - `handleWindow is not null` outranks
`HandlesClick` in UiRoot.OnMouseDown), and a grip element as a plain sibling
changes UiElement.Children's shape, which the pre-existing single-button
shelf tests assert directly (`Assert.Single(shelf.Children)`). Keeping the
whole shelf Draggable=true and excluding just the toggle's pixel rect from
an overridden HandlesClick (computed live from UiRoot.MouseX/MouseY, the
only call site) gets grip-drags/buttons-and-toggle-don't without adding any
child or touching the existing tests' shape assumptions.
- Availability (has plugin windows) vs the user's requested-visible intent
are split the same way PluginWindowVisibilityController already splits it
for individual plugin windows, but applied SYNCHRONOUSLY (not via
VisibleSource) so Visible updates immediately after Add()/unregister with
no dependency on a Tick ever running - required to keep the pre-Slice-A
unregistered-shelf test (ShelfAndMinimizeButtonsHideAndRestoreWithoutUnregisteringWindow)
green, since it never calls root.Tick().
- "The shelf was moved" (drag or a differing restored layout) is tracked via
the shelf's own RetailWindowHandle.Moved event, captured through
WindowManager.WindowRegistered the moment MountPlugins registers it - so
unregistered/legacy use (the two other pre-existing tests) never sets this
and behaves exactly as before.
- The one-time right-edge dock (no saved layout) fires on the first OnTick
with a real parent width, replacing the old per-tick pin; Reflow's
anchor math then preserves the top-right corner while still docked or the
top-left corner once positioned, on any width change (entry add/remove,
collapse, or a parent-height-driven column rewrap).
Caption finding: the Configure Keyboard row for InputAction.TogglePluginManager
resolves its label live from the installed DAT's action-map string table
(KeyboardConfigController.BuildActionRow, RetailActionMapRow.LabelHash) -
there is no "Plugin Manager" string literal anywhere in our code to rename to
"Plugin Shelf". The row keeps showing retail's own authored name; only the
acdream-side action semantics changed.
Tests added to PluginSidePanelTests.cs (all 7 fail to even compile against
the pre-Slice-A PluginSidePanel, verified by temporarily reverting the source
files and re-running): default right-edge dock; top-right corner preserved
across a Reflow-driven width change while docked; grip drag moves the shelf
and top-left survives the next reflow once positioned; drag refused under
UiLocked; collapse via the real toggle click round-trips through
CaptureWindowState/RestoreWindowState; Show/Hide toggle sequence and a hidden
shelf staying hidden when a new plugin window registers; a full
RetailWindowLayoutPersistence round trip of X/Y/Visible/Collapsed onto a
fresh shelf instance. All 3 pre-existing tests remain green unmodified.
Verified: dotnet build src/AcDream.App (Release) green; the full App test
suite passes 7294/97 skipped/36 pre-existing unrelated failures (identical
failure set confirmed present on HEAD before this change - installed-DAT
live-mount probes, Linux-only pacing/credential tests, and known alpha-flush
COUNT-only conformance divergences, none touching plugin UI).
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Erik