diff --git a/docs/ISSUES.md b/docs/ISSUES.md index a9c38a4c..a2d72701 100644 --- a/docs/ISSUES.md +++ b/docs/ISSUES.md @@ -92,6 +92,105 @@ Copy this block when adding a new issue: --- +## #267 — Vitae does not update the character panel's skills/attributes display + +**Status:** DONE (implementation) — 2026-07-30; visual confirmation with the user pending +**Severity:** MEDIUM +**Filed:** 2026-07-30 +**Component:** retained UI / character window / vitae + +**Symptom (user report):** with 5% vitae active, the skills and attributes +values in the character window did not change; retail shows the CURRENT +(vitae-reduced) value and, on click/detail, the current level with the +vitae reduction in parentheses (e.g. "(-100)"). The P1 movement chain +consumes vitae correctly (EnchantSkill for run/jump); the character +window presentation did not. + +**Scoping (2026-07-30):** confirmed — `CharacterSheetProvider`/ +`CharacterStatController` contained zero vitae/enchantment references; the +panel rendered base property values only. `LocalPlayerState.AttributeSnapshot.Current` +(Ranks+Start) and `SkillSnapshot.CurrentLevel` (an alias of `BaseLevel`) never +consulted the wired `Spellbook`. + +**Root cause / resolution:** Ported retail's three enchantment-composition +functions from the named decomp: +`CEnchantmentRegistry::EnchantAttribute` (0x00594570 — primary attributes; +mult/add buff lists only, floor at 1/10 by base<10, truncate), +`CEnchantmentRegistry::EnchantAttribute2nd` (0x00594670 — vitals, already +ported for issue #6), and `CEnchantmentRegistry::EnchantSkill` (0x005947b0 — +skills; vitae singleton applied first, then mult/add lists, zero-floor at +<0.5, truncate). **Primary attributes are vitae-immune in retail** — +`EnchantAttribute` never references the `_vitae` singleton, unlike its +Attribute2nd/Skill siblings; this is confirmed directly from the decompiled +function bodies, not assumed. +`EnchantmentMath.GetMod` gained a `requiredType` (`EnchantmentTypeFlag`: +Attribute=1, SecondAtt=2, Skill=0x10, matching ACE's +`EnchantmentTypeFlags`) and `includeVitae` parameter so a numeric key +collision across domains (e.g. key=1 is both Strength and MaxHealth) can't +leak a buff across domains; both default to the prior no-filter/vitae-always +behavior so the existing vitals path (#6) is untouched. +`Spellbook.GetAttributeMod`/`GetSkillMod` are new cached accessors mirroring +`GetVitalMod`. `LocalPlayerState.GetEffectiveAttribute`/`GetEffectiveSkill`/ +`GetSkillVitaeModifier` compose the final int, falling back to the +unenchanted base when no spellbook is wired. +`CharacterSheetProvider.BuildSheet()` now reports the effective (buffed) +value as the panel's main number for all 6 attributes and every skill's +`CharacterSkill.CurrentLevel` (previously an alias of `BaseLevel` — this also +activates the pre-existing but previously-dead +`CharacterStatController.SkillValueColor` buffed/debuffed row coloring). +`CharacterSheet.AttributeBaseValues` and `CharacterSkill.VitaeModifier` carry +the extra state the footer-title delta needs. + +The click/detail parenthetical format is cited from +`gmAttributeUI::DisplaySelectionFooter_Attribute` (0x0049d280, format +`"%s: %d"` + `" (%s%d)"`) and `gmSkillUI::DisplaySelectionFooter_Trained` +(0x0049b860, plus `SkillInfoRegion::GetVitaeModifier` 0x004f0fa0 for the +isolated vitae segment `" (%d)"`). Retail skills show up to TWO +parentheticals — vitae's own contribution (always ≤0, e.g. "(-100)") and the +buff-only residual excluding vitae — while attributes (vitae-immune) show at +most one. No parenthetical appears when the relevant delta is zero; an +increase gets an explicit "+" the natural `%d` doesn't provide, a decrease +uses the natural minus sign. Retail colors each segment independently via +`AppendTextWithFont` state 1/2/3; the current `UiText` footer-title element is +`OneLine` (single color, single run), so segments share the title's white +color rather than retail's per-run tint — a presentation simplification, not +a value/format deviation. + +Live refresh: `CharacterSheetProvider`'s existing `SubscribeChanged` binding +now also subscribes to `LocalPlayerState.Spellbook.EnchantmentsChanged` (a +newly-exposed read-only property mirroring the private reference +`LocalPlayerState` already held for issue #6), so the panel repaints when +vitae is applied/removed or a buff is cast/expires, not only on raw +property/attribute updates. + +**Files:** `src/AcDream.Core/Spells/EnchantmentMath.cs`; +`src/AcDream.Core/Spells/Spellbook.cs`; +`src/AcDream.Core/Player/LocalPlayerState.cs`; +`src/AcDream.App/UI/Layout/CharacterSheet.cs`; +`src/AcDream.App/UI/Layout/CharacterSheetProvider.cs`; +`src/AcDream.App/UI/Layout/CharacterStatController.cs`. + +**Research:** `docs/research/named-retail/acclient_2013_pseudo_c.txt` lines +416110 (`EnchantAttribute`), 416169 (`EnchantAttribute2nd`), 416240 +(`EnchantSkill`), 167258 (`gmSkillUI::DisplaySelectionFooter_Trained`), +168660 (`gmAttributeUI::DisplaySelectionFooter_Attribute`), 244033 +(`SkillInfoRegion::GetVitaeModifier`); +`references/ACE/Source/ACE.Entity/Enum/EnchantmentTypeFlags.cs`. + +**Acceptance:** Core golden tests cover 33% vitae on a known skill base +(303→203, matching the user's exact "(-100)" example), buff+vitae +composition, zero-delta no-parenthetical, and the attribute vitae-immunity +finding. `CharacterStatControllerTests` covers the full row-click → +footer-title text through `CharacterStatController.Bind` for both domains. +`CharacterSheetProviderTests` covers `BuildSheet()` effective-value output +and `SubscribeChanged` firing (and rebuilding to the new value) on +`EnchantmentsChanged`. `dotnet build AcDream.slnx -c Release` and the +complete solution test suite pass with zero failures. **Final closure pends +the user's live visual check** (5%/33% vitae against a live ACE session) per +the standard M4 UI-accuracy gate. + +--- + ## #247 — Loot ordering and local dropped-item projection regressed **Status:** DONE — 2026-07-26; connected loot/drop/selection gate passed diff --git a/src/AcDream.App/UI/Layout/CharacterSheet.cs b/src/AcDream.App/UI/Layout/CharacterSheet.cs index 2da63597..58711305 100644 --- a/src/AcDream.App/UI/Layout/CharacterSheet.cs +++ b/src/AcDream.App/UI/Layout/CharacterSheet.cs @@ -89,6 +89,10 @@ public sealed class CharacterSheet // ── Innate attributes (UpdateInnateAttributeInfo 0x004b87e0) ──────────── // InqAttribute order: 1,2,4,3,5,6 = Strength, Endurance, Quickness, Coordination, Focus, Self. + // Issue #267: these are the EFFECTIVE (post-buff) values — retail + // CACQualities::EnchantAttribute (0x00594570). Primary attributes are + // vitae-immune in retail; only active buffs move this number away from + // AttributeBaseValues below. public int Strength { get; init; } public int Endurance { get; init; } public int Quickness { get; init; } @@ -96,6 +100,17 @@ public sealed class CharacterSheet public int Focus { get; init; } public int Self { get; init; } + /// + /// Unenchanted base value for each of the 6 primary attributes, in + /// POSITIONAL order matching CharacterStatController.AttrRows + /// (Strength, Endurance, Coordination, Quickness, Focus, Self — note + /// this differs from the individual-property declaration order above). + /// Issue #267: paired with the effective values above to compute the + /// retail footer-title delta parenthetical + /// (gmAttributeUI::DisplaySelectionFooter_Attribute 0x0049d280). + /// + public int[] AttributeBaseValues { get; init; } = Array.Empty(); + // ── Skills (UpdateFakeSkills 0x004b8930) ──────────────────────────────── // Character Information uses 0xB5/0xC0 for Chess/Fishing; skill credits use 0x18. // InqInt(0x18) = available skill credits — footer 0x10000245 in the Attributes tab. @@ -187,9 +202,18 @@ public sealed record CharacterSkill( uint IconDid, CharacterSkillAdvancementClass AdvancementClass, int BaseLevel, + // Issue #267: CurrentLevel is now the EFFECTIVE (vitae + buff) level — + // retail CACQualities::EnchantSkill (0x005947b0). Previously an alias of + // BaseLevel; this activates the existing CharacterStatController. + // SkillValueColor buffed/debuffed row coloring. int CurrentLevel, bool UsableUntrained, int TrainedCost, int SpecializedCost, long RaiseCost, - long Raise10Cost = 0L); + long Raise10Cost = 0L, + // Issue #267: vitae's isolated contribution to CurrentLevel (always ≤ 0), + // retail SkillInfoRegion::GetVitaeModifier (0x004f0fa0). Used for the + // footer-title vitae-specific parenthetical, separate from the buff delta + // (CurrentLevel − VitaeModifier − BaseLevel). + int VitaeModifier = 0); diff --git a/src/AcDream.App/UI/Layout/CharacterSheetProvider.cs b/src/AcDream.App/UI/Layout/CharacterSheetProvider.cs index dc390f97..51ac37ef 100644 --- a/src/AcDream.App/UI/Layout/CharacterSheetProvider.cs +++ b/src/AcDream.App/UI/Layout/CharacterSheetProvider.cs @@ -141,12 +141,16 @@ public sealed class CharacterSheetProvider ManaCurrent = VitalCurrent(LocalPlayerState.VitalKind.Mana), ManaMax = VitalMax(LocalPlayerState.VitalKind.Mana), - Strength = AttrCurrent(LocalPlayerState.AttributeKind.Strength), - Endurance = AttrCurrent(LocalPlayerState.AttributeKind.Endurance), - Coordination = AttrCurrent(LocalPlayerState.AttributeKind.Coordination), - Quickness = AttrCurrent(LocalPlayerState.AttributeKind.Quickness), - Focus = AttrCurrent(LocalPlayerState.AttributeKind.Focus), - Self = AttrCurrent(LocalPlayerState.AttributeKind.Self), + // Issue #267: the panel's main attribute values are EFFECTIVE + // (post-buff) — retail CACQualities::EnchantAttribute. Base values + // for the footer-title delta parenthetical live in + // AttributeBaseValues below, in the same positional order. + Strength = AttrEffective(LocalPlayerState.AttributeKind.Strength), + Endurance = AttrEffective(LocalPlayerState.AttributeKind.Endurance), + Coordination = AttrEffective(LocalPlayerState.AttributeKind.Coordination), + Quickness = AttrEffective(LocalPlayerState.AttributeKind.Quickness), + Focus = AttrEffective(LocalPlayerState.AttributeKind.Focus), + Self = AttrEffective(LocalPlayerState.AttributeKind.Self), UnspentSkillCredits = skillCredits, SpecializedSkillCredits = 0, @@ -163,6 +167,15 @@ public sealed class CharacterSheetProvider UnassignedXp = unassignedXp, AttributeRaiseCosts = BuildAttributeRaiseCosts(amount: 1), AttributeRaise10Costs = BuildAttributeRaiseCosts(amount: 10), + AttributeBaseValues = new[] + { + AttrCurrent(LocalPlayerState.AttributeKind.Strength), + AttrCurrent(LocalPlayerState.AttributeKind.Endurance), + AttrCurrent(LocalPlayerState.AttributeKind.Coordination), + AttrCurrent(LocalPlayerState.AttributeKind.Quickness), + AttrCurrent(LocalPlayerState.AttributeKind.Focus), + AttrCurrent(LocalPlayerState.AttributeKind.Self), + }, Skills = BuildLiveCharacterSkills(), BurdenCurrent = props.GetInt(5u), BurdenMax = props.GetInt(96u), @@ -207,6 +220,12 @@ public sealed class CharacterSheetProvider owner._objects.Cleared += OnCleared; owner._localPlayer.AttributeChanged += OnAttributeChanged; owner._localPlayer.CharacterChanged += OnCharacterChanged; + // Issue #267: skills/attributes are now vitae + buff aware, so the + // sheet must refresh whenever the active-enchantment set changes + // (vitae applied/removed on death/lifestone, a buff cast/expired), + // not only on raw property/attribute updates. + if (owner._localPlayer.Spellbook is { } spellbook) + spellbook.EnchantmentsChanged += OnCleared; } private void OnObjectChanged(ClientObject value) @@ -235,6 +254,8 @@ public sealed class CharacterSheetProvider owner._objects.Cleared -= OnCleared; owner._localPlayer.AttributeChanged -= OnAttributeChanged; owner._localPlayer.CharacterChanged -= OnCharacterChanged; + if (owner._localPlayer.Spellbook is { } spellbook) + spellbook.EnchantmentsChanged -= OnCleared; } } @@ -349,18 +370,27 @@ public sealed class CharacterSheetProvider long raiseCost = SkillRaiseCost(xp, advancement, snapshot, 1); long raise10Cost = SkillRaiseCost(xp, advancement, snapshot, 10); + // Issue #267: CurrentLevel is the EFFECTIVE (vitae + buff) level — + // retail CACQualities::EnchantSkill (0x005947b0). VitaeModifier + // isolates vitae's own contribution for the footer's separate + // vitae parenthetical (SkillInfoRegion::GetVitaeModifier 0x004f0fa0). + int effectiveLevel = _localPlayer.GetEffectiveSkill(snapshot.SkillId) + ?? checked((int)Math.Min(int.MaxValue, snapshot.CurrentLevel)); + int vitaeModifier = _localPlayer.GetSkillVitaeModifier(snapshot.SkillId); + result.Add(new CharacterSkill( snapshot.SkillId, name, icon, advancement, checked((int)Math.Min(int.MaxValue, snapshot.BaseLevel)), - checked((int)Math.Min(int.MaxValue, snapshot.CurrentLevel)), + effectiveLevel, IsUsableUntrained(snapshot.SkillId), trainedCost, specializedCost, raiseCost, - raise10Cost)); + raise10Cost, + vitaeModifier)); } return result; @@ -418,9 +448,19 @@ public sealed class CharacterSheetProvider _ => null, }; + /// Unenchanted base attribute value (Ranks + Start). Used for + /// — the retail + /// footer-title delta parenthetical compares this against + /// . private int AttrCurrent(LocalPlayerState.AttributeKind kind) => _localPlayer.GetAttribute(kind) is { } attr ? checked((int)Math.Min(int.MaxValue, attr.Current)) : 0; + /// Issue #267 — effective (post-buff) attribute value shown as + /// the panel's main number. Retail primary attributes are vitae-immune; + /// see . + private int AttrEffective(LocalPlayerState.AttributeKind kind) => + _localPlayer.GetEffectiveAttribute(kind) ?? 0; + private int VitalCurrent(LocalPlayerState.VitalKind kind) => _localPlayer.Get(kind) is { } vital ? checked((int)Math.Min(int.MaxValue, vital.Current)) : 0; diff --git a/src/AcDream.App/UI/Layout/CharacterStatController.cs b/src/AcDream.App/UI/Layout/CharacterStatController.cs index 4ac8f737..fa60c650 100644 --- a/src/AcDream.App/UI/Layout/CharacterStatController.cs +++ b/src/AcDream.App/UI/Layout/CharacterStatController.cs @@ -1219,6 +1219,102 @@ public static class CharacterStatController }; } + // ── Issue #267 — retail vitae/buff delta parenthetical ─────────────────── + // Format cited from gmAttributeUI::DisplaySelectionFooter_Attribute + // (0x0049d280, format " (%s%d)") and gmSkillUI::DisplaySelectionFooter_Trained + // (0x0049b860, vitae segment format " (%d)" via SkillInfoRegion:: + // GetVitaeModifier 0x004f0fa0, buff segment " (%s%d)"). Retail colors each + // segment independently (AppendTextWithFont state 1/2/3); our UiText footer + // title element renders OneLine/single-color, so segments here share the + // title's white color rather than retail's per-run tint — a presentation + // simplification, not a value/format deviation. + + /// Effective value for attribute/vital row + /// as an int (numeric twin of for the 6 + /// primary-attribute rows only — vitals use "cur/max" and have no single + /// effective int). + private static int GetRowEffectiveAttributeValue(CharacterSheet sheet, int index) => index switch + { + 0 => sheet.Strength, + 1 => sheet.Endurance, + 2 => sheet.Coordination, + 3 => sheet.Quickness, + 4 => sheet.Focus, + 5 => sheet.Self, + _ => 0, + }; + + /// Buff-only delta (effective − base) for the primary-attribute + /// row at . Zero for vital rows (6-8) — vitals use + /// a separate "cur/max" footer format in retail + /// (gmAttributeUI::DisplaySelectionFooter_Vital 0x0049d6b0), not + /// this delta pattern. + internal static int GetAttributeDelta(CharacterSheet sheet, int index) + { + if ((uint)index >= (uint)AttrRows.Length) return 0; + int[] baseValues = sheet.AttributeBaseValues; + if (baseValues is null || index >= baseValues.Length) return 0; + return GetRowEffectiveAttributeValue(sheet, index) - baseValues[index]; + } + + /// Skill's buff-only delta, excluding vitae — retail computes + /// this as (current − vitaeModifier) − base + /// (gmSkillUI::DisplaySelectionFooter_Trained 0x0049b860): vitae's + /// own contribution is reported separately via + /// . + internal static int GetSkillBuffOnlyDelta(CharacterSkill skill) => + (skill.CurrentLevel - skill.VitaeModifier) - skill.BaseLevel; + + /// Retail " (%s%d)" buff-delta parenthetical: an explicit "+" + /// prefix on an increase (the natural %d has no leading plus), the + /// natural minus sign on a decrease (no prefix string set in that + /// branch), nothing when the delta is zero. + private static string FormatBuffDelta(int delta) => delta switch + { + 0 => string.Empty, + > 0 => $" (+{delta})", + _ => $" ({delta})", + }; + + /// Retail " (%d)" vitae-specific parenthetical + /// (SkillInfoRegion::GetVitaeModifier 0x004f0fa0): shown only + /// while vitae is an active penalty (modifier < 0) — vitae never + /// grants a bonus, so no "+" case exists. + private static string FormatVitaeDelta(int vitaeModifier) => + vitaeModifier < 0 ? $" ({vitaeModifier})" : string.Empty; + + /// Build the retail footer-title text for the current selection: + /// "{Name}: {value}" with the vitae + buff-delta parentheticals appended + /// for a selected trained/specialized skill or a selected attribute row. + /// Shared by both footer-state bindings (State A/B use physically + /// separate UiText elements in the imported layout) so the composition + /// logic lives in exactly one place. + private static string BuildSelectedTitleText( + CharacterStatTab tab, + Func data, + int[] attrSel, + int[] skillSel) + { + if (tab == CharacterStatTab.Skills) + { + CharacterSkill? skill = SkillAtDisplayIndex(data(), skillSel[0]); + if (skill is null) return "Select a Skill to Improve"; + if (skill.AdvancementClass < CharacterSkillAdvancementClass.Trained) + return skill.Name; + + string vitaeSuffix = FormatVitaeDelta(skill.VitaeModifier); + string buffSuffix = FormatBuffDelta(GetSkillBuffOnlyDelta(skill)); + return $"{skill.Name}: {skill.CurrentLevel}{vitaeSuffix}{buffSuffix}"; + } + + if (attrSel[0] < 0) return "Select an Attribute to Improve"; + CharacterSheet sheet = data(); + string name = GetRowName(attrSel[0]); + string value = GetRowValueString(sheet, attrSel[0]); + string delta = FormatBuffDelta(GetAttributeDelta(sheet, attrSel[0])); + return $"{name}: {value}{delta}"; + } + /// /// Add a single attribute/vital row to as a /// containing icon + name + value children. @@ -1418,25 +1514,13 @@ public static class CharacterStatController titleEl.ClickThrough = true; titleEl.LinesProvider = () => { - if (activeTab[0] == CharacterStatTab.Skills) - { - var skill = SkillAtDisplayIndex(data(), skillSel[0]); - if (skill is null) - return new[] { new UiText.Line("Select a Skill to Improve", Body) }; - - string title = skill.AdvancementClass >= CharacterSkillAdvancementClass.Trained - ? $"{skill.Name}: {skill.CurrentLevel}" - : skill.Name; - return new[] { new UiText.Line(title, Vector4.One) }; - } - - if (attrSel[0] < 0) - return new[] { new UiText.Line("Select an Attribute to Improve", Body) }; - var s = data(); - string name = GetRowName(attrSel[0]); - string value = GetRowValueString(s, attrSel[0]); - // State B title is WHITE (retail confirmed). - return new[] { new UiText.Line($"{name}: {value}", Vector4.One) }; + string title = BuildSelectedTitleText(activeTab[0], data, attrSel, skillSel); + bool nothingSelected = activeTab[0] == CharacterStatTab.Skills + ? SkillAtDisplayIndex(data(), skillSel[0]) is null + : attrSel[0] < 0; + // State B title is WHITE (retail confirmed); the "nothing + // selected" prompt keeps the dimmer Body color. + return new[] { new UiText.Line(title, nothingSelected ? Body : Vector4.One) }; }; } @@ -1518,24 +1602,11 @@ public static class CharacterStatController title.ClickThrough = true; title.LinesProvider = () => { - if (activeTab[0] == CharacterStatTab.Skills) - { - var skill = SkillAtDisplayIndex(data(), skillSel[0]); - if (skill is null) - return new[] { new UiText.Line("Select a Skill to Improve", Body) }; - - string skillTitle = skill.AdvancementClass >= CharacterSkillAdvancementClass.Trained - ? $"{skill.Name}: {skill.CurrentLevel}" - : skill.Name; - return new[] { new UiText.Line(skillTitle, Vector4.One) }; - } - - if (attrSel[0] < 0) - return new[] { new UiText.Line("Select an Attribute to Improve", Body) }; - var sheet = data(); - string name = GetRowName(attrSel[0]); - string value = GetRowValueString(sheet, attrSel[0]); - return new[] { new UiText.Line($"{name}: {value}", Vector4.One) }; + string titleText = BuildSelectedTitleText(activeTab[0], data, attrSel, skillSel); + bool nothingSelected = activeTab[0] == CharacterStatTab.Skills + ? SkillAtDisplayIndex(data(), skillSel[0]) is null + : attrSel[0] < 0; + return new[] { new UiText.Line(titleText, nothingSelected ? Body : Vector4.One) }; }; } diff --git a/src/AcDream.Core/Player/LocalPlayerState.cs b/src/AcDream.Core/Player/LocalPlayerState.cs index 89fd8480..51e574d6 100644 --- a/src/AcDream.Core/Player/LocalPlayerState.cs +++ b/src/AcDream.Core/Player/LocalPlayerState.cs @@ -181,6 +181,76 @@ public sealed class LocalPlayerState public AttributeSnapshot? GetAttribute(AttributeKind kind) => _attrs.TryGetValue(kind, out var a) ? a : null; + /// The optional wired spellbook — read-only exposure of the same + /// reference passed to the constructor. Issue #267: lets presentation + /// code (e.g. CharacterSheetProvider) subscribe to + /// for live character-sheet + /// refresh without duplicating the reference in its own constructor. + public Spellbook? Spellbook => _spellbook; + + /// + /// Issue #267 — effective (post-buff) primary-attribute value. Retail + /// CACQualities::EnchantAttribute (0x00594570): primary attributes + /// do NOT receive vitae (only 's secondary + /// attributes and 's skills do). Returns + /// the unenchanted base when no spellbook is wired (back-compat) or the + /// attribute hasn't arrived yet. + /// + public int? GetEffectiveAttribute(AttributeKind kind) + { + AttributeSnapshot? attr = GetAttribute(kind); + if (attr is null) return null; + uint baseValue = attr.Value.Current; + if (_spellbook is null) return (int)baseValue; + EnchantmentMath.VitalMod mod = _spellbook.GetAttributeMod(AttributeKindToId(kind)); + return EnchantmentMath.EnchantAttribute(mod, baseValue); + } + + /// Reverse of — the wire + /// PropertyAttribute id (1..6) for a given . + public static uint AttributeKindToId(AttributeKind kind) => kind switch + { + AttributeKind.Strength => 1u, + AttributeKind.Endurance => 2u, + AttributeKind.Quickness => 3u, + AttributeKind.Coordination => 4u, + AttributeKind.Focus => 5u, + AttributeKind.Self => 6u, + _ => 0u, + }; + + /// + /// Issue #267 — effective (post-vitae, post-buff) skill level. Retail + /// CACQualities::EnchantSkill (0x005947b0). Returns the + /// unenchanted base when no spellbook is wired (back-compat) or the + /// skill hasn't arrived yet. + /// + public int? GetEffectiveSkill(uint skillId) + { + SkillSnapshot? skill = GetSkill(skillId); + if (skill is null) return null; + uint baseValue = skill.Value.CurrentLevel; + if (_spellbook is null) return (int)baseValue; + EnchantmentMath.VitalMod mod = _spellbook.GetSkillMod(skillId); + return EnchantmentMath.EnchantSkill(mod, baseValue); + } + + /// + /// Issue #267 — vitae's isolated contribution to a skill's effective + /// level (retail SkillInfoRegion::GetVitaeModifier 0x004f0fa0), + /// separate from any buff delta. Always ≤ 0. Returns 0 when no spellbook + /// is wired, no vitae is active, or the skill hasn't arrived yet. + /// + public int GetSkillVitaeModifier(uint skillId) + { + if (_spellbook is null) return 0; + SkillSnapshot? skill = GetSkill(skillId); + if (skill is null) return 0; + return EnchantmentMath.SkillVitaeModifier( + _spellbook.ActiveEnchantments, + skill.Value.CurrentLevel); + } + /// Snapshot of the local player's current property bundle. public PropertyBundle Properties => _properties; diff --git a/src/AcDream.Core/Spells/EnchantmentMath.cs b/src/AcDream.Core/Spells/EnchantmentMath.cs index 6fd27eee..25a308e4 100644 --- a/src/AcDream.Core/Spells/EnchantmentMath.cs +++ b/src/AcDream.Core/Spells/EnchantmentMath.cs @@ -1,3 +1,4 @@ +using System; using System.Collections.Generic; namespace AcDream.Core.Spells; @@ -65,6 +66,35 @@ public static class EnchantmentMath public static readonly VitalMod Identity = new(1.0f, 0.0f); } + /// + /// Retail EnchantmentTypeFlags — the low "StatTypes" byte of the + /// wire StatMod.type field, distinguishing which domain a + /// mult/add enchantment record targets. Verified against + /// references/ACE/Source/ACE.Entity/Enum/EnchantmentTypeFlags.cs + /// and the type argument literals passed to + /// CEnchantmentRegistry::CullEnchantmentsFromList inside + /// EnchantAttribute (0x00594570, type=1), + /// EnchantAttribute2nd (0x00594670, type=2), and + /// EnchantSkill (0x005947b0, type=0x10). + /// + [Flags] + public enum EnchantmentTypeFlag : uint + { + /// Primary attribute (Strength/Endurance/Coordination/ + /// Quickness/Focus/Self). CEnchantmentRegistry::EnchantAttribute + /// filters on this bit and does NOT reference the vitae singleton. + Attribute = 0x0000001, + + /// Secondary attribute — vital max (MaxHealth/MaxStamina/ + /// MaxMana). EnchantAttribute2nd filters on this bit and DOES + /// apply vitae first. + SecondAtt = 0x0000002, + + /// Skill. EnchantSkill filters on this bit and DOES + /// apply vitae first. + Skill = 0x0000010, + } + /// /// Compute the combined buff modifier for a given stat key from /// the player's active enchantments. Returns @@ -77,10 +107,22 @@ public static class EnchantmentMath /// Target stat key (ACE /// PropertyAttribute2nd enum value: 1=MaxHealth, /// 3=MaxStamina, 5=MaxMana). + /// When non-null, mult/add records must also + /// carry this bit in their wire StatModType to contribute — this + /// is what keeps a Strength buff (Attribute, key=1) from leaking into a + /// MaxHealth (SecondAtt, key=1) computation just because the numeric key + /// happens to collide. null preserves the original no-type-filter + /// behavior (existing callers). + /// Whether the vitae singleton (Bucket 4) + /// contributes. Retail's EnchantAttribute never references vitae + /// at all (primary attributes are vitae-immune); pass false for + /// that domain. Defaults to true, matching every existing caller. public static VitalMod GetMod( IEnumerable enchantments, SpellTable table, - uint statKey) + uint statKey, + EnchantmentTypeFlag? requiredType = null, + bool includeVitae = true) { // Family-stacking: bucket the active enchantments by Family and // keep the strongest one per bucket (the one with the largest @@ -121,17 +163,24 @@ public static class EnchantmentMath // Vitae (bucket 4) is a special-case singleton on // CEnchantmentRegistry._vitae and applies its multiplier // to ALL vitals regardless of StatModKey (retail uses - // key=0 as "any vital"). Apply unconditionally and skip - // the per-key check. + // key=0 as "any vital"). Apply unconditionally (subject only + // to includeVitae — EnchantAttribute never references vitae + // at all) and skip the per-key check. if (ench.Bucket == 4) { - vitae *= val; + if (includeVitae) vitae *= val; continue; } // Multiplicative + Additive buffs filter by stat key — // only those targeting the requested vital contribute. if (ench.StatModKey is not uint key || key != statKey) continue; + // Domain filter: a numeric key can collide across Attribute / + // SecondAtt / Skill (e.g. key=1 is both Strength and MaxHealth). + // requiredType disambiguates using the wire StatModType bits. + if (requiredType is EnchantmentTypeFlag type + && (ench.StatModType.GetValueOrDefault() & (uint)type) == 0) + continue; switch (ench.Bucket) { case 1: multiplier *= val; break; @@ -139,14 +188,90 @@ public static class EnchantmentMath // Bucket 8 (Cooldown) doesn't affect vital max. } } - // Vitae is applied multiplicatively last per retail - // CEnchantmentRegistry::EnchantAttribute behaviour. + // Vitae is applied multiplicatively before the buff lists per retail + // CEnchantmentRegistry::EnchantAttribute2nd (0x00594670) / EnchantSkill + // (0x005947b0) — both apply `_vitae` to the base value first, then run + // the mult/add culled list on top. Multiplication is commutative, so + // folding vitae into `multiplier` here is equivalent regardless of + // includeVitae being honored above. Retail's primary-attribute sibling + // (EnchantAttribute, 0x00594570) never references `_vitae` at all — + // callers pass includeVitae:false for that domain. multiplier *= vitae; return multiplier == 1.0f && additive == 0.0f ? VitalMod.Identity : new VitalMod(multiplier, additive); } + /// + /// Retail CEnchantmentRegistry::EnchantAttribute (0x00594570) final + /// composition step: apply the aggregated mult/add modifier (from + /// with + /// and includeVitae:false) to the base value, floor the result at 1 + /// when the base is below 10 or at 10 otherwise, then truncate to an int + /// (retail _ftol2, C# (int) cast — both truncate toward + /// zero). Primary attributes are vitae-immune in retail; only buffs move + /// this number. + /// + public static int EnchantAttribute(VitalMod mod, uint baseValue) + { + float value = (baseValue * mod.Multiplier) + mod.Additive; + float floor = baseValue < 10u ? 1f : 10f; + if (value < floor) value = floor; + return (int)value; + } + + /// + /// Retail CEnchantmentRegistry::EnchantSkill (0x005947b0) final + /// composition step: apply the aggregated (vitae-inclusive) mult/add + /// modifier from (with + /// , includeVitae:true) to + /// the base value, zero-floor when the result drops below 0.5, then + /// truncate to an int. + /// + public static int EnchantSkill(VitalMod mod, uint baseValue) + { + float value = (baseValue * mod.Multiplier) + mod.Additive; + if (value < 0.5f) value = 0f; + return (int)value; + } + + /// + /// Isolated vitae multiplier — retail's CEnchantmentRegistry::_vitae + /// singleton applied WITHOUT the mult/add buff lists. Returns 1.0 + /// (no penalty) when no vitae enchantment is active. Vitae is never + /// subject to family-stacking dedup in retail (there is exactly one + /// _vitae field, never a list), so this scans directly rather than + /// routing through 's family-stacking pass. + /// + public static float GetVitaeMultiplier(IEnumerable enchantments) + { + ArgumentNullException.ThrowIfNull(enchantments); + float vitae = 1.0f; + foreach (ActiveEnchantmentRecord ench in enchantments) + { + if (ench.Bucket == 4 && ench.StatModValue is float val) + vitae *= val; + } + return vitae; + } + + /// + /// Retail SkillInfoRegion::GetVitaeModifier (0x004f0fa0): the + /// vitae-only contribution to a skill's effective level, reported in + /// isolation from any buffs — truncate(base × vitaeMult) − base, + /// always ≤ 0 (vitae is a strict penalty). This is the exact number the + /// retail Character window shows in the vitae-specific footer + /// parenthetical (e.g. "(-100)"), separate from any buff delta. + /// + public static int SkillVitaeModifier( + IEnumerable enchantments, + uint baseValue) + { + float vitae = GetVitaeMultiplier(enchantments); + if (vitae == 1.0f) return 0; + return (int)(baseValue * vitae) - (int)baseValue; + } + /// /// Stat-key constants matching ACE PropertyAttribute2nd /// (verified against docs/research/named-retail/acclient.h diff --git a/src/AcDream.Core/Spells/Spellbook.cs b/src/AcDream.Core/Spells/Spellbook.cs index 90c91da7..6eb2f718 100644 --- a/src/AcDream.Core/Spells/Spellbook.cs +++ b/src/AcDream.Core/Spells/Spellbook.cs @@ -24,6 +24,8 @@ public sealed class Spellbook private readonly Dictionary _activeById = new(); private readonly Dictionary> _enchantmentOrderByBucket = new(); private readonly Dictionary _vitalModCache = new(); + private readonly Dictionary _attributeModCache = new(); + private readonly Dictionary _skillModCache = new(); private readonly List[] _favoriteSpells = Enumerable.Range(0, 8) .Select(_ => new List()).ToArray(); private readonly Dictionary _desiredComponents = new(); @@ -102,6 +104,61 @@ public sealed class Spellbook return calculated; } + /// + /// Issue #267 — combined buff modifier for a primary attribute + /// (Strength/Endurance/Coordination/Quickness/Focus/Self; ACE + /// PropertyAttribute id 1-6). Retail + /// CEnchantmentRegistry::EnchantAttribute (0x00594570) never + /// references the vitae singleton, so this excludes vitae — + /// primary attributes are vitae-immune in retail. Feed the result + /// through with the + /// base value to get the final displayed int. + /// + public EnchantmentMath.VitalMod GetAttributeMod(uint attributeId) + { + if (_attributeModCache.TryGetValue( + attributeId, + out EnchantmentMath.VitalMod cached)) + { + return cached; + } + + EnchantmentMath.VitalMod calculated = EnchantmentMath.GetMod( + ActiveEnchantments, + _table, + attributeId, + EnchantmentMath.EnchantmentTypeFlag.Attribute, + includeVitae: false); + _attributeModCache.Add(attributeId, calculated); + return calculated; + } + + /// + /// Issue #267 — combined (vitae-inclusive) buff modifier for a skill + /// (ACE SkillId). Retail CEnchantmentRegistry::EnchantSkill + /// (0x005947b0) applies the vitae singleton before the mult/add buff + /// lists. Feed the result through + /// with the base skill level to get the final displayed int. + /// + public EnchantmentMath.VitalMod GetSkillMod(uint skillId) + { + if (_skillModCache.TryGetValue( + skillId, + out EnchantmentMath.VitalMod cached)) + { + return cached; + } + + EnchantmentMath.VitalMod calculated = EnchantmentMath.GetMod( + ActiveEnchantments, + _table, + skillId, + EnchantmentMath.EnchantmentTypeFlag.Skill, + includeVitae: true); + _skillModCache.Add(skillId, calculated); + return calculated; + } + /// Fires when a spell is added to the player's spellbook. public event Action? SpellLearned; @@ -356,6 +413,8 @@ public sealed class Spellbook _activeById.Clear(); _enchantmentOrderByBucket.Clear(); _vitalModCache.Clear(); + _attributeModCache.Clear(); + _skillModCache.Clear(); foreach (ActiveEnchantmentRecord enchantment in enchantments) UpsertManifestEnchantment(enchantment); @@ -430,6 +489,8 @@ public sealed class Spellbook _activeById.Clear(); _enchantmentOrderByBucket.Clear(); _vitalModCache.Clear(); + _attributeModCache.Clear(); + _skillModCache.Clear(); foreach (List tab in _favoriteSpells) tab.Clear(); _desiredComponents.Clear(); _spellbookFilters = 0x3FFFu; @@ -448,6 +509,8 @@ public sealed class Spellbook private void NotifyEnchantmentsChanged() { _vitalModCache.Clear(); + _attributeModCache.Clear(); + _skillModCache.Clear(); EnchantmentsChanged?.Invoke(); StateChanged?.Invoke(); } diff --git a/tests/AcDream.App.Tests/UI/Layout/CharacterSheetProviderTests.cs b/tests/AcDream.App.Tests/UI/Layout/CharacterSheetProviderTests.cs index b2e00561..c69004dc 100644 --- a/tests/AcDream.App.Tests/UI/Layout/CharacterSheetProviderTests.cs +++ b/tests/AcDream.App.Tests/UI/Layout/CharacterSheetProviderTests.cs @@ -3,6 +3,7 @@ using System.Linq; using AcDream.App.UI.Layout; using AcDream.Core.Items; using AcDream.Core.Player; +using AcDream.Core.Spells; using Xunit; namespace AcDream.App.Tests.UI.Layout; @@ -208,4 +209,113 @@ public sealed class CharacterSheetProviderTests Assert.Equal(400L, h.Player.Properties.GetInt64(2u)); // debited on the LPS side Assert.True(changed >= 1); // and CharacterChanged fired } + + // ── Issue #267 — vitae/buff-aware skill + attribute values ─────────────── + + private static SpellMetadata TestSpell(uint spellId) => new( + spellId, "Test", "War Magic", 0u, 0u, "", 0f, 0, + false, false, "", 0, 0, 0u, 0, false, false, true, + 0f, 0u, 0u, 0u, 0); + + private sealed class VitaeHarness + { + public ClientObjectTable Table { get; } = new(); + public Spellbook Book { get; } + public LocalPlayerState Player { get; } + public CharacterSheetProvider Provider { get; } + + public VitaeHarness() + { + Book = new Spellbook(SpellTable.Create([TestSpell(1u), TestSpell(2u)])); + Player = new LocalPlayerState(Book); + Provider = new CharacterSheetProvider( + Table, Player, + playerGuid: () => 0u, + activeToonName: () => "default", + fallbackSheet: name => new CharacterSheet { Name = name, Level = -1 }); + } + } + + [Fact] + public void BuildSheet_AttributeBuff_ShowsEffectiveValueAndBasePair() + { + var h = new VitaeHarness(); + h.Player.OnAttributeUpdate(atType: 1u, ranks: 100u, start: 100u, xp: 0u); // Strength, base 200 + h.Book.OnEnchantmentAdded(new ActiveEnchantmentRecord( + SpellId: 2u, LayerId: 1u, Duration: 60d, CasterGuid: 0u, + StatModType: (uint)EnchantmentMath.EnchantmentTypeFlag.Attribute, + StatModKey: 1u, StatModValue: 1.1f, Bucket: 1u)); + + var sheet = h.Provider.BuildSheet(); + + Assert.Equal(220, sheet.Strength); // effective: 200 * 1.1 + Assert.Equal(200, sheet.AttributeBaseValues[0]); // base unaffected + } + + [Fact] + public void BuildSheet_AttributeUnderVitae_AttributesAreVitaeImmune() + { + var h = new VitaeHarness(); + h.Player.OnAttributeUpdate(atType: 1u, ranks: 100u, start: 100u, xp: 0u); // Strength, base 200 + h.Book.OnEnchantmentAdded(new ActiveEnchantmentRecord( + SpellId: 1u, LayerId: 1u, Duration: -1d, CasterGuid: 0u, + StatModType: 0u, StatModKey: 0u, StatModValue: 0.67f, Bucket: 4u)); // 33% vitae + + var sheet = h.Provider.BuildSheet(); + + Assert.Equal(200, sheet.Strength); // unaffected by vitae + Assert.Equal(200, sheet.AttributeBaseValues[0]); + } + + [Fact] + public void BuildSheet_SkillUnderVitae_ShowsEffectiveLevelAndVitaeModifier() + { + var h = new VitaeHarness(); + h.Player.OnSkillUpdate(skillId: 6u, ranks: 300u, status: 2u, xp: 0u, + init: 3u, resistance: 0u, lastUsed: 0d, formulaBonus: 0u); // base 303 + h.Book.OnEnchantmentAdded(new ActiveEnchantmentRecord( + SpellId: 1u, LayerId: 1u, Duration: -1d, CasterGuid: 0u, + StatModType: 0u, StatModKey: 0u, StatModValue: 0.67f, Bucket: 4u)); // 33% vitae + + var sheet = h.Provider.BuildSheet(); + + var skill = Assert.Single(sheet.Skills); + Assert.Equal(303, skill.BaseLevel); + Assert.Equal(203, skill.CurrentLevel); // 303 * 0.67, truncated + Assert.Equal(-100, skill.VitaeModifier); // the exact user-reported oracle example + } + + [Fact] + public void SubscribeChanged_FiresOnEnchantmentsChanged_AndRebuildReflectsNewValue() + { + var h = new VitaeHarness(); + h.Player.OnAttributeUpdate(atType: 1u, ranks: 100u, start: 100u, xp: 0u); // Strength, base 200 + int changed = 0; + using IDisposable subscription = h.Provider.SubscribeChanged(() => changed++); + + Assert.Equal(200, h.Provider.BuildSheet().Strength); // no buff yet + + h.Book.OnEnchantmentAdded(new ActiveEnchantmentRecord( + SpellId: 2u, LayerId: 1u, Duration: 60d, CasterGuid: 0u, + StatModType: (uint)EnchantmentMath.EnchantmentTypeFlag.Attribute, + StatModKey: 1u, StatModValue: 1.1f, Bucket: 1u)); + + Assert.True(changed >= 1); // live-refresh notice fired + Assert.Equal(220, h.Provider.BuildSheet().Strength); // and the rebuilt sheet reflects it + } + + [Fact] + public void SubscribeChanged_Dispose_UnsubscribesFromEnchantmentsChanged() + { + var h = new VitaeHarness(); + int changed = 0; + IDisposable subscription = h.Provider.SubscribeChanged(() => changed++); + subscription.Dispose(); + + h.Book.OnEnchantmentAdded(new ActiveEnchantmentRecord( + SpellId: 1u, LayerId: 1u, Duration: -1d, CasterGuid: 0u, + StatModType: 0u, StatModKey: 0u, StatModValue: 0.67f, Bucket: 4u)); + + Assert.Equal(0, changed); + } } diff --git a/tests/AcDream.App.Tests/UI/Layout/CharacterStatControllerTests.cs b/tests/AcDream.App.Tests/UI/Layout/CharacterStatControllerTests.cs index 5d2a39f0..debb7509 100644 --- a/tests/AcDream.App.Tests/UI/Layout/CharacterStatControllerTests.cs +++ b/tests/AcDream.App.Tests/UI/Layout/CharacterStatControllerTests.cs @@ -969,7 +969,11 @@ public class CharacterStatControllerTests var rows = SkillRows(list); rows[1].OnClick!(); - Assert.Equal("War Magic: 285", title.LinesProvider()[0].Text); + // SampleData's War Magic row is intentionally Base=280/Current=285 (an + // illustrative buffed-skill sample — see SampleData.cs comment); issue + // #267 wires the retail "(+5)" buff-delta parenthetical onto that gap, + // which was previously computed but never surfaced in the title text. + Assert.Equal("War Magic: 285 (+5)", title.LinesProvider()[0].Text); Assert.Equal("Experience To Raise:", l1Label.LinesProvider()[0].Text); Assert.Equal((11_100_000L).ToString("N0"), l1Value.LinesProvider()[0].Text); Assert.Equal("Unassigned Experience:", l2Label.LinesProvider()[0].Text); @@ -1270,6 +1274,178 @@ public class CharacterStatControllerTests public void GetRowName_NegativeIndex_ReturnsEmpty() => Assert.Equal(string.Empty, CharacterStatController.GetRowName(-1)); + // ── Issue #267 — vitae/buff delta parenthetical ────────────────────────── + // Retail format cited from gmAttributeUI::DisplaySelectionFooter_Attribute + // (0x0049d280, " (%s%d)") and gmSkillUI::DisplaySelectionFooter_Trained + // (0x0049b860, vitae segment " (%d)" + buff segment " (%s%d)"). + + [Fact] + public void GetAttributeDelta_ComputesEffectiveMinusBase() + { + var sheet = new CharacterSheet { Strength = 220, AttributeBaseValues = [200, 0, 0, 0, 0, 0] }; + Assert.Equal(20, CharacterStatController.GetAttributeDelta(sheet, 0)); + } + + [Fact] + public void GetAttributeDelta_VitalRowIndex_ReturnsZero() + { + // Vitals (rows 6-8) use a "cur/max" footer format in retail + // (DisplaySelectionFooter_Vital), not this delta pattern. + var sheet = new CharacterSheet { AttributeBaseValues = [200, 0, 0, 0, 0, 0] }; + Assert.Equal(0, CharacterStatController.GetAttributeDelta(sheet, 6)); + } + + [Fact] + public void GetAttributeDelta_EmptyBaseValues_ReturnsZero() + { + // SampleData / older sheets that don't populate AttributeBaseValues + // must not throw or fabricate a delta. + var sheet = new CharacterSheet { Strength = 220 }; + Assert.Equal(0, CharacterStatController.GetAttributeDelta(sheet, 0)); + } + + [Fact] + public void GetSkillBuffOnlyDelta_IsolatesBuffFromVitae() + { + // CurrentLevel=251 already includes vitae (-99); the buff-only + // residual is (251 - (-99)) - 300 = 50. + var skill = new CharacterSkill(1u, "S", 0u, CharacterSkillAdvancementClass.Trained, + BaseLevel: 300, CurrentLevel: 251, UsableUntrained: true, + TrainedCost: 0, SpecializedCost: 0, RaiseCost: 0, VitaeModifier: -99); + Assert.Equal(50, CharacterStatController.GetSkillBuffOnlyDelta(skill)); + } + + [Fact] + public void RowClick_AttributeWithBuff_FooterTitleShowsPositiveDelta() + { + var title = new UiText(); + var list = new UiPanel(); + var layout = Fake( + (CharacterStatController.FooterTitleId, title), + (CharacterStatController.ListBoxId, list)); + + CharacterSheet Sheet() => new() { Strength = 220, AttributeBaseValues = [200, 0, 0, 0, 0, 0] }; + CharacterStatController.Bind(layout, Sheet); + + list.Children.OfType().ToList()[0].OnClick!(); // Strength = index 0 + + Assert.Equal("Strength: 220 (+20)", title.LinesProvider()[0].Text); + } + + [Fact] + public void RowClick_AttributeWithDebuff_FooterTitleShowsNegativeDelta() + { + var title = new UiText(); + var list = new UiPanel(); + var layout = Fake( + (CharacterStatController.FooterTitleId, title), + (CharacterStatController.ListBoxId, list)); + + CharacterSheet Sheet() => new() { Endurance = 180, AttributeBaseValues = [0, 200, 0, 0, 0, 0] }; + CharacterStatController.Bind(layout, Sheet); + + list.Children.OfType().ToList()[1].OnClick!(); // Endurance = index 1 + + Assert.Equal("Endurance: 180 (-20)", title.LinesProvider()[0].Text); + } + + [Fact] + public void RowClick_AttributeZeroDelta_NoParenthetical() + { + var title = new UiText(); + var list = new UiPanel(); + var layout = Fake( + (CharacterStatController.FooterTitleId, title), + (CharacterStatController.ListBoxId, list)); + + CharacterSheet Sheet() => new() { Strength = 200, AttributeBaseValues = [200, 0, 0, 0, 0, 0] }; + CharacterStatController.Bind(layout, Sheet); + + list.Children.OfType().ToList()[0].OnClick!(); + + Assert.Equal("Strength: 200", title.LinesProvider()[0].Text); + } + + [Fact] + public void SkillClick_VitaeOnly_FooterTitleShowsVitaeParenthetical() + { + // User-reported oracle (ISSUES.md #267): a base-303 skill under 33% + // vitae shows the current (reduced) level with "(-100)". + var list = new UiPanel { Width = 300 }; + var title = new UiText(); + var layout = Fake( + (CharacterStatController.ListBoxId, list), + (CharacterStatController.FooterTitleId, title)); + + CharacterSheet Sheet() => VitaeSkillSheet(currentLevel: 203, baseLevel: 303, vitaeModifier: -100); + CharacterStatController.Bind(layout, Sheet, spriteResolve: id => (id, 16, 16)); + + ClickTab(layout, left: 92f); + SkillRows(list)[0].OnClick!(); + + Assert.Equal("Test Skill: 203 (-100)", title.LinesProvider()[0].Text); + } + + [Fact] + public void SkillClick_VitaePlusBuff_FooterTitleShowsBothParentheticals() + { + var list = new UiPanel { Width = 300 }; + var title = new UiText(); + var layout = Fake( + (CharacterStatController.ListBoxId, list), + (CharacterStatController.FooterTitleId, title)); + + // CurrentLevel=251, base=300, vitae modifier=-99 (300*0.67-300). + // Buff-only delta = (251 - (-99)) - 300 = 50. + CharacterSheet Sheet() => VitaeSkillSheet(currentLevel: 251, baseLevel: 300, vitaeModifier: -99); + CharacterStatController.Bind(layout, Sheet, spriteResolve: id => (id, 16, 16)); + + ClickTab(layout, left: 92f); + SkillRows(list)[0].OnClick!(); + + Assert.Equal("Test Skill: 251 (-99) (+50)", title.LinesProvider()[0].Text); + } + + [Fact] + public void SkillClick_ZeroDelta_NoParentheticals() + { + var list = new UiPanel { Width = 300 }; + var title = new UiText(); + var layout = Fake( + (CharacterStatController.ListBoxId, list), + (CharacterStatController.FooterTitleId, title)); + + CharacterSheet Sheet() => VitaeSkillSheet(currentLevel: 300, baseLevel: 300, vitaeModifier: 0); + CharacterStatController.Bind(layout, Sheet, spriteResolve: id => (id, 16, 16)); + + ClickTab(layout, left: 92f); + SkillRows(list)[0].OnClick!(); + + Assert.Equal("Test Skill: 300", title.LinesProvider()[0].Text); + } + + private static CharacterSheet VitaeSkillSheet(int currentLevel, int baseLevel, int vitaeModifier) => new() + { + SkillCredits = 0, + UnassignedXp = 1_000_000, + Skills = + [ + new CharacterSkill( + 200u, + "Test Skill", + 0x06000001u, + CharacterSkillAdvancementClass.Trained, + BaseLevel: baseLevel, + CurrentLevel: currentLevel, + UsableUntrained: true, + TrainedCost: 0, + SpecializedCost: 0, + RaiseCost: 100, + Raise10Cost: 1000, + VitaeModifier: vitaeModifier), + ], + }; + // ── SampleData sanity ───────────────────────────────────────────────────── [Fact] diff --git a/tests/AcDream.Core.Tests/Player/LocalPlayerStateTests.cs b/tests/AcDream.Core.Tests/Player/LocalPlayerStateTests.cs index 82d22ca6..638eecf7 100644 --- a/tests/AcDream.Core.Tests/Player/LocalPlayerStateTests.cs +++ b/tests/AcDream.Core.Tests/Player/LocalPlayerStateTests.cs @@ -1,5 +1,6 @@ using AcDream.Core.Items; using AcDream.Core.Player; +using AcDream.Core.Spells; namespace AcDream.Core.Tests.Player; @@ -477,4 +478,117 @@ public sealed class LocalPlayerStateTests Assert.Empty(s.Skills); Assert.Empty(s.Properties.Ints); } + + // ── Issue #267 — effective attribute/skill values (vitae + buff aware) ── + + [Fact] + public void GetEffectiveAttribute_NoSpellbook_ReturnsBaseValue() + { + var s = new LocalPlayerState(); // no spellbook wired — back-compat + s.OnAttributeUpdate(atType: 1u, ranks: 100u, start: 100u, xp: 0u); // Strength, base 200 + + Assert.Equal(200, s.GetEffectiveAttribute(LocalPlayerState.AttributeKind.Strength)); + } + + [Fact] + public void GetEffectiveAttribute_Unseen_ReturnsNull() + { + var s = new LocalPlayerState(new Spellbook()); + Assert.Null(s.GetEffectiveAttribute(LocalPlayerState.AttributeKind.Strength)); + } + + [Fact] + public void GetEffectiveAttribute_VitaeActive_AttributesAreVitaeImmune() + { + // Retail CACQualities::EnchantAttribute (0x00594570) never references + // the vitae singleton — a 33% vitae penalty must NOT move Strength. + var book = new Spellbook(SpellTable.Create([TestSpell(1u), TestSpell(2u)])); + book.OnEnchantmentAdded(new ActiveEnchantmentRecord( + SpellId: 1u, LayerId: 1u, Duration: -1d, CasterGuid: 0u, + StatModType: 0u, StatModKey: 0u, StatModValue: 0.67f, Bucket: 4u)); + var s = new LocalPlayerState(book); + s.OnAttributeUpdate(atType: 1u, ranks: 100u, start: 100u, xp: 0u); // Strength, base 200 + + Assert.Equal(200, s.GetEffectiveAttribute(LocalPlayerState.AttributeKind.Strength)); + } + + [Fact] + public void GetEffectiveAttribute_Buff_AppliesMultiplierAndTruncates() + { + var book = new Spellbook(SpellTable.Create([TestSpell(1u), TestSpell(2u)])); + book.OnEnchantmentAdded(new ActiveEnchantmentRecord( + SpellId: 2u, LayerId: 1u, Duration: 60d, CasterGuid: 0u, + StatModType: (uint)EnchantmentMath.EnchantmentTypeFlag.Attribute, + StatModKey: 1u /* Strength */, StatModValue: 1.1f, Bucket: 1u)); + var s = new LocalPlayerState(book); + s.OnAttributeUpdate(atType: 1u, ranks: 100u, start: 100u, xp: 0u); // Strength, base 200 + + // 200 * 1.1 = 220. + Assert.Equal(220, s.GetEffectiveAttribute(LocalPlayerState.AttributeKind.Strength)); + } + + [Fact] + public void GetEffectiveSkill_NoSpellbook_ReturnsBaseValue() + { + var s = new LocalPlayerState(); + s.OnSkillUpdate(skillId: 6u, ranks: 300u, status: 2u, xp: 0u, + init: 3u, resistance: 0u, lastUsed: 0d, formulaBonus: 0u); // base 303 + + Assert.Equal(303, s.GetEffectiveSkill(6u)); + Assert.Equal(0, s.GetSkillVitaeModifier(6u)); + } + + [Fact] + public void GetEffectiveSkill_Unseen_ReturnsNull() + { + var s = new LocalPlayerState(new Spellbook()); + Assert.Null(s.GetEffectiveSkill(6u)); + } + + [Fact] + public void GetEffectiveSkill_ThirtyThreePercentVitae_MatchesUserReportedGolden() + { + // User-reported oracle (ISSUES.md #267): a base-303 skill under 33% + // vitae shows the current (reduced) level with "(-100)" in parens. + var book = new Spellbook(SpellTable.Create([TestSpell(1u), TestSpell(2u)])); + book.OnEnchantmentAdded(new ActiveEnchantmentRecord( + SpellId: 1u, LayerId: 1u, Duration: -1d, CasterGuid: 0u, + StatModType: 0u, StatModKey: 0u, StatModValue: 0.67f, Bucket: 4u)); + var s = new LocalPlayerState(book); + s.OnSkillUpdate(skillId: 6u, ranks: 300u, status: 2u, xp: 0u, + init: 3u, resistance: 0u, lastUsed: 0d, formulaBonus: 0u); // base 303 + + Assert.Equal(203, s.GetEffectiveSkill(6u)); + Assert.Equal(-100, s.GetSkillVitaeModifier(6u)); + } + + [Fact] + public void GetEffectiveSkill_BuffPlusVitaeComposition() + { + var book = new Spellbook(SpellTable.Create([TestSpell(1u), TestSpell(2u)])); + book.OnEnchantmentAdded(new ActiveEnchantmentRecord( + SpellId: 1u, LayerId: 1u, Duration: -1d, CasterGuid: 0u, + StatModType: 0u, StatModKey: 0u, StatModValue: 0.67f, Bucket: 4u)); // 33% vitae + book.OnEnchantmentAdded(new ActiveEnchantmentRecord( + SpellId: 2u, LayerId: 2u, Duration: 60d, CasterGuid: 0u, + StatModType: (uint)EnchantmentMath.EnchantmentTypeFlag.Skill, + StatModKey: 6u, StatModValue: 50f, Bucket: 2u)); // +50 additive buff + var s = new LocalPlayerState(book); + s.OnSkillUpdate(skillId: 6u, ranks: 297u, status: 2u, xp: 0u, + init: 3u, resistance: 0u, lastUsed: 0d, formulaBonus: 0u); // base 300 + + // (300 * 0.67) + 50 = 201 + 50 = 251. Vitae-only contribution stays + // isolated at (300*0.67) - 300 = -99. + Assert.Equal(251, s.GetEffectiveSkill(6u)); + Assert.Equal(-99, s.GetSkillVitaeModifier(6u)); + } + + /// Minimal SpellTable row — 's + /// family-stacking pass skips any enchantment whose SpellId isn't in the + /// table, so vitae/buff test records need an entry here even with + /// Family=0 (no dedup bucket). + private static SpellMetadata TestSpell(uint spellId) => new( + spellId, "Test", "War Magic", 0u, 0u, "", 0f, 0, + false, false, "", 0, 0, 0u, 0, false, false, true, + 0f, 0u, 0u, 0u, 0); } diff --git a/tests/AcDream.Core.Tests/Spells/EnchantmentMathTests.cs b/tests/AcDream.Core.Tests/Spells/EnchantmentMathTests.cs index 08f6c574..5fb493b7 100644 --- a/tests/AcDream.Core.Tests/Spells/EnchantmentMathTests.cs +++ b/tests/AcDream.Core.Tests/Spells/EnchantmentMathTests.cs @@ -219,6 +219,150 @@ public sealed class EnchantmentMathTests Assert.Equal(1.5f, mod.Multiplier, precision: 3); } + // ── Issue #267 — Attribute/Skill domain filter + EnchantAttribute/EnchantSkill goldens ── + + [Fact] + public void GetMod_RequiredType_ExcludesCrossDomainKeyCollision() + { + // A Strength buff (Attribute, key=1) must NOT leak into a MaxHealth + // (SecondAtt, key=1) computation just because the numeric key collides. + var table = LoadTable((50u, "Strength Buff", 0u)); + var enchantments = new[] + { + MakeTypedMultRecord(spellId: 50, layer: 1, statKey: 1u, + statModType: (uint)EnchantmentMath.EnchantmentTypeFlag.Attribute, val: 1.5f), + }; + + var secondAttMod = EnchantmentMath.GetMod(enchantments, table, statKey: 1u, + EnchantmentMath.EnchantmentTypeFlag.SecondAtt); + Assert.Equal(EnchantmentMath.VitalMod.Identity, secondAttMod); + + var attributeMod = EnchantmentMath.GetMod(enchantments, table, statKey: 1u, + EnchantmentMath.EnchantmentTypeFlag.Attribute); + Assert.Equal(1.5f, attributeMod.Multiplier, precision: 3); + } + + [Fact] + public void GetMod_IncludeVitaeFalse_ExcludesVitaeEvenWhenActive() + { + // Primary attributes (EnchantAttribute) never reference the vitae + // singleton in retail — includeVitae:false must fully exclude it. + var table = LoadTable((60u, "Vitae", 0u)); + var enchantments = new[] + { + MakeVitaeRecord(spellId: 60, layer: 1, statKey: 0u, val: 0.67f), + }; + + var mod = EnchantmentMath.GetMod(enchantments, table, statKey: 1u, + EnchantmentMath.EnchantmentTypeFlag.Attribute, includeVitae: false); + Assert.Equal(EnchantmentMath.VitalMod.Identity, mod); + } + + [Fact] + public void EnchantAttribute_NoMods_ReturnsBaseTruncated() + { + Assert.Equal(200, EnchantmentMath.EnchantAttribute(EnchantmentMath.VitalMod.Identity, 200u)); + } + + [Fact] + public void EnchantAttribute_Buff_AppliesMultiplierAndTruncates() + { + // 200 base with a +10% buff (unrelated to vitae — attributes are + // vitae-immune) → 220. + var mod = new EnchantmentMath.VitalMod(1.1f, 0f); + Assert.Equal(220, EnchantmentMath.EnchantAttribute(mod, 200u)); + } + + [Fact] + public void EnchantAttribute_FloorsAtOne_WhenBaseBelowTenAndDebuffed() + { + // Base 5 (< 10) crushed by a 0.1 multiplier would compute to 0.5, + // but retail floors small attributes at 1 rather than letting them + // hit zero (0x00594570: `< 0xa` branch floors at 1f). + var mod = new EnchantmentMath.VitalMod(0.1f, 0f); + Assert.Equal(1, EnchantmentMath.EnchantAttribute(mod, 5u)); + } + + [Fact] + public void EnchantAttribute_FloorsAtTen_WhenBaseAtOrAboveTenAndDebuffed() + { + // Base 50 (>= 10) crushed by a 0.1 multiplier would compute to 5, + // but retail floors at 10 for this base range. + var mod = new EnchantmentMath.VitalMod(0.1f, 0f); + Assert.Equal(10, EnchantmentMath.EnchantAttribute(mod, 50u)); + } + + [Fact] + public void EnchantSkill_ThirtyThreePercentVitae_MatchesGoldenValue() + { + // 33% vitae penalty on a base-303 skill: 303 * 0.67 = 203.01 -> 203. + var mod = new EnchantmentMath.VitalMod(0.67f, 0f); + Assert.Equal(203, EnchantmentMath.EnchantSkill(mod, 303u)); + } + + [Fact] + public void EnchantSkill_BuffPlusVitaeComposition_MatchesGoldenValue() + { + // Base 300, vitae 0.67 (33%) composed with a +50 additive buff: + // (300 * 0.67) + 50 = 201 + 50 = 251. + var mod = new EnchantmentMath.VitalMod(0.67f, 50f); + Assert.Equal(251, EnchantmentMath.EnchantSkill(mod, 300u)); + } + + [Fact] + public void EnchantSkill_ZeroFloorsBelowHalf() + { + // A crushing vitae/debuff combination that drops the result under + // 0.5 floors to 0 rather than truncating to a stray small value. + var mod = new EnchantmentMath.VitalMod(0.001f, 0f); + Assert.Equal(0, EnchantmentMath.EnchantSkill(mod, 10u)); + } + + [Fact] + public void GetVitaeMultiplier_NoVitae_ReturnsOne() + { + var enchantments = new[] + { + MakeMultRecord(spellId: 1, layer: 1, statKey: 1u, val: 1.5f), + }; + Assert.Equal(1.0f, EnchantmentMath.GetVitaeMultiplier(enchantments)); + } + + [Fact] + public void GetVitaeMultiplier_WithVitae_ReturnsItsValue() + { + var enchantments = new[] + { + MakeVitaeRecord(spellId: 1, layer: 1, statKey: 0u, val: 0.67f), + MakeMultRecord(spellId: 2, layer: 2, statKey: 1u, val: 1.5f), // must not affect vitae isolation + }; + Assert.Equal(0.67f, EnchantmentMath.GetVitaeMultiplier(enchantments), precision: 3); + } + + [Fact] + public void SkillVitaeModifier_ThirtyThreePercent_MatchesGoldenValue() + { + // Base 303 skill, 33% vitae: truncate(303 * 0.67) - 303 = 203 - 303 = -100. + // This is the user-reported oracle example: "(-100)". + var enchantments = new[] + { + MakeVitaeRecord(spellId: 1, layer: 1, statKey: 0u, val: 0.67f), + }; + Assert.Equal(-100, EnchantmentMath.SkillVitaeModifier(enchantments, baseValue: 303u)); + } + + [Fact] + public void SkillVitaeModifier_NoVitae_ReturnsZero() + { + Assert.Equal(0, EnchantmentMath.SkillVitaeModifier( + System.Array.Empty(), baseValue: 303u)); + } + + private static ActiveEnchantmentRecord MakeTypedMultRecord( + uint spellId, uint layer, uint statKey, uint statModType, float val) => + new(spellId, layer, 60f, 0u, StatModType: statModType, StatModKey: statKey, + StatModValue: val, Bucket: 1u); + private static ActiveEnchantmentRecord MakeMultRecord(uint spellId, uint layer, uint statKey, float val) => new(spellId, layer, 60f, 0u, StatModType: 0, StatModKey: statKey, StatModValue: val, Bucket: 1u);