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

fix(ui): Campaign AS AS2 fix round — review findings F13-F18

Browse source 
Opus's dual-lens review of the AS2 header-identity-block commit
(f8a22589) approved the retail port as exact and ordered six
mechanical fixes:

- F13: correct the plan doc's "static tinted preview" claim about
  retail's examine-window paperdoll — the decomp shows retail's clone
  IS animated (BasicCreatureExamineUI::Init @0x004AB9C0 ->
  CPhysicsObj::makeObject -> MorphToExistingObject +
  play_script_internal, CreatureMode::Render driving update_position
  every frame), just decoupled from the live target. AD-114 already
  records the correct deviation; only the plan doc's rationale text
  was wrong.
- F14: add a regression test pinning the monster path (character:
  false, success:false, no String 5 / Int 0x105 marker) so element
  0x1000053A stays cleared — the exact case that used to emit the
  invented "Assessment incomplete" literal AS2 deleted. This is the
  test coverage the AS2 commit message over-claimed already existed.
- F15: dedup the PK/PKLite PWD bit constants that were copy-pasted
  across three files into named PublicWeenieFlags.PlayerKiller
  (0x20) / PlayerKillerLite (0x02000000) values, routing all three
  consumers (AppraisalUiController, CharacterSheetProvider,
  LocalPlayerTeleportController) through them. Pure mechanical
  substitution; the write side (PlayerKillerStatusBitfield.Apply) is
  untouched.
- F16: file register row AD-115 for a real, previously-unrecorded
  deviation the AS2 commit introduced: BuildCharacterTitleDisplay
  clears element 0x10000151 when neither source resolves, where
  retail (CharExamineUI::Show, BasicCreatureExamineUI::Init) never
  clears it and would show the previous target's stale title
  instead. Marked intentional — ours is deliberately better.
- F17: narrow BuildPlayerKillerDisplay's doc comment — retail's
  `cur_weenobj != 0` guard @0x004b482f skips only the PK line, not
  the whole response; the whole-response drop on a missing local
  object is separate, pre-existing, and out of scope here.
- F18: ledger row for AS2 now reads "review fix round" with the land
  commit and a note that the fix SHA is recorded at review-close.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Erik 2026-08-25 08:59:29 +02:00
parent f8a2258979
commit cc5290af7d
7 changed files with 85 additions and 30 deletions
Download patch file Download diff file Expand all files Collapse all files

3
docs/architecture/retail-divergence-register.md
View file

File diff suppressed because one or more lines are too long

16
docs/plans/2026-08-25-assess-window-parity-campaign.md
View file

@ -23,9 +23,17 @@ Missing vs retail:
Explicit owner rulings:
- **The animated 3D paperdoll is an INTENTIONAL acdream deviation** (retail
shows a static tinted preview; its colors are buggy in retail). Keep it.
Register row required (added in the first slice that touches the window).
- **The animated 3D paperdoll is an INTENTIONAL acdream deviation.** Retail's
examine preview clone is NOT a static tinted preview — it is
INDEPENDENTLY ANIMATED, just decoupled from the live target:
`BasicCreatureExamineUI::Init @0x004AB9C0` clones the selected object via
`CPhysicsObj::makeObject @0x005144B0` (which runs `MorphToExistingObject`
then `play_script_internal(setup->default_script_id)`), sets the clone's
heading to 191.367905°, and `CreatureMode::Render @0x004529D0` runs
`update_position` on it every frame — its colors are buggy in retail.
acdream's deviation is that our preview mirrors the target's LIVE motion
instead of playing its own private, decoupled cycle. Keep ours. Register
row required (added in the first slice that touches the window).
- Retail comparison is the oracle for text composition; all strings come
from DAT StringTables per the decomp — **never hardcoded English
literals**.
@ -127,7 +135,7 @@ round → narrow re-review → REVIEW-CLOSED.
| Slice | State | Land / fix commits | Notes |
|---|---|---|---|
| AS1 | **DONE 2026-08-25** | (docs commit) | 3-agent research; ground-truth doc committed |
| AS2 | ready to dispatch | — | |
| AS2 | review fix round | `f8a22589` | fix round (this commit) |
| AS3 | pending AS2 review-close | — | |
| AS4 | pending AS3 review-close | — | |
| AS5 | pending AS4 review-close | — | |

7
src/AcDream.App/Streaming/LocalPlayerTeleportController.cs
View file

@ -262,11 +262,12 @@ internal sealed class RuntimeLocalPlayerLogoutOperations
{
get
{
uint bitfield = _objects.Get(_identity.ServerGuid)
?.PublicWeenieBitfield ?? 0u;
var bitfield = (AcDream.Core.Items.PublicWeenieFlags)(_objects.Get(_identity.ServerGuid)
?.PublicWeenieBitfield ?? 0u);
// IsPlayerKiller @ 0x0058C910: (bitfield & 0x20) | (bitfield &
// 0x2000000) — the PK and PKLite PWD bits.
return (bitfield & 0x20u) != 0u || (bitfield & 0x2000000u) != 0u;
return (bitfield & AcDream.Core.Items.PublicWeenieFlags.PlayerKiller) != 0
|| (bitfield & AcDream.Core.Items.PublicWeenieFlags.PlayerKillerLite) != 0;
}
}

29
src/AcDream.App/UI/Layout/AppraisalUiController.cs
View file

@ -750,6 +750,12 @@ public sealed class AppraisalUiController : IRetainedPanelController
/// Ground truth §2a, gap G2. <see cref="CharacterMarkerIntProperty"/>
/// (0x105 == 261 decimal) is the SAME property <see cref="SelectView"/>
/// already keys the Character-subview dispatch on.
/// <para><b>Deliberate divergence (register row AD-115):</b> when neither
/// source resolves, <see cref="ClearCreatureText"/> has already blanked
/// this element and this method returns empty, so it stays cleared.
/// Retail never clears <c>0x10000151</c> in this situation — it leaves
/// the PREVIOUS target's stale title on screen. Do not "fix" this back
/// toward retail's stale-text behavior without revisiting AD-115.</para>
/// </summary>
private string BuildCharacterTitleDisplay(PropertyBundle p)
{
@ -762,12 +768,6 @@ public sealed class AppraisalUiController : IRetainedPanelController
return GetString(p, TemplateStringProperty);
}
/// <summary>PWD bit 5 — <c>ACCWeenieObject::IsPK @0x0058C8B0</c>.</summary>
private const uint PlayerKillerPwdBit = 0x20u;
/// <summary>PWD bit 0x19 (25) — <c>ACCWeenieObject::IsPKLite @0x0058C8A0</c>.</summary>
private const uint PlayerKillerLitePwdBit = 0x02000000u;
/// <summary>
/// Composes examination element <c>0x10000152</c> (PlayerKiller) for a
/// player target. Ruling R7: retail reads the LOCAL weenie's PWD bits —
@ -775,16 +775,21 @@ public sealed class AppraisalUiController : IRetainedPanelController
/// <paramref name="obj"/> is the SAME <see cref="ClientObject"/>
/// <see cref="BuildTitle"/> already resolves names from — the assessed
/// object <see cref="Apply"/> located before <c>ApplyCreature</c> ever
/// runs. When that lookup fails, <see cref="Apply"/> already bails out
/// before reaching this method, matching retail's "weenie is gone, leave
/// the line cleared" behavior. Ground truth §2a, gap G3.
/// runs. Retail's own guard, <c>if (cur_weenobj != 0)</c>
/// @0x004b482f, is scoped to exactly this line — it skips ONLY the PK
/// line when the weenie is gone, while heritage/title/allegiance/level
/// still update from whatever the response carries. When the local
/// lookup fails, <see cref="Apply"/> bails out before reaching this
/// method (a wider, pre-existing drop of the whole response, out of
/// scope here); for the PK line alone, that cleared result matches
/// retail's guarded skip. Ground truth §2a, gap G3.
/// </summary>
private static string BuildPlayerKillerDisplay(ClientObject obj)
{
uint bitfield = obj.PublicWeenieBitfield ?? 0u;
if ((bitfield & PlayerKillerPwdBit) != 0u)
var bitfield = (PublicWeenieFlags)(obj.PublicWeenieBitfield ?? 0u);
if ((bitfield & PublicWeenieFlags.PlayerKiller) != 0)
return "Player Killer";
if ((bitfield & PlayerKillerLitePwdBit) != 0u)
if ((bitfield & PublicWeenieFlags.PlayerKillerLite) != 0)
return "Player Killer Lite";
return "Non-Player Killer";
}

13
src/AcDream.App/UI/Layout/CharacterSheetProvider.cs
View file

@ -612,22 +612,15 @@ public sealed class CharacterSheetProvider
/// </summary>
private static string? PkStatusText(uint publicWeenieBitfield, Func<string, string?>? resolveUiString)
{
string key = (publicWeenieBitfield & PkPwdBit) != 0u
var bitfield = (PublicWeenieFlags)publicWeenieBitfield;
string key = (bitfield & PublicWeenieFlags.PlayerKiller) != 0
? "ID_StatManagement_Header_PKStatus_PK"
: (publicWeenieBitfield & PkLitePwdBit) != 0u
: (bitfield & PublicWeenieFlags.PlayerKillerLite) != 0
? "ID_StatManagement_Header_PKStatus_PKL"
: "ID_StatManagement_Header_PKStatus_NPK";
return resolveUiString?.Invoke(key);
}
/// <summary>PWD bit 5 — <c>ACCWeenieObject::IsPK @0x0058c8b0</c>:
/// <c>(bitfield &gt;&gt; 5) &amp; 1</c>.</summary>
private const uint PkPwdBit = 0x20u;
/// <summary>PWD bit 0x19 (25) — <c>ACCWeenieObject::IsPKLite @0x0058c8a0</c>:
/// <c>(bitfield &gt;&gt; 0x19) &amp; 1</c>.</summary>
private const uint PkLitePwdBit = 0x02000000u;
/// <summary>Unenchanted base attribute value (Ranks + Start). Used for
/// <see cref="CharacterSheet.AttributeBaseValues"/> — the retail
/// footer-title delta parenthetical compares this against

4
src/AcDream.Core/Items/ItemInteractionPolicy.cs
View file

@ -18,6 +18,8 @@ public enum PublicWeenieFlags : uint
Stuck = 0x00000004,
Player = 0x00000008,
Attackable = 0x00000010,
/// <summary>PWD bit 5 — <c>ACCWeenieObject::IsPK @0x0058C8B0</c>.</summary>
PlayerKiller = 0x00000020,
Vendor = 0x00000200,
PlayerKillerSwitch = 0x00000400,
NonPlayerKillerSwitch = 0x00000800,
@ -33,6 +35,8 @@ public enum PublicWeenieFlags : uint
/// <see cref="VendorSellAcceptability"/>.
/// </summary>
Retained = 0x01000000,
/// <summary>PWD bit 0x19 (25) — <c>ACCWeenieObject::IsPKLite @0x0058C8A0</c>.</summary>
PlayerKillerLite = 0x02000000,
VolatileRare = 0x10000000,
WieldOnUse = 0x20000000,
WieldLeft = 0x40000000,

43
tests/AcDream.App.Tests/UI/Layout/AppraisalUiControllerTests.cs
View file

@ -759,6 +759,49 @@ public sealed class AppraisalUiControllerTests
Assert.Equal(string.Empty, HeaderText(layout, 0x1000053Au));
}
[Fact]
public void FailedMonsterAssess_DoesNotEmitInventedAssessmentIncompleteLiteral()
{
// Regression pin (review F14): pre-AS2, a FAILED monster assess
// (character:false — no String 5 Template, no Int 0x105 marker) was
// the exact case that set 0x1000053A to the invented, zero-retail-
// provenance "Assessment incomplete" literal. The monster branch of
// ApplyCreature never composes 0x1000053A at all (see
// CreatureResponse_HeaderIdentityElementsUnaffectedByPlayerFix
// above), so the element must stay cleared/empty here too, success
// or not.
ImportedLayout layout = FixtureLoader.LoadExamination();
var objects = new ClientObjectTable();
objects.AddOrUpdate(new ClientObject
{
ObjectId = ObjectId,
Name = "Drudge",
Type = ItemType.Creature,
});
using var interaction = NewInteraction(objects, []);
using AppraisalUiController controller = Bind(
layout,
objects,
interaction,
new CombatState(),
[],
[],
() => { },
() => { },
creatureNames: new CreatureDisplayNameResolver(
new Dictionary<uint, string> { [11u] = "Drudge" }))!;
interaction.ExamineSelectedOrEnterMode(ObjectId);
var properties = new PropertyBundle();
properties.Ints[2u] = 11; // CreatureType — no String 5 / Int 261 marker present.
Assert.True(controller.Apply(
Parsed(properties, MinimalCreatureProfile(), success: false)));
Assert.Equal(AppraisalView.Creature, controller.ActiveView);
Assert.Equal(string.Empty, HeaderText(layout, 0x1000053Au));
}
[Fact]
public void ResponseForNeitherPendingNorCurrent_IsIgnored()
{