From f3e95a3ebdff2663e6637c564183f4612bd5384b Mon Sep 17 00:00:00 2001 From: Erik Date: Wed, 29 Jul 2026 01:21:10 +0200 Subject: [PATCH] fix(core): correct DamageType's rotated bits and ItemType's shifted craft ladder Two enums disagreed with the retail client, and both disagreements were the quiet kind - nothing read the wrong members, so nothing was visibly broken. They were traps armed for the first person to write a comparison against them. DamageType had its four drain/restore bits rotated. acdream assigned Nether/Mana/Health/Stamina to 0x80/0x100/0x200/0x400; retail's DAMAGE_TYPE (acclient.h:3788) assigns Health/Stamina/Mana/Nether. The ACE weenie corpus attests retail's order independently - 0x100 Stamina, 0x200 Mana, 0x400 Nether - and so does the vendored client-side enum catalog. Tellingly, both of acdream's live damage-type name tables, CombatChatTranslator.FormatDamageType (ported from holtburger) and ItemAppraisalTextFormatter.TryDamageTypeName, already used retail's order reading the raw wire uint directly. The enum was the only thing in the tree that was wrong. Retail's BASE_DAMAGE_TYPE (0x10000000) was also missing; CombatChatTranslator already knew about it. ItemType had two separate problems. The craft ladder was shifted one bit: CraftAlchemyIntermediate sat on 0x02000000, which retail leaves unused, and an invented CraftCookingIntermediate occupied 0x04000000, which is retail's real alchemy-intermediate bit. The weenie corpus attests 0x04000000 as Craft_Alchemy_Intermediate 235 times and contains no cooking-intermediate at all - there is no such item type. Separately, the composite masks were recomputed locally from the bits above them instead of transcribed, which is exactly how the ladder drifted in the first place. That made Weapon (retail 0x101, melee|missile) an exact alias of WeaponOrCaster (0x8101), and left Item at 0x830F where retail's TYPE_ITEM is 0x2DFBEF - a mask two orders of magnitude broader. The composites are now transcribed as literals with retail's value, not derived, and the five retail-only masks acdream never had (portal/lockable magic targets, the enchantable and redirectable targets, and the two vendor masks) come along. Note for the reader wondering why the campaign trusted retail over the catalog here: on CraftFletchingBase the catalog is the one that is wrong (it says 0x02000000; retail and acdream both say 0x01000000). No single oracle was assumed correct - retail's header decided, with the weenie corpus as the tiebreak. Behavior: no production code reads any changed member. The only reference in the tree is a test that wants a nonzero HookItemTypes and does not care which. So no branch changes and no wire behavior moves - but the values did change, which is why this is a fix commit and not a data commit. No divergence-register row: these were unintentional errors, now retired, not deviations we chose. RetailEnumConformanceTests pins both enums to the acclient.h tables, asserts acdream declares nothing retail does not, and calls out the two specific traps - that 0x02000000 stays unclaimed, and that Weapon and WeaponOrCaster are no longer the same value. Core tests 3,726 -> 3,785. Co-Authored-By: Claude Fable 5 --- src/AcDream.Core/Combat/CombatModel.cs | 21 ++- src/AcDream.Core/Items/ClientObject.cs | 36 +++-- .../Properties/RetailEnumConformanceTests.cs | 152 ++++++++++++++++++ 3 files changed, 196 insertions(+), 13 deletions(-) create mode 100644 tests/AcDream.Core.Tests/Properties/RetailEnumConformanceTests.cs diff --git a/src/AcDream.Core/Combat/CombatModel.cs b/src/AcDream.Core/Combat/CombatModel.cs index 1e752102..2e7b8b5d 100644 --- a/src/AcDream.Core/Combat/CombatModel.cs +++ b/src/AcDream.Core/Combat/CombatModel.cs @@ -204,6 +204,18 @@ public enum AttackType : uint | OffhandDoubleThrust | OffhandTripleThrust, } +/// +/// Verbatim retail DAMAGE_TYPE (docs/research/named-retail/acclient.h:3788). +/// +/// The four drain/restore bits used to be rotated here: Nether/Mana/Health/Stamina +/// were assigned 0x80/0x100/0x200/0x400 where retail assigns Health/Stamina/Mana/Nether. +/// Retail, the vendored client-side enum catalog, and the ACE weenie corpus all agree on +/// retail's order (the corpus attests 0x100=Stamina, 0x200=Mana, 0x400=Nether directly), +/// and both of acdream's live damage-type name tables — CombatChatTranslator and +/// ItemAppraisalTextFormatter — were already reading the retail order off the raw +/// wire uint. Nothing consumed the rotated members, so this corrects a latent trap rather +/// than a live mislabel. +/// [Flags] public enum DamageType : uint { @@ -215,10 +227,11 @@ public enum DamageType : uint Fire = 0x0010, Acid = 0x0020, Electric = 0x0040, - Nether = 0x0080, - Mana = 0x0100, - Health = 0x0200, - Stamina = 0x0400, + Health = 0x0080, + Stamina = 0x0100, + Mana = 0x0200, + Nether = 0x0400, + Base = 0x10000000, } /// diff --git a/src/AcDream.Core/Items/ClientObject.cs b/src/AcDream.Core/Items/ClientObject.cs index 8cb5f0d4..fc8b5a6d 100644 --- a/src/AcDream.Core/Items/ClientObject.cs +++ b/src/AcDream.Core/Items/ClientObject.cs @@ -10,9 +10,17 @@ namespace AcDream.Core.Items; /// /// AC's ItemType is a 32-bit flags enum — a single dat weenie can -/// assert multiple type bits. From ACE.Entity.Enum.ItemType -/// cross-checked against the decompile paperdoll tooltip dispatcher. -/// Full bit list in the research doc §1. +/// assert multiple type bits. Verbatim retail ITEM_TYPE +/// (docs/research/named-retail/acclient.h:3300). +/// +/// The craft ladder used to be shifted: CraftAlchemyIntermediate sat on +/// 0x02000000 (retail leaves that bit unused and puts alchemy-intermediate on +/// 0x04000000), and an invented CraftCookingIntermediate occupied the real +/// alchemy-intermediate bit. The ACE weenie corpus attests 0x04000000 = +/// Craft_Alchemy_Intermediate 235 times and contains no cooking-intermediate at all. +/// The composite masks were likewise recomputed locally rather than transcribed, which +/// made Weapon an alias of WeaponOrCaster and left Item two orders +/// of magnitude narrower than retail's. Full bit list in the research doc §1. /// [Flags] public enum ItemType : uint @@ -43,17 +51,27 @@ public enum ItemType : uint CraftCookingBase = 0x00400000, CraftAlchemyBase = 0x00800000, CraftFletchingBase = 0x01000000, - CraftAlchemyIntermediate= 0x02000000, - CraftCookingIntermediate= 0x04000000, + // 0x02000000 is deliberately unused in retail's ladder. + CraftAlchemyIntermediate= 0x04000000, CraftFletchingIntermediate = 0x08000000, LifeStone = 0x10000000, TinkeringTool = 0x20000000, TinkeringMaterial = 0x40000000, Gameboard = 0x80000000u, - Vestements = Armor | Clothing, - Weapon = MeleeWeapon | MissileWeapon | Caster, - WeaponOrCaster = Weapon, - Item = Weapon | Armor | Clothing | Jewelry | Container, + + // Composite masks, transcribed verbatim rather than recomputed from the bits + // above — retail's TYPE_ITEM in particular is far broader than any obvious + // union, and deriving these locally is how the craft ladder drifted. + Vestements = 0x00000006, // TYPE_VESTEMENTS + Weapon = 0x00000101, // TYPE_WEAPON: melee | missile, no caster + WeaponOrCaster = 0x00008101, // TYPE_WEAPON_OR_CASTER + LockableMagicTarget = 0x00000280, // TYPE_LOCKABLE_MAGIC_TARGET + RedirectableItemEnchantmentTarget = 0x00008107, + PortalMagicTarget = 0x10010000, // TYPE_PORTAL_MAGIC_TARGET + ItemEnchantableTarget = 0x00088B8F, // TYPE_ITEM_ENCHANTABLE_TARGET + Item = 0x002DFBEF, // TYPE_ITEM + VendorShopkeep = 0x480467A7, // TYPE_VENDOR_SHOPKEEP + VendorGrocer = 0x00446220, // TYPE_VENDOR_GROCER } /// diff --git a/tests/AcDream.Core.Tests/Properties/RetailEnumConformanceTests.cs b/tests/AcDream.Core.Tests/Properties/RetailEnumConformanceTests.cs new file mode 100644 index 00000000..f5641e9d --- /dev/null +++ b/tests/AcDream.Core.Tests/Properties/RetailEnumConformanceTests.cs @@ -0,0 +1,152 @@ +using System; +using System.Linq; +using AcDream.Core.Combat; +using AcDream.Core.Items; +using Xunit; + +namespace AcDream.Core.Tests.Properties; + +/// +/// Pins the wire-adjacent enums acdream shares with the retail client to the values +/// in the Sept 2013 EoR header, docs/research/named-retail/acclient.h. Each +/// table below is transcribed from the cited enum block, so a hand edit that +/// drifts from retail fails here. +/// +/// These tables are the reason the 2026-07-29 enum campaign found real bugs: +/// had its four drain/restore bits rotated, and +/// had a shifted craft ladder plus locally recomputed composite +/// masks. Both are corrected and pinned below. +/// +public sealed class RetailEnumConformanceTests +{ + /// acclient.h:3788, enum DAMAGE_TYPE. FORCE_*_32_BIT omitted. + public static TheoryData RetailDamageType => new() + { + { "Undef", 0x0 }, + { "Slash", 0x1 }, + { "Pierce", 0x2 }, + { "Bludgeon", 0x4 }, + { "Cold", 0x8 }, + { "Fire", 0x10 }, + { "Acid", 0x20 }, + { "Electric", 0x40 }, + { "Health", 0x80 }, + { "Stamina", 0x100 }, + { "Mana", 0x200 }, + { "Nether", 0x400 }, + { "Base", 0x10000000 }, + }; + + [Theory] + [MemberData(nameof(RetailDamageType))] + public void DamageTypeMatchesRetail(string name, uint value) + { + Assert.True(Enum.IsDefined(typeof(DamageType), name), + $"DamageType.{name} is missing"); + Assert.Equal(value, (uint)Enum.Parse(name)); + } + + [Fact] + public void DamageTypeDeclaresNothingRetailDoesNot() + { + var expected = RetailDamageType.Select(r => (string)r[0]).OrderBy(n => n, StringComparer.Ordinal); + var actual = Enum.GetNames().OrderBy(n => n, StringComparer.Ordinal); + Assert.Equal(expected, actual); + } + + /// + /// acclient.h:3300, enum ITEM_TYPE. TYPE_UNDEF and TYPE_SELF both sit on 0 + /// in retail; acdream spells that single zero None. FORCE_*_32_BIT omitted. + /// + public static TheoryData RetailItemType => new() + { + { "None", 0x0 }, + { "MeleeWeapon", 0x1 }, + { "Armor", 0x2 }, + { "Vestements", 0x6 }, + { "Clothing", 0x4 }, + { "Jewelry", 0x8 }, + { "Creature", 0x10 }, + { "Food", 0x20 }, + { "Money", 0x40 }, + { "Misc", 0x80 }, + { "MissileWeapon", 0x100 }, + { "Weapon", 0x101 }, + { "Container", 0x200 }, + { "LockableMagicTarget", 0x280 }, + { "Useless", 0x400 }, + { "Gem", 0x800 }, + { "SpellComponents", 0x1000 }, + { "Writable", 0x2000 }, + { "Key", 0x4000 }, + { "Caster", 0x8000 }, + { "WeaponOrCaster", 0x8101 }, + { "RedirectableItemEnchantmentTarget", 0x8107 }, + { "Portal", 0x10000 }, + { "Lockable", 0x20000 }, + { "PromissoryNote", 0x40000 }, + { "ManaStone", 0x80000 }, + { "ItemEnchantableTarget", 0x88B8F }, + { "Service", 0x100000 }, + { "MagicWieldable", 0x200000 }, + { "Item", 0x2DFBEF }, + { "CraftCookingBase", 0x400000 }, + { "VendorGrocer", 0x446220 }, + { "CraftAlchemyBase", 0x800000 }, + { "CraftFletchingBase", 0x1000000 }, + { "CraftAlchemyIntermediate", 0x4000000 }, + { "CraftFletchingIntermediate", 0x8000000 }, + { "LifeStone", 0x10000000 }, + { "PortalMagicTarget", 0x10010000 }, + { "TinkeringTool", 0x20000000 }, + { "TinkeringMaterial", 0x40000000 }, + { "VendorShopkeep", 0x480467A7 }, + { "Gameboard", 0x80000000 }, + }; + + [Theory] + [MemberData(nameof(RetailItemType))] + public void ItemTypeMatchesRetail(string name, uint value) + { + Assert.True(Enum.IsDefined(typeof(ItemType), name), + $"ItemType.{name} is missing"); + Assert.Equal(value, (uint)Enum.Parse(name)); + } + + [Fact] + public void ItemTypeDeclaresNothingRetailDoesNot() + { + var expected = RetailItemType.Select(r => (string)r[0]).OrderBy(n => n, StringComparer.Ordinal); + var actual = Enum.GetNames().OrderBy(n => n, StringComparer.Ordinal); + Assert.Equal(expected, actual); + } + + /// + /// The craft ladder specifically: retail leaves 0x02000000 unused, and the ACE + /// weenie corpus attests 0x04000000 as Craft_Alchemy_Intermediate 235 times. This + /// is the pairing acdream had wrong. + /// + [Fact] + public void CraftLadderMatchesRetailAndLeavesTheUnusedBitUnclaimed() + { + Assert.Equal(0x00400000u, (uint)ItemType.CraftCookingBase); + Assert.Equal(0x00800000u, (uint)ItemType.CraftAlchemyBase); + Assert.Equal(0x01000000u, (uint)ItemType.CraftFletchingBase); + Assert.Equal(0x04000000u, (uint)ItemType.CraftAlchemyIntermediate); + Assert.Equal(0x08000000u, (uint)ItemType.CraftFletchingIntermediate); + Assert.DoesNotContain(Enum.GetValues(), t => (uint)t == 0x02000000u); + } + + /// + /// Weapon and WeaponOrCaster were aliases before the campaign, because + /// Weapon was recomputed locally as melee|missile|caster. Retail keeps them + /// distinct and only the latter includes the caster bit. + /// + [Fact] + public void WeaponExcludesCasterAndWeaponOrCasterIncludesIt() + { + Assert.NotEqual(ItemType.Weapon, ItemType.WeaponOrCaster); + Assert.Equal(ItemType.MeleeWeapon | ItemType.MissileWeapon, ItemType.Weapon); + Assert.Equal(ItemType.Weapon | ItemType.Caster, ItemType.WeaponOrCaster); + } +}