diff --git a/docs/ISSUES.md b/docs/ISSUES.md index 21948fa1..3782b521 100644 --- a/docs/ISSUES.md +++ b/docs/ISSUES.md @@ -100,7 +100,13 @@ Copy this block when adding a new issue: ## #267 — Vitae does not update the character panel's skills/attributes display -**Status:** OPEN +**Status:** IMPLEMENTED 2026-07-30 (`cf2605fa`, merged) — closure pends +the user visual check. Retail finding: primary attributes are +VITAE-IMMUNE (`EnchantAttribute` 0x00594570 never references the vitae +singleton) — only skills and vitals take the penalty. Panel now shows +effective values; skill footer shows the vitae parenthetical (e.g. +"(-100)", `SkillInfoRegion::GetVitaeModifier` 0x004f0fa0) plus a +separate buff residual; refresh fires on EnchantmentsChanged. **Severity:** MEDIUM (matrix live gate 2026-07-30) **Component:** retained UI / character window / vitae 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 d0c0cb4e..e5f76b48 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 @@ -76,20 +106,23 @@ public static class EnchantmentMath /// (only one buff per wins). /// Target stat key (ACE /// PropertyAttribute2nd enum value: 1=MaxHealth, - /// 3=MaxStamina, 5=MaxMana — or a Skill id when - /// is - /// ). - /// When set, a record's - /// StatModType must carry this flag bit to be considered a - /// candidate — disambiguates namespaces that share numeric keys (e.g. - /// vital key 5=MaxMana vs skill id 5). null (default) preserves - /// the original vitals behavior with no type check, unchanged from - /// before Campaign P. + /// 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? requiredStatModTypeFlag = null) + 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 @@ -147,29 +180,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; - continue; - } - - // Campaign P (2026-07-30): an optional type-flag gate - // disambiguates numeric-key collisions across namespaces (e.g. - // vital key 5=MaxMana vs skill id 5) — mirrors retail's - // CullEnchantmentsFromList `category` argument (2 for - // Attribute2nd, 0x10=Skill for EnchantSkill). - if (requiredStatModTypeFlag is uint typeFlag - && (ench.StatModType is not uint recordType - || (recordType & typeFlag) == 0)) - { + if (includeVitae) vitae *= val; continue; } // Multiplicative + Additive buffs filter by stat key — // only those targeting the requested vital/skill 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; @@ -177,8 +205,14 @@ 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 @@ -186,15 +220,82 @@ public static class EnchantmentMath } /// - /// Campaign P Slice P1 (2026-07-30) — Skill-namespace convenience over - /// , matching retail CEnchantmentRegistry:: - /// EnchantSkill (0x005947b0): vitae applies unconditionally (same - /// as vitals), multiplicative/additive skill buffs are filtered to - /// records whose StatModType carries - /// AND whose StatModKey - /// equals (ACE Skill enum ordinal — Run=24, - /// Jump=22). Scoped to the run/jump query path per the P1 plan; not a - /// general effective-skill engine. + /// 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; + } + + /// + /// Campaign P Slice P1 — Skill-namespace convenience over + /// (retail CEnchantmentRegistry::EnchantSkill + /// 0x005947b0): vitae applies (Bucket 4), mult/add records filter on + /// + the skill id. Consumed by + /// RuntimeCharacterState's run/jump recompute; #267's panel path + /// uses the same aggregation via . /// public static VitalMod GetSkillMod( IEnumerable enchantments, @@ -202,18 +303,6 @@ public static class EnchantmentMath uint skillId) => GetMod(enchantments, table, skillId, EnchantmentTypeFlag.Skill); - /// - /// Retail EnchantmentTypeFlags bits relevant to disambiguating - /// 's statKey namespace (ACE - /// ACE.Entity.Enum.EnchantmentTypeFlags, cross-referenced — - /// StatModType is a bitfield the wire already carries per-enchantment). - /// - public static class EnchantmentTypeFlag - { - public const uint SecondAtt = 0x0000002u; - public const uint Skill = 0x0000010u; - } - /// /// 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 cb678abd..b852ff69 100644 --- a/src/AcDream.Core/Spells/Spellbook.cs +++ b/src/AcDream.Core/Spells/Spellbook.cs @@ -24,6 +24,7 @@ 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(); @@ -104,12 +105,40 @@ public sealed class Spellbook } /// - /// Campaign P Slice P1 (2026-07-30) — combined vitae + skill-enchantment - /// buff modifier for a Skill id (ACE Skill enum ordinal — Run=24, - /// Jump=22), matching retail CEnchantmentRegistry::EnchantSkill - /// (0x005947b0). Mirrors 's caching shape; - /// consumed by AcDream.Runtime.Gameplay.RuntimeCharacterState's - /// run/jump skill recompute. + /// 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) { @@ -380,6 +409,7 @@ public sealed class Spellbook _activeById.Clear(); _enchantmentOrderByBucket.Clear(); _vitalModCache.Clear(); + _attributeModCache.Clear(); _skillModCache.Clear(); foreach (ActiveEnchantmentRecord enchantment in enchantments) { @@ -469,6 +499,7 @@ public sealed class Spellbook _activeById.Clear(); _enchantmentOrderByBucket.Clear(); _vitalModCache.Clear(); + _attributeModCache.Clear(); _skillModCache.Clear(); foreach (List tab in _favoriteSpells) tab.Clear(); _desiredComponents.Clear(); @@ -488,6 +519,7 @@ 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 16e3de9c..4bab2215 100644 --- a/tests/AcDream.App.Tests/UI/Layout/CharacterStatControllerTests.cs +++ b/tests/AcDream.App.Tests/UI/Layout/CharacterStatControllerTests.cs @@ -968,7 +968,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); @@ -1269,6 +1273,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 b9597294..b18a9179 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); @@ -235,7 +379,7 @@ public sealed class EnchantmentMathTests uint spellId, uint layer, uint skillId, float val) => new( spellId, layer, 60f, 0u, - StatModType: EnchantmentMath.EnchantmentTypeFlag.Skill, + StatModType: (uint)EnchantmentMath.EnchantmentTypeFlag.Skill, StatModKey: skillId, StatModValue: val, Bucket: 1u); @@ -244,7 +388,7 @@ public sealed class EnchantmentMathTests uint spellId, uint layer, uint skillId, float val) => new( spellId, layer, 60f, 0u, - StatModType: EnchantmentMath.EnchantmentTypeFlag.Skill, + StatModType: (uint)EnchantmentMath.EnchantmentTypeFlag.Skill, StatModKey: skillId, StatModValue: val, Bucket: 2u); @@ -253,8 +397,8 @@ public sealed class EnchantmentMathTests public void EnchantmentTypeFlag_Skill_MatchesAceEnchantmentTypeFlags() { // ACE.Entity.Enum.EnchantmentTypeFlags.Skill = 0x0000010. - Assert.Equal(0x0000010u, EnchantmentMath.EnchantmentTypeFlag.Skill); - Assert.Equal(0x0000002u, EnchantmentMath.EnchantmentTypeFlag.SecondAtt); + Assert.Equal(0x0000010u, (uint)EnchantmentMath.EnchantmentTypeFlag.Skill); + Assert.Equal(0x0000002u, (uint)EnchantmentMath.EnchantmentTypeFlag.SecondAtt); } [Fact] @@ -304,7 +448,7 @@ public sealed class EnchantmentMathTests { new ActiveEnchantmentRecord( 53u, 1u, 60f, 0u, - StatModType: EnchantmentMath.EnchantmentTypeFlag.SecondAtt, + StatModType: (uint)EnchantmentMath.EnchantmentTypeFlag.SecondAtt, StatModKey: 24u, StatModValue: 1.5f, Bucket: 1u), diff --git a/tests/AcDream.Runtime.Tests/Gameplay/RuntimeCharacterStateTests.cs b/tests/AcDream.Runtime.Tests/Gameplay/RuntimeCharacterStateTests.cs index eba17774..3f2dd06f 100644 --- a/tests/AcDream.Runtime.Tests/Gameplay/RuntimeCharacterStateTests.cs +++ b/tests/AcDream.Runtime.Tests/Gameplay/RuntimeCharacterStateTests.cs @@ -248,7 +248,7 @@ public sealed class RuntimeCharacterStateTests LayerId: 1u, Duration: 60f, CasterGuid: 0u, - StatModType: EnchantmentMath.EnchantmentTypeFlag.Skill, + StatModType: (uint)EnchantmentMath.EnchantmentTypeFlag.Skill, StatModKey: RuntimeCharacterState.RunSkillId, StatModValue: 1.5f, Bucket: 1u));