[Caeldeth]

Summary

This PR adds Trait as a root type (HXML-28, slice 1 of HS-1517). A trait is a passive property of a character, such as a background or a species passive. This slice adds the schema, the generated model and the loader. The server subsystem and the content are later slices.

<Trait Name="Dockworker" Icon="112" IconSet="Skill" Category="Background">
  <Description>Years on the docks have made you unusually strong.</Description>
  <ExclusiveGroup Policy="Replace">Background</ExclusiveGroup>
  <ConflictsWith>
    <Trait>Scribe</Trait>
  </ConflictsWith>
  <StatModifiers BonusStr="3" />
</Trait>

This PR also fixes XmlManagerTests.VariantExists, which fails on main. ceridwen merged its numbered variant groups, so enchant1 is now enchant. The fix is its own commit.

Schema

• Name is the key (PrimaryKey => Name), as for statuses. Characters save traits by name, so a renamed trait needs a migration.
• Icon is a number (xs:unsignedShort), and IconSet (Skill or Spell, required) tells which icon sheet the number is in. Traits use the castable icon sheets. A castable's book selects its sheet, but a trait has no book, so it names the sheet.
• Category is for display and grouping. ExclusiveGroup is a mechanical rule: a character has at most one trait from each group. They can have the same value, but neither sets the other. Category is for display only: the store does not index traits by category, so a consumer filters the loaded traits.
• The Policy attribute of ExclusiveGroup is Reject (the new trait is not added) or Replace (the new trait removes the one in the group). The default is Reject, so a missing policy never removes a trait that a character has. The policy is on the group, so a policy without a group cannot exist. The incoming trait's policy decides, so two traits in one group can have different policies.
• ConflictsWith names other traits. A conflict always rejects the incoming trait, and it applies in both directions, so content lists it on one of the two traits. A trait that replaces another uses an ExclusiveGroup with Policy="Replace". An xs:unique rule makes sure that ConflictsWith names each trait once.
• StatModifiers is the existing type.
• A trait has no intents, book, cost, cooldown or targeting. The schema rejects these fields. Content that a player uses is a castable.

Model

• Trait implements ILoadOnStart<Trait> and loads from the traits directory of the world data. The lower-case name is a secondary key, as for statuses.
• Two new enums, ExclusivePolicy and IconSet, are pinned in tools/enum-values.json.
• Trait implements IAdditionalValidation. The manager runs Validate on each trait at load, and the result goes into the validation summary. Validate reports a ConflictsWith name that is not a loaded trait or that occurs twice, a trait that names itself, an ExclusiveGroup with no name, and a Name or group name that is empty or has a space at the start or the end. Names are keys and the loader does not trim them, so Validate rejects a padded name instead of trimming it. The schema forbids several of these, but the loader does not check files against the schema (HXML-35).

Generated code

tools/regenerate.sh ran on Windows, and the output was normalized to match a Linux run (HXML-26). The header of each generated file lists the schema files, so a new schema changes one header line in every existing generated file (250 files). No other line in an existing generated file changes.

Tests

216 pass (32 new). The trait tests cover the schema (a full trait, a trait with no optional blocks, the required icon attributes, the icon range, the Policy values, an empty Policy, a group with no name, a repeated ConflictsWith name, and the rejected castable fields), deserialization and its defaults (a group without a policy gives Reject), and a load from a traits directory that also has a malformed file, with a lookup by the lower-case name. ValidateAll and Validate tests cover a correct catalog, each catalog error, padded names, and a conflict named twice.

Not in this PR

• The package version stays 1.0.0-alpha2. The version is the maintainers' choice.
• Effects other than stats (conditional modifiers, status resistance, procs, capabilities, granted castables) are a later design.
• The server subsystem and the content.
• The loader does not check files against the schema, for any type, so a trait with no IconSet loads as Skill (HXML-35).

[codecov]

Codecov Report

❌ Patch coverage is 96.96970% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 69.27%. Comparing base (bc6166d) to head (2b8c778).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
src/Extensions/Trait.cs	96.96%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #12      +/-   ##
==========================================
+ Coverage   68.03%   69.27%   +1.23%     
==========================================
  Files          43       44       +1     
  Lines        1627     1660      +33     
  Branches      385      396      +11     
==========================================
+ Hits         1107     1150      +43     
+ Misses        412      402      -10     
  Partials      108      108              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[imbas-forosnai]

Review from Imbas (J's AI collaborator, posting from its own account), with an independent cold review by Codex. Each finding says who found it. This is a comment review, not an approval; @baughj is the requested reviewer.

Before merge

1. ConflictsWith has no policy (P2: a contract gap this PR introduces; resolve before merge). Imbas and Codex. Codex raised it to blocking.
The plan says "explicit conflicts and exclusive groups support Replace or Reject policies" (HS-1517-traits-plan.md, Add steps 3–4). Decision 4 moved Policy onto ExclusiveGroup. As a result, a trait with ConflictsWith and no group can't say whether it rejects or replaces. A trait that has both doesn't say whether the group's policy also covers its conflicts. Slice 2 can't implement the plan's choice from the XML. It would have to assume Reject without saying so, or borrow a group policy that is about something else. Either of these would close it:

• Explicit conflicts always Reject. Say so in the plan and in the XSD comment.
• Add Policy to ConflictsWith, defaulting to Reject. Codex prefers this if Replace is a real requirement. It keeps the two policies independent and still can't produce an orphan policy.

Correctness in this slice

2. Name isn't checked for a blank or padded value (P2). Codex; Imbas ran it.
Name is the persisted key, but Validate never looks at it, and the loader doesn't apply the schema (HXML-35). Three cases, each loaded from a traits/ directory:

• Name="": 0 load errors, 0 validation errors, and stored under the key "".
• Name=" ": schema-valid, because String8 is xs:string with minLength 1, and it validates clean.
• Name=" Dockworker ": validates clean. TryGetValue("Dockworker") misses; TryGetValue(" Dockworker ") hits.

Suggest Validate reject a blank name and leading or trailing whitespace. Codex's view, which I share, is to reject rather than trim, because changing a persisted identity should be an explicit authoring decision.

3. <ExclusiveGroup> Background </ExclusiveGroup> is a separate group (P3). Codex found it; Imbas narrowed the trigger.
The text is kept as " Background ", validates clean, and doesn't match Background. A group that is only whitespace is already caught: XmlSerializer reads it as null, so the existing "no name" check fires. The same leading/trailing check as in 2 would close it.

4. A repeated ConflictsWith name isn't caught on the load path (P3). Codex.
The xs:unique rule is right, but only the schema tests use it. <Trait>Scribe</Trait><Trait>Scribe</Trait> loads and validates with 0 errors. The only cost is repeated work, so it isn't urgent.

Contract questions for slice 2

None of these is a defect in this slice; each needs a sentence of contract.

• Category and FindByCategory. Codex. Trait doesn't implement ICategorizable, so GetStore<Trait>().FindByCategory("Background") comes back empty. Status, Castable and Item are in the category index. If "for display and grouping" means consumers group Values<Trait>() themselves, that's fine. A sentence saying so would stop slice 2 from expecting the index.
• Mixed policies in one group. Imbas, with Codex's correction. Two traits in the same group can declare different policies. The XSD comment makes the incoming trait's policy decide, which is deterministic and can be useful (held A/Reject plus incoming B/Replace means B replaces A; the reverse is rejected). I first proposed checking that they agree. Codex argued against it, and I agree: that would invent a group entity that doesn't exist. I'd suggest documenting "incoming wins" and having slice 2 test both directions.
• Conflicts in both directions. Imbas, with Codex's correction. If A lists B and B doesn't list A, a check of only the incoming trait's list lets B onto an A-holder. "Cannot coexist" is symmetric, so slice 2 should check both lists. Requiring both traits to declare the conflict would just create a second copy that can drift.

Existing behaviour, not this PR's

• Case-only name collisions. Codex. Scribe and scribe both write the lower-case key scribe, and the last one stored wins: in the test, adding them in one order gave scribe, in the other order Scribe. Under parallel load, which one survives is a race. Status has the same pattern. I'll add the test to HXML-34.
• Exact duplicate names. Imbas. Two files with the same Name overwrite each other silently in XmlStore.StoreItem, and the parallel load makes the survivor a race. Every root type has this.
• Whitespace in xs:token enum values. Codex found it; Imbas ran it. IconSet=" Skill " and Policy=" Replace " pass the XSD, but XmlSerializer throws "not a valid value". The failure is loud (a load error), and the same mismatch applies to the 37 other xs:token restrictions in the XSDs.

Verified

• 211/211 tests pass against ceridwen main (6052cae).
• tools/regenerate.sh on Linux reproduces src/Objects/ byte for byte, so HXML-26's normalization holds.
• The 250 one-line files change only the xscgen header line.
• The enchant1 → enchant fix matches ceridwen's merged var_enchant.xml.

Every finding above was demonstrated by running it on a copy of the PR head, not only read from source.

[Caeldeth]

Thank you, @imbas-forosnai, and Codex. Every point is addressed in 2b8c778. 216 tests pass.

1. ConflictsWith has no policy

We chose the first option: an explicit conflict always rejects the incoming trait. A conflict that should replace is a group of two, and <ExclusiveGroup Policy="Replace"> already says that. Every replace case in the plan's proof content is a group. The rule is in the XSD comment on ConflictsWith, in the PR description, and in the traits plan (Add, steps 3 and 4).

2 and 3. Blank or padded names

Validate now rejects a Name that is empty, blank, or has a space at the start or the end, and the same for an ExclusiveGroup name. We reject and do not trim, as you suggest: a change to a saved key must be explicit. The species slice (HXML-37, not yet open) gets the same check for species keys and body keys.

4. A repeated ConflictsWith name on the load path

Validate now reports a name that occurs twice.

Contract questions for slice 2

All three are now in the XSD comments and in the traits plan:

• Category is for display only. The store does not index traits by category, so a consumer filters the loaded traits.
• In a group, the incoming trait's policy decides. Slice 2 tests both directions.
• A conflict applies in both directions. The server checks the incoming trait's ConflictsWith and the ConflictsWith of each held trait, and content lists each conflict once.

Existing behaviour

• Case-only name collisions: HXML-34. Thank you for adding the test there.
• Exact duplicate names: HXML-38.
• Spaces around an xs:token enum value: a new card, HXML-39. It compares making the schema reject the spaces with making the loader collapse them.