diff --git a/docs/plans/2026-08-15-character-creation-campaign.md b/docs/plans/2026-08-15-character-creation-campaign.md index 42d8b340..67d93676 100644 --- a/docs/plans/2026-08-15-character-creation-campaign.md +++ b/docs/plans/2026-08-15-character-creation-campaign.md @@ -187,6 +187,34 @@ worktrees). CC4 ∥ CC6a after CC3. CC5 last before CC7. arbiter. 7. `references/*` absent in worktrees (except WorldBuilder, uninitialized submodule) — agents read ACE/holtburger from the MAIN checkout path. +8. **CC7 landmine (found in the CC1 review fix round, 2026-08-15):** ACE's + `PlayerFactory.CreatePlayer` heritage-override branch + (references/ACE/Source/ACE.Server/Factories/PlayerFactory.cs:184-211) + over-deducts skill credits when specializing a skill the active + heritage's own list prices. For a skill priced ONLY by the global + SkillTable, ACE correctly computes the incremental specialize cost via + `SkillBase.UpgradeCostFromTrainedToSpecialized` (= `SpecializedCost - + TrainedCost`) and charges `TrainSkill(trainedCost) + + SpecializeSkill(incrementalCost)` = the field's TOTAL, matching retail. + But when the heritage's own list has an entry, ACE sets + `specializedCost = skillGroup.PrimaryCost` directly — `PrimaryCost` is + already the TOTAL cost to reach Specialized (acdream's own + `ChargenSkillCost.PrimaryCost` convention, confirmed against retail) — + and then still charges `TrainSkill(NormalCost) + + SpecializeSkill(PrimaryCost)`, over-deducting by an extra `NormalCost` + credits versus what retail's client computed and what the player agreed + to spend. Practical impact for CC7's connected gate: a retail-legal + character build that specializes a skill the ACTIVE HERITAGE prices + (every one of the 13 installed heritages has exactly one such skill — + see `ChargenTableReaderInstalledDatTests.InstalledHeritages_SkillCostFallbackCoversTheKnownUncostableSkillSet`) + may be REJECTED by local ACE with `FailedToSpecializeSkill` even though + acdream sent the byte-correct 0xF656 body. If CC7's gate hits this, + it is an ACE-side bug reproduced from its own source, NOT an acdream + wire or math defect — do not "fix" acdream's cost math to match ACE's + over-deduction. Register: file an AD row if CC7 needs a documented + workaround (e.g. picking a Specialized skill combination that avoids + the heritage-priced skill for the connected gate) rather than silently + adjusting acdream's send. ## Review protocol @@ -200,7 +228,7 @@ the user gate. | Slice | Status | Commits | Review | Notes | |---|---|---|---|---| -| CC1 | implemented; review in flight | `04450041` | dispatched 2026-08-15 | Core model (no Chorizite leak) + Content projector; 31 math units + 6 installed-DAT gates (13 heritages). FINDING for CC3: each human heritage's "Adventurer" template IS retail's Custom entry point — attributes at the 10-floor (60/330), a real TemplateCG row, not a UI special case | +| CC1 | implemented; review fix round complete | `04450041`, `459a87f2` | dispatched 2026-08-15 | Core model (no Chorizite leak) + Content projector; 31 math units + 6 installed-DAT gates (13 heritages). FINDING for CC3: each human heritage's "Adventurer" template IS retail's Custom entry point — attributes at the 10-floor (60/330), a real TemplateCG row, not a UI special case. **Review fix round (`459a87f2`):** F1 doc corrected — Custom IS template index 0 (the Adventurer row), per `gmCGProfessionPage::UpdateProfession @ 0x004821b0` (case 0 → button 0x100003d9 / `ID_CharGen_CustomText`) and `CharGenState::SetTemplate @ 0x005C5A60` (commits via `CharGenState::ApplyTemplate @ 0x005C5080`, i.e. selecting Custom resets sliders to the floor spread, it does not bypass templates); F2 two-tier skill-cost fallback implemented (`ChargenOptions.GlobalSkillCostsBySkillId` from portal.dat 0x0E000004, `ChargenSkillCreditMath` checks heritage list then global list) + installed-DAT completeness assertion recording reality: the global SkillTable prices 38/54 advancement skill ids, every one of the 13 heritages ships EXACTLY one heritage-specific override (always also present in the global table), and 16 skill ids are genuinely uncostable in both tiers (retail's -1 case) — see `ChargenTableReaderInstalledDatTests.InstalledHeritages_SkillCostFallbackCoversTheKnownUncostableSkillSet`; F3 every `ChargenTableReader` collection is now frozen at projection (`ToFrozenDictionary`/`ToArray`, matching `MagicCatalog`'s pattern) including both `ChargenOptions.Empty` dictionaries; F4 a reflection guard test (`ChargenNoChoriziteLeakTests`) pins the no-Chorizite-leak contract by walking every public `AcDream.Core.CharGen` member; F5 `HasAnyAppearanceOptions`'s doc reworded to state precisely what it proves (an OR across eight lists, omitting the three color lists) + a new installed-DAT gate records per-list reality — found COMPLETE, every gender of every heritage has non-empty lists across all eight plus the three color lists, even the sparse Gear Knight/Olthoi variants; F6 `TryGetHeritage`/`TryGetStarterArea` annotated `[MaybeNullWhen(false)]` (matching the house `EmptyDatReaderWriter` pattern), all affected call sites (more than the originally estimated five) fixed across both test projects. Filed CC7 risk item 8: ACE's `PlayerFactory` heritage-override branch over-deducts skill credits when specializing a heritage-priced skill (references/ACE/Source/ACE.Server/Factories/PlayerFactory.cs:184-211) — a retail-legal build may be rejected by local ACE at the CC7 connected gate; this is an ACE bug, not an acdream defect. | | CC2 | implemented (campaign-cc2); review in flight | `5eaad2c8` | dispatched 2026-08-15 | Byte-exact 0xF656 (19-term checksum vs CG_Pack accumulator), shared 0xF643 type, correlation latch, status events + contract amendment. Core.Net 993 / Runtime 1667 / Launcher.Core 323, Windows+WSL | | CC3 | — | | | | | CC4 | — | | | | diff --git a/src/AcDream.Content/CharGen/ChargenTableReader.cs b/src/AcDream.Content/CharGen/ChargenTableReader.cs index 4b30a776..7426e539 100644 --- a/src/AcDream.Content/CharGen/ChargenTableReader.cs +++ b/src/AcDream.Content/CharGen/ChargenTableReader.cs @@ -1,22 +1,30 @@ +using System.Collections.Frozen; using AcDream.Core.CharGen; using DatReaderWriter.DBObjs; using DatReaderWriter.Types; using CoreChargenObjDesc = AcDream.Core.CharGen.ChargenObjDesc; using DatCharGen = DatReaderWriter.DBObjs.CharGen; using DatObjDesc = DatReaderWriter.Types.ObjDesc; +using DatSkillTable = DatReaderWriter.DBObjs.SkillTable; namespace AcDream.Content.CharGen; /// /// Projects portal.dat's CharGen table (id , -/// retail ACCharGenData::Serialize @ 0x005C36D0) into acdream's +/// retail ACCharGenData::Serialize @ 0x005C36D0) PLUS the global +/// SkillTable (id ) into acdream's /// presentation-free tree. /// Mirrors MagicCatalog.Load's shape: one static entry point over -/// , no Chorizite types cross into the +/// , every returned collection is frozen at +/// projection (ToFrozenDictionary / ToArray, matching +/// MagicCatalog's pattern), and no Chorizite types cross into the /// returned model. Cross-checked against ACE's /// ACE.DatLoader.FileTypes.CharGen + /// ACE.DatLoader.Entity.HeritageGroupCG/SexCG/TemplateCG loaders, -/// which unpack the identical field order from the same DAT bytes. +/// which unpack the identical field order from the same DAT bytes, and +/// against ACE.DatLoader.FileTypes.SkillTable for the global +/// skill-cost fallback (see +/// ). /// public static class ChargenTableReader { @@ -24,48 +32,83 @@ public static class ChargenTableReader /// ACE.DatLoader.FileTypes.CharGen.FILE_ID). public const uint ChargenTableDid = 0x0E000002u; + /// Retail's global SkillTable DAT file id (ACE: + /// ACE.DatLoader.FileTypes.SkillTable.FILE_ID) — the fallback + /// retail's ACCharGenData::GetSkillTrainedCost @ 0x005C26D0 / + /// GetSkillSpecializedCost @ 0x005C27D0 read via + /// DBCache::GetFromEnumStatic(4, 2, 0x10000004) when a heritage's + /// own skill-cost list has no entry for a skill id. + public const uint SkillTableDid = 0x0E000004u; + /// - /// Loads and projects the installed CharGen table. Returns - /// if the table is missing from the - /// supplied dat source (mirrors MagicCatalog's tolerance for a - /// missing optional table — callers that require the table present - /// should check HeritagesById.Count themselves). + /// Loads and projects the installed CharGen table plus the global + /// SkillTable fallback. Returns if + /// the CharGen table is missing from the supplied dat source (mirrors + /// MagicCatalog's tolerance for a missing optional table — + /// callers that require the table present should check + /// HeritagesById.Count themselves). A missing global SkillTable + /// degrades to an empty fallback dictionary rather than failing the + /// whole load — the per-heritage costs (the common case) still work. /// public static ChargenOptions Load(IDatReaderWriter dats) { ArgumentNullException.ThrowIfNull(dats); DatCharGen? table = dats.Get(ChargenTableDid); - return table is null ? ChargenOptions.Empty : Project(table); + if (table is null) + return ChargenOptions.Empty; + + DatSkillTable? skillTable = dats.Get(SkillTableDid); + return Project(table, skillTable); } /// Pure projection from an already-loaded DAT record — split out /// from so tests can exercise it against - /// hand-built fixtures without a live DAT. - public static ChargenOptions Project(DatCharGen table) + /// hand-built fixtures without a live DAT. + /// is optional (mirrors 's + /// missing-table tolerance) and projects into + /// . + public static ChargenOptions Project(DatCharGen table, DatSkillTable? skillTable = null) { ArgumentNullException.ThrowIfNull(table); - var starterAreas = new List(table.StartingAreas.Count); + var starterAreas = new ChargenStarterArea[table.StartingAreas.Count]; for (int i = 0; i < table.StartingAreas.Count; i++) - starterAreas.Add(ProjectStarterArea(i, table.StartingAreas[i])); + starterAreas[i] = ProjectStarterArea(i, table.StartingAreas[i]); var heritagesById = new Dictionary(table.HeritageGroups.Count); foreach (KeyValuePair pair in table.HeritageGroups) heritagesById[pair.Key] = ProjectHeritage(pair.Key, pair.Value); - return new ChargenOptions(starterAreas, heritagesById); + var globalSkillCosts = new Dictionary(skillTable?.Skills.Count ?? 0); + if (skillTable is not null) + { + foreach (KeyValuePair pair in skillTable.Skills) + { + uint skillId = (uint)pair.Key; + globalSkillCosts[skillId] = new ChargenSkillCost( + skillId, + pair.Value.TrainedCost, + pair.Value.SpecializedCost); + } + } + + return new ChargenOptions( + starterAreas, + heritagesById.ToFrozenDictionary(), + globalSkillCosts.ToFrozenDictionary()); } private static ChargenStarterArea ProjectStarterArea(int index, StartingArea area) { - var locations = new List(area.Locations.Count); - foreach (Position position in area.Locations) + var locations = new ChargenPosition[area.Locations.Count]; + for (int i = 0; i < area.Locations.Count; i++) { - locations.Add(new ChargenPosition( + Position position = area.Locations[i]; + locations[i] = new ChargenPosition( position.CellId, position.Frame.Origin, - position.Frame.Orientation)); + position.Frame.Orientation); } return new ChargenStarterArea(index, area.Name.Value, locations); } @@ -79,9 +122,9 @@ public static class ChargenTableReader skillCosts[skillId] = new ChargenSkillCost(skillId, skill.NormalCost, skill.PrimaryCost); } - var templates = new List(cg.Templates.Count); - foreach (TemplateCG template in cg.Templates) - templates.Add(ProjectTemplate(template)); + var templates = new ChargenTemplate[cg.Templates.Count]; + for (int i = 0; i < cg.Templates.Count; i++) + templates[i] = ProjectTemplate(cg.Templates[i]); var gendersByKey = new Dictionary(cg.Genders.Count); foreach (KeyValuePair pair in cg.Genders) @@ -95,22 +138,22 @@ public static class ChargenTableReader cg.EnvironmentSetupId.DataId, cg.AttributeCredits, cg.SkillCredits, - new List(cg.PrimaryStartAreas), - new List(cg.SecondaryStartAreas), - skillCosts, + cg.PrimaryStartAreas.ToArray(), + cg.SecondaryStartAreas.ToArray(), + skillCosts.ToFrozenDictionary(), templates, - gendersByKey); + gendersByKey.ToFrozenDictionary()); } private static ChargenTemplate ProjectTemplate(TemplateCG template) { - var normalSkills = new List(template.NormalSkills.Count); - foreach (var skillId in template.NormalSkills) - normalSkills.Add((uint)skillId); + var normalSkills = new uint[template.NormalSkills.Count]; + for (int i = 0; i < template.NormalSkills.Count; i++) + normalSkills[i] = (uint)template.NormalSkills[i]; - var primarySkills = new List(template.PrimarySkills.Count); - foreach (var skillId in template.PrimarySkills) - primarySkills.Add((uint)skillId); + var primarySkills = new uint[template.PrimarySkills.Count]; + for (int i = 0; i < template.PrimarySkills.Count; i++) + primarySkills[i] = (uint)template.PrimarySkills[i]; return new ChargenTemplate( template.Name.Value, @@ -129,33 +172,41 @@ public static class ChargenTableReader private static ChargenGenderOptions ProjectGender(int genderKey, SexCG sex) { - var hairStyles = new List(sex.HairStyles.Count); - foreach (HairStyleCG hair in sex.HairStyles) + var hairStyles = new ChargenHairStyle[sex.HairStyles.Count]; + for (int i = 0; i < sex.HairStyles.Count; i++) { - hairStyles.Add(new ChargenHairStyle( + HairStyleCG hair = sex.HairStyles[i]; + hairStyles[i] = new ChargenHairStyle( hair.IconId.DataId, hair.Bald, hair.AlternateSetup, - ProjectObjDesc(hair.ObjDesc))); + ProjectObjDesc(hair.ObjDesc)); } - var eyeStrips = new List(sex.EyeStrips.Count); - foreach (EyeStripCG eye in sex.EyeStrips) + var eyeStrips = new ChargenEyeStrip[sex.EyeStrips.Count]; + for (int i = 0; i < sex.EyeStrips.Count; i++) { - eyeStrips.Add(new ChargenEyeStrip( + EyeStripCG eye = sex.EyeStrips[i]; + eyeStrips[i] = new ChargenEyeStrip( eye.IconId.DataId, eye.BaldIconId, ProjectObjDesc(eye.ObjDesc), - ProjectObjDesc(eye.BaldObjDesc))); + ProjectObjDesc(eye.BaldObjDesc)); } - var noseStrips = new List(sex.NoseStrips.Count); - foreach (FaceStripCG strip in sex.NoseStrips) - noseStrips.Add(new ChargenFaceStrip(strip.IconId.DataId, ProjectObjDesc(strip.ObjDesc))); + var noseStrips = new ChargenFaceStrip[sex.NoseStrips.Count]; + for (int i = 0; i < sex.NoseStrips.Count; i++) + { + FaceStripCG strip = sex.NoseStrips[i]; + noseStrips[i] = new ChargenFaceStrip(strip.IconId.DataId, ProjectObjDesc(strip.ObjDesc)); + } - var mouthStrips = new List(sex.MouthStrips.Count); - foreach (FaceStripCG strip in sex.MouthStrips) - mouthStrips.Add(new ChargenFaceStrip(strip.IconId.DataId, ProjectObjDesc(strip.ObjDesc))); + var mouthStrips = new ChargenFaceStrip[sex.MouthStrips.Count]; + for (int i = 0; i < sex.MouthStrips.Count; i++) + { + FaceStripCG strip = sex.MouthStrips[i]; + mouthStrips[i] = new ChargenFaceStrip(strip.IconId.DataId, ProjectObjDesc(strip.ObjDesc)); + } return new ChargenGenderOptions( genderKey, @@ -170,9 +221,9 @@ public static class ChargenTableReader sex.MotionTable.DataId, sex.CombatTable.DataId, ProjectObjDesc(sex.BaseObjDesc), - new List(sex.HairColors), + sex.HairColors.ToArray(), hairStyles, - new List(sex.EyeColors), + sex.EyeColors.ToArray(), eyeStrips, noseStrips, mouthStrips, @@ -180,35 +231,45 @@ public static class ChargenTableReader ProjectGearList(sex.Shirts), ProjectGearList(sex.Pants), ProjectGearList(sex.Footwear), - new List(sex.ClothingColors)); + sex.ClothingColors.ToArray()); } - private static List ProjectGearList(List gearList) + private static ChargenGearOption[] ProjectGearList(List gearList) { - var result = new List(gearList.Count); - foreach (GearCG gear in gearList) - result.Add(new ChargenGearOption(gear.Name.Value, gear.ClothingTable.DataId, gear.WeenieDefault)); + var result = new ChargenGearOption[gearList.Count]; + for (int i = 0; i < gearList.Count; i++) + { + GearCG gear = gearList[i]; + result[i] = new ChargenGearOption(gear.Name.Value, gear.ClothingTable.DataId, gear.WeenieDefault); + } return result; } private static CoreChargenObjDesc ProjectObjDesc(DatObjDesc objDesc) { - var subPalettes = new List(objDesc.SubPalettes.Count); - foreach (SubPalette sub in objDesc.SubPalettes) - subPalettes.Add(new ChargenSubPalette(sub.SubId.DataId, sub.Offset, sub.NumColors)); - - var textureChanges = new List(objDesc.TextureChanges.Count); - foreach (TextureMapChange change in objDesc.TextureChanges) + var subPalettes = new ChargenSubPalette[objDesc.SubPalettes.Count]; + for (int i = 0; i < objDesc.SubPalettes.Count; i++) { - textureChanges.Add(new ChargenTextureChange( - change.PartIndex, - change.OldTexture.DataId, - change.NewTexture.DataId)); + SubPalette sub = objDesc.SubPalettes[i]; + subPalettes[i] = new ChargenSubPalette(sub.SubId.DataId, sub.Offset, sub.NumColors); } - var animPartChanges = new List(objDesc.AnimPartChanges.Count); - foreach (AnimationPartChange change in objDesc.AnimPartChanges) - animPartChanges.Add(new ChargenAnimPartChange(change.PartIndex, change.PartId.DataId)); + var textureChanges = new ChargenTextureChange[objDesc.TextureChanges.Count]; + for (int i = 0; i < objDesc.TextureChanges.Count; i++) + { + TextureMapChange change = objDesc.TextureChanges[i]; + textureChanges[i] = new ChargenTextureChange( + change.PartIndex, + change.OldTexture.DataId, + change.NewTexture.DataId); + } + + var animPartChanges = new ChargenAnimPartChange[objDesc.AnimPartChanges.Count]; + for (int i = 0; i < objDesc.AnimPartChanges.Count; i++) + { + AnimationPartChange change = objDesc.AnimPartChanges[i]; + animPartChanges[i] = new ChargenAnimPartChange(change.PartIndex, change.PartId.DataId); + } return new CoreChargenObjDesc(objDesc.PaletteId.DataId, subPalettes, textureChanges, animPartChanges); } diff --git a/src/AcDream.Core/CharGen/ChargenGenderOptions.cs b/src/AcDream.Core/CharGen/ChargenGenderOptions.cs index 0ee4b2e0..da79e60c 100644 --- a/src/AcDream.Core/CharGen/ChargenGenderOptions.cs +++ b/src/AcDream.Core/CharGen/ChargenGenderOptions.cs @@ -36,11 +36,22 @@ public sealed record ChargenGenderOptions( IReadOnlyList ClothingColors) { /// - /// Every appearance option list is non-empty for a playable gender — - /// CC1's installed-DAT gate asserts this holds for at least one gender - /// per heritage. A gender missing an option list can still be a valid - /// data shape (e.g. a bald-only heritage's hair styles), so callers - /// building UI should still defend against empty lists individually. + /// True when AT LEAST ONE of the eight lists below is non-empty (an OR + /// across all eight, not a per-list guarantee). Deliberately omits + /// , , and + /// — CC6's color-wheel controls need those + /// three independently of this property and must check them + /// separately. CC1's installed-DAT gate + /// (ChargenTableReaderInstalledDatTests.InstalledHeritages_EachHasAtLeastOneGenderWithNonEmptyAppearanceOptions) + /// only proves "at least one gender per heritage has at least one + /// non-empty list among these eight" — it does NOT prove every list is + /// non-empty for every gender of every heritage, and it does not cover + /// the three color lists at all; see + /// ChargenTableReaderInstalledDatTests.InstalledHeritages_AppearanceOptionListsRecordedPerListCompleteness + /// for the per-list installed-DAT reality. A gender missing an + /// individual option list can still be a valid data shape (e.g. a + /// bald-only heritage's hair styles), so callers building UI must still + /// defend against empty lists individually. /// public bool HasAnyAppearanceOptions => HairStyles.Count > 0 diff --git a/src/AcDream.Core/CharGen/ChargenOptions.cs b/src/AcDream.Core/CharGen/ChargenOptions.cs index e68555a7..218479f1 100644 --- a/src/AcDream.Core/CharGen/ChargenOptions.cs +++ b/src/AcDream.Core/CharGen/ChargenOptions.cs @@ -1,34 +1,52 @@ +using System.Collections.Frozen; +using System.Diagnostics.CodeAnalysis; + namespace AcDream.Core.CharGen; /// /// Top-level, presentation-free, immutable projection of retail's CharGen /// DAT table (portal.dat 0x0E000002, ACCharGenData::Serialize @ -/// 0x005C36D0). Production builds create this from the installed DAT -/// through Content's AcDream.Content.CharGen.ChargenTableReader.Load; -/// this type itself has no DAT/Chorizite dependency so it is safe to hand -/// to plugin-facing or test code. Everything a "typed chargen options -/// model" needs — starter areas, heritages, templates, per-gender -/// appearance option lists, skill costs — hangs off this one root. +/// 0x005C36D0) PLUS the global SkillTable (portal.dat 0x0E000004) that +/// retail falls back to when a heritage's own skill-cost list has no entry +/// for a given skill id. Retail's ACCharGenData::GetSkillTrainedCost @ +/// 0x005C26D0 / GetSkillSpecializedCost @ 0x005C27D0 both scan +/// the heritage's own list first and, on a miss (or an empty list), fall +/// through to DBCache::GetFromEnumStatic(4, 2, 0x10000004) — the +/// SAME global SkillTable every other skill-cost lookup in the client +/// reads — rather than treating the skill as free or invalid. See +/// . Production builds create this +/// from the installed DAT through Content's +/// AcDream.Content.CharGen.ChargenTableReader.Load; this type itself +/// has no DAT/Chorizite dependency so it is safe to hand to plugin-facing +/// or test code. Every collection is frozen/immutable at construction (a +/// caller cannot downcast an +/// back to a mutable and mutate this +/// process-shared model out from under other readers). Everything a "typed +/// chargen options model" needs — starter areas, heritages, templates, +/// per-gender appearance option lists, skill costs — hangs off this one +/// root. /// public sealed record ChargenOptions( IReadOnlyList StarterAreas, - IReadOnlyDictionary HeritagesById) + IReadOnlyDictionary HeritagesById, + IReadOnlyDictionary GlobalSkillCostsBySkillId) { public static ChargenOptions Empty { get; } = new( Array.Empty(), - new Dictionary()); + FrozenDictionary.Empty, + FrozenDictionary.Empty); - public bool TryGetHeritage(uint heritageId, out ChargenHeritageOptions heritage) => - HeritagesById.TryGetValue(heritageId, out heritage!); + public bool TryGetHeritage(uint heritageId, [MaybeNullWhen(false)] out ChargenHeritageOptions heritage) => + HeritagesById.TryGetValue(heritageId, out heritage); - public bool TryGetStarterArea(int index, out ChargenStarterArea area) + public bool TryGetStarterArea(int index, [MaybeNullWhen(false)] out ChargenStarterArea area) { if (index >= 0 && index < StarterAreas.Count) { area = StarterAreas[index]; return true; } - area = null!; + area = default; return false; } } diff --git a/src/AcDream.Core/CharGen/ChargenSkillCreditMath.cs b/src/AcDream.Core/CharGen/ChargenSkillCreditMath.cs index 2a8ab4fa..019c07e7 100644 --- a/src/AcDream.Core/CharGen/ChargenSkillCreditMath.cs +++ b/src/AcDream.Core/CharGen/ChargenSkillCreditMath.cs @@ -7,23 +7,35 @@ namespace AcDream.Core.CharGen; /// for Specialized (never both), and subtract the total from the heritage's /// SkillCredits budget. No DAT/DatReaderWriter dependency — callers /// (CC3's Runtime owner) pass in the heritage's already-projected -/// lookup. +/// lookup plus the global SkillTable +/// fallback lookup (). /// public static class ChargenSkillCreditMath { /// /// Total credits spent across every Trained/Specialized skill in - /// . A skill with no cost entry for the - /// active heritage (i.e. the heritage doesn't offer it) is skipped — - /// retail's own UI can never reach that state, so this is defensive - /// rather than a documented retail behavior. + /// . Retail's cost lookup + /// (ACCharGenData::GetSkillTrainedCost @ 0x005C26D0 / + /// GetSkillSpecializedCost @ 0x005C27D0) is TWO-TIERED: it scans + /// the active heritage's own list + /// first, and only on a miss falls through to the global SkillTable + /// (, portal.dat 0x0E000004 via + /// DBCache::GetFromEnumStatic(4, 2, 0x10000004)). A skill id + /// missing from BOTH tiers is retail's -1/"no cost" case; this port + /// treats that as uncostable and skips it (mirrors ACE's identical + /// precedence in + /// references/ACE/Source/ACE.Server/Factories/PlayerFactory.cs:184-196, + /// which seeds from SkillTable.SkillBaseHash[i] and then applies + /// a heritage override). /// public static int ComputeSpent( ChargenSkillAdvancementSet advancement, - IReadOnlyDictionary costsBySkillId) + IReadOnlyDictionary costsBySkillId, + IReadOnlyDictionary globalCostsBySkillId) { ArgumentNullException.ThrowIfNull(advancement); ArgumentNullException.ThrowIfNull(costsBySkillId); + ArgumentNullException.ThrowIfNull(globalCostsBySkillId); int spent = 0; for (uint skillId = 1; skillId < ChargenSkillAdvancementSet.SlotCount; skillId++) @@ -35,8 +47,11 @@ public static class ChargenSkillCreditMath continue; } - if (!costsBySkillId.TryGetValue(skillId, out ChargenSkillCost cost)) + if (!costsBySkillId.TryGetValue(skillId, out ChargenSkillCost cost) + && !globalCostsBySkillId.TryGetValue(skillId, out cost)) + { continue; + } spent += cls == ChargenSkillAdvancementClass.Specialized ? cost.PrimaryCost @@ -57,6 +72,7 @@ public static class ChargenSkillCreditMath public static int RemainingCredits( uint totalSkillCredits, ChargenSkillAdvancementSet advancement, - IReadOnlyDictionary costsBySkillId) => - checked((int)totalSkillCredits) - ComputeSpent(advancement, costsBySkillId); + IReadOnlyDictionary costsBySkillId, + IReadOnlyDictionary globalCostsBySkillId) => + checked((int)totalSkillCredits) - ComputeSpent(advancement, costsBySkillId, globalCostsBySkillId); } diff --git a/src/AcDream.Core/CharGen/ChargenTemplate.cs b/src/AcDream.Core/CharGen/ChargenTemplate.cs index 0bdc5ceb..484b12a4 100644 --- a/src/AcDream.Core/CharGen/ChargenTemplate.cs +++ b/src/AcDream.Core/CharGen/ChargenTemplate.cs @@ -1,14 +1,36 @@ namespace AcDream.Core.CharGen; /// -/// One profession preset (Bowhunter, Swashbuckler, Lifecaster, Warmage, -/// Wayfarer, Soldier, ...) offered on the Profession page -/// (UpdateProfession @ 0x00478d1c/0x0047a4a4-adjacent per the -/// campaign plan's recon; template buttons 0x100003da..df). "Custom" is NOT -/// one of these — it is retail's own free-attribute-assignment mode -/// selected by button 0x100003d9 and has no -/// entry. Retail schema: Template_CG::Serialize @ 0x005C0450 -/// (ACE's TemplateCG.Unpack mirrors the same field order). +/// One profession preset offered on the Profession page. Retail's +/// gmCGProfessionPage::UpdateProfession @ 0x004821b0 switches on +/// CharGenState.template_ and resolves BOTH the button to highlight +/// AND the description string from the SAME 0..6 index: 0 → button +/// 0x100003d9 / ID_CharGen_CustomText ("Custom"), 1 → 0x100003da +/// (Bow Hunter), 2 → 0x100003df (Swashbuckler), 3 → 0x100003db (Life +/// Caster), 4 → 0x100003dc (War Caster), 5 → 0x100003dd (Wayfarer), 6 → +/// 0x100003de (Soldier). "Custom" is therefore template index 0, NOT a +/// special UI-only mode with no data — it is a real +/// entry (confirmed against the installed DAT: each human heritage ships +/// this row as "Adventurer", sitting at the attribute floor rather than +/// spending the full credit budget). The seven profession buttons all wire +/// to CharGenState::SetTemplate(state, N, 1) @ 0x005C5A60 (N = +/// 0..6, the second arg a "commit" flag); SetTemplate writes +/// template_ = N and, because N != 0xffffffff (retail's +/// no-template sentinel — never sent by any button), immediately calls +/// CharGenState::ApplyTemplate @ 0x005C5080, which re-reads that +/// template row's six attributes and skill list and re-applies them. +/// Selecting "Custom" therefore RESETS the attribute sliders and skill +/// picks to the Adventurer row's floor spread rather than leaving the +/// current values untouched — retail's own Custom-button handler +/// (gmCGProfessionPage::ListenToElementMessage case 0xed) calls +/// SetTemplate(state, 0, 1) then gmCGProfessionPage::UpdateToDefaultAttributes +/// @ 0x00482860 to refresh the slider UI to match. ACE's +/// PlayerFactory.CreatePlayer confirms the same indexing server-side: +/// it indexes heritageGroup.Templates[characterCreateInfo.TemplateOption] +/// with no special-cased "no template" branch +/// (references/ACE/Source/ACE.Server/Factories/PlayerFactory.cs:135-138). +/// Retail schema: Template_CG::Serialize @ 0x005C0450 (ACE's +/// TemplateCG.Unpack mirrors the same field order). /// public sealed record ChargenTemplate( string Name, diff --git a/tests/AcDream.Content.Tests/CharGen/ChargenTableReaderInstalledDatTests.cs b/tests/AcDream.Content.Tests/CharGen/ChargenTableReaderInstalledDatTests.cs index 9c8f4f08..0753ef07 100644 --- a/tests/AcDream.Content.Tests/CharGen/ChargenTableReaderInstalledDatTests.cs +++ b/tests/AcDream.Content.Tests/CharGen/ChargenTableReaderInstalledDatTests.cs @@ -59,22 +59,22 @@ public sealed class ChargenTableReaderInstalledDatTests using var adapter = new DatCollectionAdapter(dats); ChargenOptions options = ChargenTableReader.Load(adapter); - Assert.True(options.TryGetHeritage(AluvianId, out ChargenHeritageOptions aluvian)); - Assert.Equal("Aluvian", aluvian.Name); + Assert.True(options.TryGetHeritage(AluvianId, out ChargenHeritageOptions? aluvian)); + Assert.Equal("Aluvian", aluvian!.Name); - Assert.True(options.TryGetHeritage(GharundimId, out ChargenHeritageOptions gharundim)); - Assert.Equal("Gharu'ndim", gharundim.Name); + Assert.True(options.TryGetHeritage(GharundimId, out ChargenHeritageOptions? gharundim)); + Assert.Equal("Gharu'ndim", gharundim!.Name); - Assert.True(options.TryGetHeritage(ShoId, out ChargenHeritageOptions sho)); - Assert.Equal("Sho", sho.Name); + Assert.True(options.TryGetHeritage(ShoId, out ChargenHeritageOptions? sho)); + Assert.Equal("Sho", sho!.Name); - Assert.True(options.TryGetHeritage(ViamontianId, out ChargenHeritageOptions viamontian)); - Assert.Equal("Viamontian", viamontian.Name); + Assert.True(options.TryGetHeritage(ViamontianId, out ChargenHeritageOptions? viamontian)); + Assert.Equal("Viamontian", viamontian!.Name); - Assert.True(options.TryGetHeritage(OlthoiId, out ChargenHeritageOptions olthoi)); - Assert.True(olthoi.IsOlthoi); - Assert.True(options.TryGetHeritage(OlthoiAcidId, out ChargenHeritageOptions olthoiAcid)); - Assert.True(olthoiAcid.IsOlthoi); + Assert.True(options.TryGetHeritage(OlthoiId, out ChargenHeritageOptions? olthoi)); + Assert.True(olthoi!.IsOlthoi); + Assert.True(options.TryGetHeritage(OlthoiAcidId, out ChargenHeritageOptions? olthoiAcid)); + Assert.True(olthoiAcid!.IsOlthoi); } [Fact] @@ -178,9 +178,9 @@ public sealed class ChargenTableReaderInstalledDatTests { indicesChecked++; Assert.True( - options.TryGetStarterArea(index, out ChargenStarterArea area), + options.TryGetStarterArea(index, out ChargenStarterArea? area), $"{heritage.Name}: start-area index {index} does not resolve into the shared StarterAreas list."); - Assert.False(string.IsNullOrEmpty(area.Name)); + Assert.False(string.IsNullOrEmpty(area!.Name)); } } @@ -201,8 +201,8 @@ public sealed class ChargenTableReaderInstalledDatTests using var adapter = new DatCollectionAdapter(dats); ChargenOptions options = ChargenTableReader.Load(adapter); - Assert.True(options.TryGetHeritage(AluvianId, out ChargenHeritageOptions aluvian)); - Assert.NotEmpty(aluvian.SkillCostsBySkillId); + Assert.True(options.TryGetHeritage(AluvianId, out ChargenHeritageOptions? aluvian)); + Assert.NotEmpty(aluvian!.SkillCostsBySkillId); foreach (var pair in aluvian.SkillCostsBySkillId) { Assert.Equal(pair.Key, pair.Value.SkillId); @@ -211,4 +211,134 @@ public sealed class ChargenTableReaderInstalledDatTests Assert.True(pair.Value.PrimaryCost >= 0); } } + + /// + /// F2's completeness assertion: records EXACTLY what the installed DAT's + /// heritage-vs-global skill-cost coverage looks like (found + /// 2026-08-15), so 's two-tier + /// fallback (see ) + /// has a real regression gate instead of only synthetic unit fixtures. + /// Findings: the global SkillTable (portal.dat 0x0E000004) prices 38 of + /// the 54 advancement skill ids; EVERY one of the 13 installed + /// heritages ships EXACTLY one heritage-specific skill-cost override + /// (never zero, never more), and that one override always ALSO has a + /// global entry — no heritage in this DAT relies on a heritage-only + /// price the global table doesn't know about. The remaining 16 skill + /// ids are absent from BOTH tiers in every heritage — retail's genuine + /// -1/"no cost" case (non-advancement or deprecated skill slots such as + /// the pre-Skill-DID-remap gaps). If a future DAT drop changes any of + /// this shape, this test should fail and get updated with the new + /// reality, not be loosened silently. + /// + [Fact] + public void InstalledHeritages_SkillCostFallbackCoversTheKnownUncostableSkillSet() + { + string? datDir = ContentConformanceDats.ResolveDatDir(); + if (datDir is null) + { + Console.WriteLine("SKIP: installed retail DAT directory is unavailable."); + return; + } + + using var dats = new DatCollection(datDir, DatAccessType.Read); + using var adapter = new DatCollectionAdapter(dats); + ChargenOptions options = ChargenTableReader.Load(adapter); + + Assert.Equal(38, options.GlobalSkillCostsBySkillId.Count); + + // Retail's -1/"no cost" case: absent from BOTH the global SkillTable + // AND every heritage's own list in the installed DAT. + uint[] expectedUncostableSkillIds = + [1, 2, 3, 4, 5, 8, 9, 10, 11, 12, 13, 17, 25, 26, 42, 53]; + + Assert.NotEmpty(options.HeritagesById); + foreach (ChargenHeritageOptions heritage in options.HeritagesById.Values) + { + Assert.Single(heritage.SkillCostsBySkillId); + + var uncostable = new List(); + foreach (KeyValuePair pair in heritage.SkillCostsBySkillId) + { + Assert.True( + options.GlobalSkillCostsBySkillId.ContainsKey(pair.Key), + $"{heritage.Name}: skill {pair.Key} is heritage-only with no global fallback entry — " + + "new ground truth found; update this test's recorded reality."); + } + + for (uint skillId = 1; skillId < ChargenSkillAdvancementSet.SlotCount; skillId++) + { + bool inHeritage = heritage.SkillCostsBySkillId.ContainsKey(skillId); + bool inGlobal = options.GlobalSkillCostsBySkillId.ContainsKey(skillId); + if (!inHeritage && !inGlobal) + uncostable.Add(skillId); + } + + Assert.Equal(expectedUncostableSkillIds, uncostable.OrderBy(id => id)); + } + } + + /// + /// F5's strengthened installed-DAT gate. + /// only proves an OR across eight lists for at least one gender per + /// heritage (see + /// above). This test records the STRONGER fact actually found in the + /// installed DAT (2026-08-15): every one of the eight + /// HasAnyAppearanceOptions lists, PLUS the three color lists it + /// deliberately excludes (, + /// , + /// ), is non-empty for + /// EVERY gender of EVERY one of the 13 heritages — even the sparse ones + /// (Gear Knight and both Olthoi variants ship as few as 1-2 entries per + /// list, but never 0). This is a fact about today's data, not a + /// structural guarantee the DAT format enforces — the type's own doc + /// comment still warns callers to defend against an empty list + /// individually, and a future DAT could reintroduce a gap (e.g. a + /// bald-only heritage's hair styles). If this test starts failing on a + /// new DAT drop, that is this gate doing its job, not a reader bug. + /// + [Fact] + public void InstalledHeritages_AppearanceOptionListsRecordedPerListCompleteness() + { + string? datDir = ContentConformanceDats.ResolveDatDir(); + if (datDir is null) + { + Console.WriteLine("SKIP: installed retail DAT directory is unavailable."); + return; + } + + using var dats = new DatCollection(datDir, DatAccessType.Read); + using var adapter = new DatCollectionAdapter(dats); + ChargenOptions options = ChargenTableReader.Load(adapter); + + Assert.NotEmpty(options.HeritagesById); + var emptyLists = new List(); + foreach (ChargenHeritageOptions heritage in options.HeritagesById.Values) + { + foreach (KeyValuePair genderPair in heritage.GendersByKey) + { + ChargenGenderOptions gender = genderPair.Value; + string label = $"{heritage.Name}/{gender.Name} (heritage={heritage.HeritageId}, gender={genderPair.Key})"; + + void RecordIfEmpty(string listName, int count) + { + if (count == 0) + emptyLists.Add($"{label}: {listName}"); + } + + RecordIfEmpty(nameof(gender.HairStyles), gender.HairStyles.Count); + RecordIfEmpty(nameof(gender.EyeStrips), gender.EyeStrips.Count); + RecordIfEmpty(nameof(gender.NoseStrips), gender.NoseStrips.Count); + RecordIfEmpty(nameof(gender.MouthStrips), gender.MouthStrips.Count); + RecordIfEmpty(nameof(gender.Headgears), gender.Headgears.Count); + RecordIfEmpty(nameof(gender.Shirts), gender.Shirts.Count); + RecordIfEmpty(nameof(gender.Pants), gender.Pants.Count); + RecordIfEmpty(nameof(gender.Footwear), gender.Footwear.Count); + RecordIfEmpty(nameof(gender.HairColors), gender.HairColors.Count); + RecordIfEmpty(nameof(gender.EyeColors), gender.EyeColors.Count); + RecordIfEmpty(nameof(gender.ClothingColors), gender.ClothingColors.Count); + } + } + + Assert.Empty(emptyLists); + } } diff --git a/tests/AcDream.Content.Tests/CharGen/ChargenTableReaderTests.cs b/tests/AcDream.Content.Tests/CharGen/ChargenTableReaderTests.cs index e519653a..1804053c 100644 --- a/tests/AcDream.Content.Tests/CharGen/ChargenTableReaderTests.cs +++ b/tests/AcDream.Content.Tests/CharGen/ChargenTableReaderTests.cs @@ -203,8 +203,8 @@ public sealed class ChargenTableReaderTests { ChargenOptions options = ChargenTableReader.Project(BuildFixture()); - Assert.True(options.TryGetHeritage(1u, out ChargenHeritageOptions heritage)); - Assert.Equal("Aluvian", heritage.Name); + Assert.True(options.TryGetHeritage(1u, out ChargenHeritageOptions? heritage)); + Assert.Equal("Aluvian", heritage!.Name); Assert.Equal(0x06000001u, heritage.IconId); Assert.Equal(0x02000010u, heritage.SetupId); Assert.Equal(0x02000020u, heritage.EnvironmentSetupId); @@ -219,10 +219,10 @@ public sealed class ChargenTableReaderTests public void Project_MapsSkillCostsKeyedByRawSkillId() { ChargenOptions options = ChargenTableReader.Project(BuildFixture()); - options.TryGetHeritage(1u, out ChargenHeritageOptions heritage); + Assert.True(options.TryGetHeritage(1u, out ChargenHeritageOptions? heritage)); uint axeId = (uint)DatReaderWriter.Enums.SkillId.Axe; - Assert.True(heritage.SkillCostsBySkillId.TryGetValue(axeId, out ChargenSkillCost cost)); + Assert.True(heritage!.SkillCostsBySkillId.TryGetValue(axeId, out ChargenSkillCost cost)); Assert.Equal(axeId, cost.SkillId); Assert.Equal(4, cost.NormalCost); Assert.Equal(12, cost.PrimaryCost); @@ -232,9 +232,9 @@ public sealed class ChargenTableReaderTests public void Project_MapsTemplateAttributesAndSkillLists() { ChargenOptions options = ChargenTableReader.Project(BuildFixture()); - options.TryGetHeritage(1u, out ChargenHeritageOptions heritage); + Assert.True(options.TryGetHeritage(1u, out ChargenHeritageOptions? heritage)); - ChargenTemplate template = Assert.Single(heritage.Templates); + ChargenTemplate template = Assert.Single(heritage!.Templates); Assert.Equal("Soldier", template.Name); Assert.Equal(42u, template.TitleStringId); Assert.Equal(new ChargenAttributeValues(40, 40, 40, 20, 20, 20), template.Attributes); @@ -247,9 +247,9 @@ public sealed class ChargenTableReaderTests public void Project_MapsGenderScalarsAndOptionLists() { ChargenOptions options = ChargenTableReader.Project(BuildFixture()); - options.TryGetHeritage(1u, out ChargenHeritageOptions heritage); + Assert.True(options.TryGetHeritage(1u, out ChargenHeritageOptions? heritage)); - Assert.True(heritage.GendersByKey.TryGetValue(0, out ChargenGenderOptions? gender)); + Assert.True(heritage!.GendersByKey.TryGetValue(0, out ChargenGenderOptions? gender)); Assert.Equal("Male", gender!.Name); Assert.Equal(1000000u, gender.Scale); Assert.Equal(0x02000030u, gender.SetupId); @@ -288,8 +288,8 @@ public sealed class ChargenTableReaderTests public void Project_MapsObjDescPaletteSubPaletteTextureAndAnimPartChanges() { ChargenOptions options = ChargenTableReader.Project(BuildFixture()); - options.TryGetHeritage(1u, out ChargenHeritageOptions heritage); - heritage.GendersByKey.TryGetValue(0, out ChargenGenderOptions? gender); + Assert.True(options.TryGetHeritage(1u, out ChargenHeritageOptions? heritage)); + heritage!.GendersByKey.TryGetValue(0, out ChargenGenderOptions? gender); ChargenObjDesc baseDesc = gender!.BaseObjDesc; Assert.Equal(0x04000002u, baseDesc.PaletteId); diff --git a/tests/AcDream.Core.Tests/CharGen/ChargenNoChoriziteLeakTests.cs b/tests/AcDream.Core.Tests/CharGen/ChargenNoChoriziteLeakTests.cs new file mode 100644 index 00000000..b055a96a --- /dev/null +++ b/tests/AcDream.Core.Tests/CharGen/ChargenNoChoriziteLeakTests.cs @@ -0,0 +1,150 @@ +using System.Reflection; +using AcDream.Core.CharGen; + +namespace AcDream.Core.Tests.CharGen; + +/// +/// Pins the CC1 no-leak contract with a real assertion rather than a doc +/// comment. AcDream.Core references Chorizite.DatReaderWriter (for +/// TextureHelpers), so "no Chorizite type on any public CharGen +/// member" was convention only until this guard exists — nothing stopped a +/// future edit from putting e.g. a DatReaderWriter.Enums.SkillId +/// directly on a public property. This test walks every public type in the +/// AcDream.Core.CharGen namespace and asserts that no public +/// property, indexer, constructor parameter, or method return/parameter +/// type — nor any of their generic type arguments, recursively — comes +/// from the DatReaderWriter assembly or any assembly whose name +/// starts with Chorizite. +/// +public sealed class ChargenNoChoriziteLeakTests +{ + [Fact] + public void PublicCharGenSurface_NeverExposesChoriziteOrDatReaderWriterTypes() + { + Assembly coreAssembly = typeof(ChargenOptions).Assembly; + Type[] publicCharGenTypes = coreAssembly.GetTypes() + .Where(t => t.IsPublic && t.Namespace == "AcDream.Core.CharGen") + .ToArray(); + + // Guards the guard: if the namespace ever ends up empty (e.g. a + // rename), this test must fail loudly rather than vacuously pass. + Assert.True( + publicCharGenTypes.Length > 5, + $"Expected multiple public types in AcDream.Core.CharGen, found {publicCharGenTypes.Length}. " + + "Did the namespace get renamed or moved?"); + + var offenders = new List(); + + foreach (Type type in publicCharGenTypes) + { + foreach (PropertyInfo property in type.GetProperties( + BindingFlags.Public | BindingFlags.Instance | BindingFlags.Static)) + { + CheckSite(property.PropertyType, $"{type.FullName}.{property.Name} (property)", offenders); + + foreach (ParameterInfo indexParam in property.GetIndexParameters()) + { + CheckSite( + indexParam.ParameterType, + $"{type.FullName}.{property.Name}[{indexParam.Name}] (indexer parameter)", + offenders); + } + } + + foreach (ConstructorInfo ctor in type.GetConstructors( + BindingFlags.Public | BindingFlags.Instance)) + { + foreach (ParameterInfo param in ctor.GetParameters()) + { + CheckSite( + param.ParameterType, + $"{type.FullName}..ctor({param.Name}) (constructor parameter)", + offenders); + } + } + + foreach (MethodInfo method in type.GetMethods( + BindingFlags.Public | BindingFlags.Instance | BindingFlags.Static)) + { + // Property accessors, operators, and other compiler-emitted + // members are IsSpecialName; the property/indexer loop above + // already covers accessor types directly. + if (method.IsSpecialName) + continue; + + CheckSite(method.ReturnType, $"{type.FullName}.{method.Name} (return type)", offenders); + + foreach (ParameterInfo param in method.GetParameters()) + { + CheckSite( + param.ParameterType, + $"{type.FullName}.{method.Name}({param.Name}) (method parameter)", + offenders); + } + } + } + + Assert.True( + offenders.Count == 0, + "A Chorizite/DatReaderWriter type leaked onto a public AcDream.Core.CharGen member:\n" + + string.Join('\n', offenders)); + } + + private static void CheckSite(Type type, string site, List offenders) + { + foreach (Type candidate in FlattenTypeArguments(type)) + { + string? assemblyName = candidate.Assembly.GetName().Name; + if (assemblyName is null) + continue; + + bool isForbidden = + assemblyName.Equals("DatReaderWriter", StringComparison.OrdinalIgnoreCase) + || assemblyName.StartsWith("Chorizite", StringComparison.OrdinalIgnoreCase); + + if (isForbidden) + offenders.Add($"{site}: {candidate.FullName} (assembly '{assemblyName}')"); + } + } + + /// Yields itself plus every generic + /// type argument, array element type, and by-ref (out/ref parameter) + /// element type, recursively — so e.g. out IReadOnlyDictionary + /// <uint, ChargenSkillCost> is checked against + /// ChargenSkillCost, not just the outer dictionary type. + private static IEnumerable FlattenTypeArguments(Type type) + { + yield return type; + + if (type.IsByRef || type.IsPointer) + { + Type? element = type.GetElementType(); + if (element is not null) + { + foreach (Type inner in FlattenTypeArguments(element)) + yield return inner; + } + yield break; + } + + if (type.IsArray) + { + Type? element = type.GetElementType(); + if (element is not null) + { + foreach (Type inner in FlattenTypeArguments(element)) + yield return inner; + } + yield break; + } + + if (type.IsGenericType) + { + foreach (Type argument in type.GetGenericArguments()) + { + foreach (Type inner in FlattenTypeArguments(argument)) + yield return inner; + } + } + } +} diff --git a/tests/AcDream.Core.Tests/CharGen/ChargenOptionsTests.cs b/tests/AcDream.Core.Tests/CharGen/ChargenOptionsTests.cs index 154ea031..c3dd5e2f 100644 --- a/tests/AcDream.Core.Tests/CharGen/ChargenOptionsTests.cs +++ b/tests/AcDream.Core.Tests/CharGen/ChargenOptionsTests.cs @@ -37,9 +37,10 @@ public sealed class ChargenOptionsTests var aluvian = MakeHeritage(1u, "Aluvian"); var options = new ChargenOptions( [], - new Dictionary { [1u] = aluvian }); + new Dictionary { [1u] = aluvian }, + new Dictionary()); - Assert.True(options.TryGetHeritage(1u, out ChargenHeritageOptions found)); + Assert.True(options.TryGetHeritage(1u, out ChargenHeritageOptions? found)); Assert.Same(aluvian, found); } @@ -55,9 +56,12 @@ public sealed class ChargenOptionsTests public void TryGetStarterArea_ResolvesByIndexAndRejectsOutOfRange() { var area = new ChargenStarterArea(0, "Holtburg", []); - var options = new ChargenOptions([area], new Dictionary()); + var options = new ChargenOptions( + [area], + new Dictionary(), + new Dictionary()); - Assert.True(options.TryGetStarterArea(0, out ChargenStarterArea found)); + Assert.True(options.TryGetStarterArea(0, out ChargenStarterArea? found)); Assert.Same(area, found); Assert.False(options.TryGetStarterArea(1, out _)); Assert.False(options.TryGetStarterArea(-1, out _)); diff --git a/tests/AcDream.Core.Tests/CharGen/ChargenSkillCreditMathTests.cs b/tests/AcDream.Core.Tests/CharGen/ChargenSkillCreditMathTests.cs index 1ae40564..5c20aa1b 100644 --- a/tests/AcDream.Core.Tests/CharGen/ChargenSkillCreditMathTests.cs +++ b/tests/AcDream.Core.Tests/CharGen/ChargenSkillCreditMathTests.cs @@ -5,7 +5,9 @@ namespace AcDream.Core.Tests.CharGen; /// /// Tests for — retail's skill-credit /// spend port (CharGenState::UpdateRemainingSkillCredits @ -/// 0x005C37C0). +/// 0x005C37C0), including the two-tier heritage/global cost lookup +/// (ACCharGenData::GetSkillTrainedCost @ 0x005C26D0 / +/// GetSkillSpecializedCost @ 0x005C27D0). /// public sealed class ChargenSkillCreditMathTests { @@ -17,6 +19,10 @@ public sealed class ChargenSkillCreditMathTests // Deliberately no entry for skill id 2 (Bow) — heritage doesn't offer it. }; + /// Empty global fallback — tests that only exercise the + /// heritage tier pass this so a miss is a genuine both-tiers miss. + private static readonly Dictionary NoGlobalCosts = new(); + [Fact] public void ComputeSpent_IgnoresInactiveAndUntrainedSkills() { @@ -26,7 +32,7 @@ public sealed class ChargenSkillCreditMathTests [11u] = ChargenSkillAdvancementClass.Untrained, }; - Assert.Equal(0, ChargenSkillCreditMath.ComputeSpent(advancement, Costs)); + Assert.Equal(0, ChargenSkillCreditMath.ComputeSpent(advancement, Costs, NoGlobalCosts)); } [Fact] @@ -34,7 +40,7 @@ public sealed class ChargenSkillCreditMathTests { var advancement = new ChargenSkillAdvancementSet { [1u] = ChargenSkillAdvancementClass.Trained }; - Assert.Equal(4, ChargenSkillCreditMath.ComputeSpent(advancement, Costs)); + Assert.Equal(4, ChargenSkillCreditMath.ComputeSpent(advancement, Costs, NoGlobalCosts)); } [Fact] @@ -44,7 +50,7 @@ public sealed class ChargenSkillCreditMathTests // both — PrimaryCost is the TOTAL cost to reach Specialized. var advancement = new ChargenSkillAdvancementSet { [1u] = ChargenSkillAdvancementClass.Specialized }; - Assert.Equal(12, ChargenSkillCreditMath.ComputeSpent(advancement, Costs)); + Assert.Equal(12, ChargenSkillCreditMath.ComputeSpent(advancement, Costs, NoGlobalCosts)); } [Fact] @@ -57,15 +63,67 @@ public sealed class ChargenSkillCreditMathTests [24u] = ChargenSkillAdvancementClass.Trained, // 1 }; - Assert.Equal(17, ChargenSkillCreditMath.ComputeSpent(advancement, Costs)); + Assert.Equal(17, ChargenSkillCreditMath.ComputeSpent(advancement, Costs, NoGlobalCosts)); } [Fact] - public void ComputeSpent_SkillWithNoCostEntryIsSkippedDefensively() + public void ComputeSpent_SkillWithNoCostEntryInEitherTierIsSkipped() { + // Retail's -1/"no cost" case: absent from BOTH the heritage list AND + // the global SkillTable (ACCharGenData::GetSkillTrainedCost @ + // 0x005C26D0 returns 0xffffffff when even the global lookup misses). var advancement = new ChargenSkillAdvancementSet { [2u] = ChargenSkillAdvancementClass.Trained }; - Assert.Equal(0, ChargenSkillCreditMath.ComputeSpent(advancement, Costs)); + Assert.Equal(0, ChargenSkillCreditMath.ComputeSpent(advancement, Costs, NoGlobalCosts)); + } + + [Fact] + public void ComputeSpent_HeritageCostWinsOverGlobalCostWhenBothPresent() + { + // Skill id 1 (Axe) is priced differently by the heritage list and the + // global SkillTable — retail's lookup checks the heritage's own list + // FIRST and never consults the global table when the heritage + // provides its own entry. + var globalCosts = new Dictionary + { + [1u] = new ChargenSkillCost(1u, NormalCost: 999, PrimaryCost: 999), + }; + var advancement = new ChargenSkillAdvancementSet { [1u] = ChargenSkillAdvancementClass.Trained }; + + Assert.Equal(4, ChargenSkillCreditMath.ComputeSpent(advancement, Costs, globalCosts)); + } + + [Fact] + public void ComputeSpent_FallsBackToGlobalCostWhenHeritageListHasNoEntry() + { + // Skill id 2 (Bow) is absent from the heritage's own list but present + // in the global SkillTable fallback — retail charges the global cost + // rather than treating the skill as free. + var globalCosts = new Dictionary + { + [2u] = new ChargenSkillCost(2u, NormalCost: 6, PrimaryCost: 18), + }; + var advancement = new ChargenSkillAdvancementSet + { + [2u] = ChargenSkillAdvancementClass.Trained, + }; + + Assert.Equal(6, ChargenSkillCreditMath.ComputeSpent(advancement, Costs, globalCosts)); + } + + [Fact] + public void ComputeSpent_FallsBackToGlobalCostForSpecializedSkillsToo() + { + var globalCosts = new Dictionary + { + [2u] = new ChargenSkillCost(2u, NormalCost: 6, PrimaryCost: 18), + }; + var advancement = new ChargenSkillAdvancementSet + { + [2u] = ChargenSkillAdvancementClass.Specialized, + }; + + Assert.Equal(18, ChargenSkillCreditMath.ComputeSpent(advancement, Costs, globalCosts)); } [Fact] @@ -77,10 +135,10 @@ public sealed class ChargenSkillCreditMathTests [11u] = ChargenSkillAdvancementClass.Specialized, }; - Assert.Equal(84, ChargenSkillCreditMath.RemainingCredits(100u, advancement, Costs)); + Assert.Equal(84, ChargenSkillCreditMath.RemainingCredits(100u, advancement, Costs, NoGlobalCosts)); // Retail's Finish gate never checks remainingSkillCredits, so // overspending relative to the (small, synthetic) budget below is a // representable state, not a thrown exception. - Assert.Equal(-16, ChargenSkillCreditMath.RemainingCredits(0u, advancement, Costs)); + Assert.Equal(-16, ChargenSkillCreditMath.RemainingCredits(0u, advancement, Costs, NoGlobalCosts)); } }