From 2c00d53db2f4501b40e29a8dfe0e2112943a87b5 Mon Sep 17 00:00:00 2001 From: Erik Date: Thu, 23 Jul 2026 18:25:17 +0200 Subject: [PATCH] fix(net): sign-extend retail capacity bytes Mirror PublicWeenieDesc::UnPack MOVSX behavior so ACE's FF capacity sentinels remain -1 instead of becoming 255. This suppresses non-container capacity prose through the normal retail appraisal checks, with raw-wire and formatter regression coverage. Co-authored-by: Codex --- docs/ISSUES.md | 24 +++++----- ...26-07-23-retail-appraisal-ui-pseudocode.md | 14 +++++- .../research/deepdives/r06-items-inventory.md | 4 +- .../UI/Layout/ItemAppraisalTextFormatter.cs | 8 ++-- src/AcDream.Core.Net/Messages/CreateObject.cs | 19 +++++--- .../Layout/ItemAppraisalTextFormatterTests.cs | 44 ++++++++++++++----- .../Messages/CreateObjectTests.cs | 29 ++++++++++-- 7 files changed, 101 insertions(+), 41 deletions(-) diff --git a/docs/ISSUES.md b/docs/ISSUES.md index ef6e9eac..27502cb3 100644 --- a/docs/ISSUES.md +++ b/docs/ISSUES.md @@ -110,18 +110,18 @@ crafting, ratings, rare state, and prose. AP-110 records the remaining specialized/player-dependent/DAT-name/creature-font/object-preview branches explicitly. -The item-format conformance correction then fixed the live Black Phyntos Hive -counterexample at its source. CreateObject had walked past -`PublicWeenieDesc.HookItemTypes` and `HookType` without retaining them, so the -item formatter could not execute retail `ACCWeenieObject::IsHook`. It also -fell back from a deliberately incomplete appraisal to public Value/Burden and -printed the hook's 255/255 sentinel capacities as ordinary storage. Hook -identity now survives the wire/session/object-table path; Value/Burden use -only the appraisal profile and display `???`/`Unknown` when absent; -`HasHookedItem` suppresses the hook capacity; lock wording and page-count -property order match retail; and each report fragment preserves -`AddItemInfo`'s line/paragraph flag plus LayoutDesc `0x1B`'s white/green/red -font-color indices. +The item-format conformance correction first preserved +`PublicWeenieDesc.HookItemTypes` and `HookType`, stopped falling back from an +incomplete appraisal to public Value/Burden, and ported hook-profile capacity +suppression, lock wording, page-count order, `AddItemInfo` paragraph +boundaries, and the authored white/green/red font list. The connected +Black Phyntos Hive gate then exposed a separate wire-width error: WCID 28249 +is an NPC-lookalike `Creature`, not a hook, and ACE sends its `-1/-1` +capacity sentinels as `FF/FF`. Retail `PublicWeenieDesc::UnPack @ 0x005AD470` +uses `MOVSX` at `0x005AD530` and `0x005AD545`; acdream had zero-extended those +bytes into `255/255`. CreateObject now sign-extends both capacity bytes, so +`Appraisal_ShowCapacity`'s existing positive tests naturally suppress the +sentinels without a name/type workaround. **Files:** `src/AcDream.Core.Net/Messages/AppraiseInfoParser.cs`; `src/AcDream.App/UI/RetailUiRuntime.cs`; diff --git a/docs/research/2026-07-23-retail-appraisal-ui-pseudocode.md b/docs/research/2026-07-23-retail-appraisal-ui-pseudocode.md index 59eeca37..ae252c2e 100644 --- a/docs/research/2026-07-23-retail-appraisal-ui-pseudocode.md +++ b/docs/research/2026-07-23-retail-appraisal-ui-pseudocode.md @@ -363,14 +363,24 @@ IsHook(object): return PublicWeenieDesc.HookType != 0 and PublicWeenieDesc.HookItemTypes != TYPE_UNDEF +UnpackCapacityByte(wireByte): + # PublicWeenieDesc::UnPack uses MOVSX at 0x005AD530 and 0x005AD545. + # The generated pseudocode's uint8_t temporary is misleading. + return sign_extend_int8_to_int32(wireByte) + ShowCapacity(object, profile): if object is not a hook or profile does not contain a HookProfile: append authored item/container capacity as a new paragraph if profile contains PageCount (175) and PageUsed (174): append " of pages full." as a new paragraph - # A hook with a HookProfile describes its mounted item. Its sentinel - # 255/255 PublicWeenieDesc capacities are not player-facing capacity. + # A hook with a HookProfile describes its mounted item, so the hook's own + # capacity is not player-facing capacity. + + # Ordinary creature/NPC-lookalike templates can carry -1/-1 capacity + # sentinels. They travel as FF/FF but UnpackCapacityByte restores -1/-1, + # so the routine's > 0 tests emit no capacity prose. Black Phyntos Hive + # (WCID 28249) is this case: Creature, not Hook. ShowLock(object, profile): if object is a hook: diff --git a/docs/research/deepdives/r06-items-inventory.md b/docs/research/deepdives/r06-items-inventory.md index 74c7a6ba..d20ef655 100644 --- a/docs/research/deepdives/r06-items-inventory.md +++ b/docs/research/deepdives/r06-items-inventory.md @@ -411,8 +411,8 @@ if (Behavior & 0x04000000 IncludesSecondHeader): Per-flag (in exact order): 0x00000001 PluralName : WString16L - 0x00000002 ItemsCapacity : byte - 0x00000004 ContainersCapacity: byte + 0x00000002 ItemsCapacity : int8, sign-extended to int32 + 0x00000004 ContainersCapacity: int8, sign-extended to int32 0x00000100 AmmoType : uint16 0x00000008 Value : uint32 0x00000010 Usable : uint32 (Usable enum) diff --git a/src/AcDream.App/UI/Layout/ItemAppraisalTextFormatter.cs b/src/AcDream.App/UI/Layout/ItemAppraisalTextFormatter.cs index 98df929d..e1d15d0f 100644 --- a/src/AcDream.App/UI/Layout/ItemAppraisalTextFormatter.cs +++ b/src/AcDream.App/UI/Layout/ItemAppraisalTextFormatter.cs @@ -808,9 +808,11 @@ public static class ItemAppraisalTextFormatter AppraiseInfoParser.Parsed appraisal) { PropertyBundle properties = appraisal.Properties; - // Appraisal_ShowCapacity @ 0x004B2680 suppresses the hook object's - // own container capacity while a HookProfile describes the item - // mounted on it. This is the Black Phyntos Hive 255/255 case. + // Appraisal_ShowCapacity @ 0x004B2680 suppresses a hook object's own + // capacity while a HookProfile describes the item mounted on it. + // Non-container sentinels are independently preserved as -1 by + // PublicWeenieDesc::UnPack's signed-byte loads, so they fail the + // positive-capacity tests below exactly as they do in retail. if (!obj.IsHook || appraisal.HookProfile is null) { if (obj.ItemsCapacity > 0 && obj.ContainersCapacity > 0) diff --git a/src/AcDream.Core.Net/Messages/CreateObject.cs b/src/AcDream.Core.Net/Messages/CreateObject.cs index cfafabd5..f36dd25b 100644 --- a/src/AcDream.Core.Net/Messages/CreateObject.cs +++ b/src/AcDream.Core.Net/Messages/CreateObject.cs @@ -833,8 +833,8 @@ public static class CreateObject // --------- ------------------ ----- // 0x04000000 (objDescFlags) weenieFlags2 u32 (skip) // 0x00000001 PluralName String16L CAPTURE - // 0x00000002 ItemsCapacity u8 (skip) - // 0x00000004 ContainersCapacity u8 (skip) + // 0x00000002 ItemsCapacity s8 -> int CAPTURE + // 0x00000004 ContainersCapacity s8 -> int CAPTURE // 0x00000100 AmmoType u16 (skip) // 0x00000008 Value u32 (skip) // 0x00000010 Usable u32 KEPT @@ -905,15 +905,22 @@ public static class CreateObject if ((weenieFlags & 0x00000001u) != 0) // PluralName pluralName = ReadString16L(body, ref pos); - if ((weenieFlags & 0x00000002u) != 0) // ItemsCapacity u8 + if ((weenieFlags & 0x00000002u) != 0) // ItemsCapacity s8 -> int { if (body.Length - pos < 1) throw new FormatException("trunc ItemCap"); - wItemsCapacity = body[pos]; pos += 1; + // PublicWeenieDesc::UnPack @ 0x005AD52C uses MOVSX. + // ACE stores the creature/non-container sentinel -1 as + // 0xFF; retail retains -1 rather than turning it into 255. + wItemsCapacity = unchecked((sbyte)body[pos]); + pos += 1; } - if ((weenieFlags & 0x00000004u) != 0) // ContainersCapacity u8 + if ((weenieFlags & 0x00000004u) != 0) // ContainersCapacity s8 -> int { if (body.Length - pos < 1) throw new FormatException("trunc ContCap"); - wContainersCapacity = body[pos]; pos += 1; + // PublicWeenieDesc::UnPack @ 0x005AD53E likewise uses + // MOVSX before assigning the 32-bit public field. + wContainersCapacity = unchecked((sbyte)body[pos]); + pos += 1; } if ((weenieFlags & 0x00000100u) != 0) // AmmoType u16 { diff --git a/tests/AcDream.App.Tests/UI/Layout/ItemAppraisalTextFormatterTests.cs b/tests/AcDream.App.Tests/UI/Layout/ItemAppraisalTextFormatterTests.cs index 61d2a33f..8267ca3a 100644 --- a/tests/AcDream.App.Tests/UI/Layout/ItemAppraisalTextFormatterTests.cs +++ b/tests/AcDream.App.Tests/UI/Layout/ItemAppraisalTextFormatterTests.cs @@ -88,17 +88,15 @@ public sealed class ItemAppraisalTextFormatterTests } [Fact] - public void HookWithHookProfile_UsesRetailUnknownsAndSuppressesBogusCapacity() + public void NpcLooksLikeObjectCreature_UsesRetailUnknownsAndSuppressesNegativeCapacity() { var obj = new ClientObject { ObjectId = 0x50000005u, Name = "Black Phyntos Hive", - Type = ItemType.Container, - ItemsCapacity = 255, - ContainersCapacity = 255, - HookItemTypes = (uint)ItemType.Container, - HookType = 4u, + Type = ItemType.Creature, + ItemsCapacity = -1, + ContainersCapacity = -1, }; var properties = new PropertyBundle(); properties.Strings[16u] = @@ -106,12 +104,7 @@ public sealed class ItemAppraisalTextFormatterTests ItemAppraisalReport report = ItemAppraisalTextFormatter.BuildReport( obj, - Parsed( - properties, - hook: new AppraiseInfoParser.HookProfile( - Flags: 0u, - ValidLocations: 0u, - AmmoType: 0u)), + Parsed(properties), _ => null); Assert.Equal( @@ -138,6 +131,33 @@ public sealed class ItemAppraisalTextFormatterTests }); } + [Fact] + public void HookWithHookProfile_SuppressesHooksOwnCapacity() + { + var obj = new ClientObject + { + ObjectId = 0x50000015u, + Name = "Weapon Hook", + Type = ItemType.Container, + ItemsCapacity = 24, + ContainersCapacity = 1, + HookItemTypes = (uint)ItemType.Weapon, + HookType = 4u, + }; + + ItemAppraisalReport report = ItemAppraisalTextFormatter.BuildReport( + obj, + Parsed( + new PropertyBundle(), + hook: new AppraiseInfoParser.HookProfile( + Flags: 0u, + ValidLocations: 0u, + AmmoType: 0u)), + _ => null); + + Assert.DoesNotContain("Can hold", report.ToString()); + } + [Fact] public void OrdinaryContainer_PresentsCapacityAsRetailParagraph() { diff --git a/tests/AcDream.Core.Net.Tests/Messages/CreateObjectTests.cs b/tests/AcDream.Core.Net.Tests/Messages/CreateObjectTests.cs index b5bdecdf..c72836d9 100644 --- a/tests/AcDream.Core.Net.Tests/Messages/CreateObjectTests.cs +++ b/tests/AcDream.Core.Net.Tests/Messages/CreateObjectTests.cs @@ -559,6 +559,25 @@ public sealed class CreateObjectTests Assert.Equal(7.5f, p.Workmanship); } + [Fact] + public void TryParse_CapacityBytes_AreSignExtendedLikeRetail() + { + byte[] body = BuildMinimalCreateObjectWithWeenieHeader( + guid: 0x50000029u, + name: "Black Phyntos Hive", + itemType: (uint)ItemType.Creature, + weenieClassId: 28249u, + weenieFlags: 0x00000002u | 0x00000004u, + itemsCapacity: -1, + containersCapacity: -1); + + var parsed = CreateObject.TryParse(body); + + Assert.NotNull(parsed); + Assert.Equal(-1, parsed.Value.ItemsCapacity); + Assert.Equal(-1, parsed.Value.ContainersCapacity); + } + [Fact] public void TryParse_HookIdentityFields_AreCapturedWithoutMisaligningOverlay() { @@ -723,8 +742,8 @@ public sealed class CreateObjectTests ushort? burden = null, uint weenieClassId = 0x1234, uint? maxStackSize = null, - byte? itemsCapacity = null, - byte? containersCapacity = null, + sbyte? itemsCapacity = null, + sbyte? containersCapacity = null, uint? container = null, uint? wielder = null, uint? validLocations = null, @@ -802,8 +821,10 @@ public sealed class CreateObjectTests // its weenieFlags bit is set, matching the parser's walker exactly. // Fields not parameterized above default to 0. if ((weenieFlags & 0x00000001u) != 0) WriteString16L(bytes, pluralName ?? ""); - if ((weenieFlags & 0x00000002u) != 0) bytes.Add(itemsCapacity ?? 0); // ItemsCapacity u8 - if ((weenieFlags & 0x00000004u) != 0) bytes.Add(containersCapacity ?? 0); // ContainersCapacity u8 + if ((weenieFlags & 0x00000002u) != 0) + bytes.Add(unchecked((byte)(itemsCapacity ?? 0))); // ItemsCapacity s8 + if ((weenieFlags & 0x00000004u) != 0) + bytes.Add(unchecked((byte)(containersCapacity ?? 0))); // ContainersCapacity s8 if ((weenieFlags & 0x00000100u) != 0) WriteU16(bytes, ammoType ?? 0); // AmmoType u16 if ((weenieFlags & 0x00000008u) != 0) WriteU32(bytes, value ?? 0u); // Value u32 if ((weenieFlags & 0x00000010u) != 0) WriteU32(bytes, useability ?? 0u); // Usable u32