[Caeldeth]

Summary

The ElementType enum lists the elements today. Each change to the element set needs a schema change, a regeneration and a package release. This PR moves the element set into the element table, as world data. It is a breaking change (HXML-24).

This PR also fixes a bug on main: no validation result reaches the result summaries (see "Validation summaries").

Element table

<ElementTable Name="HybElements2" DefaultMultiplier="1.0">
  <Elements>
    <Element Key="Force" Id="0" Default="true" />
    <Element Key="Fire" Id="1" RetailId="1" />
    <Element Key="Undead" Id="9" RetailId="9" />
  </Elements>
  <Groups>
    <Group Key="Cardinal" Members="Fire Water Wind Earth" />
  </Groups>
  <Source Element="Fire">
    <Target Element="Wind" Multiplier="1.2" />
  </Source>
  <Source Element="Undead" DefaultMultiplier="1.2">
    <Target Element="Undead" Multiplier="0.5" />
  </Source>
</ElementTable>

• <Elements> is required. Key is the name that content uses, and it must not change. Id is the modern wire value (0 to 254, and 255 means "no element"). RetailId is the retail wire value (0 to 9, default 0). The retail client indexes a 10-entry name table with this byte and does not check the range. Name, Default and Ivernic are optional.
• <Groups> is optional. Content picks a random member of a group with Random:<Key>. Each entry in Members is one chance, so a member listed twice is picked twice as often. Content can use this to weight a group.
• A Source row can have its own DefaultMultiplier, and Target is now optional.
• xs:key and xs:keyref make sure that each Source and Target names an element that the table defines. Element keys, element ids and group keys must be unique.
• A pair uses the Target multiplier, then the row DefaultMultiplier, then the table DefaultMultiplier.

Content

Attribute	Type now
StatModifiers BaseOffensiveElement, BaseDefensiveElement, OffensiveElementOverride, DefensiveElementOverride	Element key. No default: an absent attribute means "not set".
ElementalModifier/@Element, StatusDamage/@Element	Element key
Castable/@Elements, SpawnDamage/@Elements, SpawnDefense/@Elements	List of selectors

A selector is an element key, Random:<Group>, Current, Belt or Necklace. The schema checks the form of a key ([A-Za-z][A-Za-z0-9]*). It cannot check a key against the element table, because the table is a different document.

API changes

• ElementType is removed, and its entry in tools/enum-values.json is removed.
• Each element property is a string. null means "not set". There is no None.
• ElementTable gets DefaultElement, TryGetElement, TryResolve, CanResolve and Validate. GetMultiplier takes keys, and an overload takes resolved elements. ElementTable.KeyComparer compares keys without case, and every element-key comparison uses it.
• TryResolve resolves an element key, Random:<Group>, and the old names RandomTemuair and RandomExpanded. It never picks a group member that the table does not define. It returns false for Current, Belt and Necklace, because these need creature state. CanResolve gives the same answer without a pick, and is true for those three. The server can use it to check content at load.
• ElementTable implements IAdditionalValidation. The manager runs Validate on each table at load. Validate checks the rules that the schema cannot check: exactly one default element, reserved keys, group members, and wire values in a table built in code. It also checks a group with no key and a table with no Source rows, because the loader does not check files against the schema (HXML-35).
• The old names now pick from the world's Cardinal and Ordinal groups. Before this change, RandomTemuair picked by enum position, so it also picked Light and Dark.
• Spawn.OffensiveElement, Spawn.DefensiveElement and Castable.Element return the selector. They do not resolve it.
• Item.Element returns a key or null. StatModifiers.GetElementalAugment and GetElementalResistance take a key.
• CreatureBehaviorSet.ImmuneToElement(ElementType, …) is removed. The string overload stays, and now compares with KeyComparer (before this change, it compared with the current culture).
• Variant keeps the code from fix ResolveVariant's arithmetic (HS-1552) #11. Only its element checks change: an element that the variant leaves unset is null or empty, not None. The element tests from fix ResolveVariant's arithmetic (HS-1552) #11 use keys.

Validation summaries

GetResultSummaries read the validation stage with GetProcessResult, so no validation result reached a summary. CreatureBehaviorSet.ValidateAll hid this, because it wrote an XmlProcessResult, which also replaced the result of its ProcessAll pass. The validation stage now reads GetValidationResult. A new XmlValidationResult implements IValidationResult, and CreatureBehaviorSet.ValidateAll uses it. This fix is its own commit.

What must change with this release

• server (HS-1239): the server must use element keys, read the active table from ServerConfig, resolve selectors with TryResolve, send RetailId in 0x08, and use GetMultiplier. The server cannot take this package before that change.
• content (world, ceridwen): each element table needs an <Elements> block. None becomes an absent attribute, and Wood becomes Nature. An old table still loads, but it defines no elements, and its validation summary reports errors.
• tooling: Creidhne (HTOO-581) and hybindex-ts (HTOO-582).

Generated code

tools/regenerate.sh ran on Windows, and the output was normalized to match a Linux run (HXML-26). The script does not delete the file of a type that the schema removes, so this PR deletes src/Objects/ElementType.cs by hand.

Tests

184 pass, with the 17 tests from #11. The element table tests cover the schema rules (Elements, keys, ids, groups, keyref), the three-step multiplier lookup, TryResolve and CanResolve, Validate, the weight of a repeated group member, and the validation summary.

Other tests cover StatModifiers.GetElementalAugment and GetElementalResistance, Castable.Element, and a spawn element pick from more than one element. Validate tests also use tables built in code, because the schema rejects duplicate ids, group keys, source rows and targets in a file before Validate sees them. The changed lines that are not covered are null checks only (?.).

Not in this PR

• The package version stays 1.0.0-alpha2. This is a breaking change, so the version is the maintainers' choice.
• The server change and the content migration.

[codecov]

Codecov Report

❌ Patch coverage is 96.62162% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.03%. Comparing base (f0807ad) to head (bc6166d).

Files with missing lines	Patch %	Lines
src/Extensions/ElementTable.cs	96.77%	0 Missing and 4 partials ⚠️
src/Extensions/CreatureBehaviorSet.cs	66.66%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #10      +/-   ##
==========================================
+ Coverage   63.12%   68.03%   +4.91%     
==========================================
  Files          43       43              
  Lines        1524     1627     +103     
  Branches      346      385      +39     
==========================================
+ Hits          962     1107     +145     
+ Misses        457      412      -45     
- Partials      105      108       +3     

☔ 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.

[Caeldeth]

@imbas-forosnai — please review this PR as well. GitHub cannot request your review here, because the account is not a collaborator on this repository.

[imbas-forosnai]

I'm Imbas (@imbas-forosnai), the AI collaborator pattern J works with. Angelique asked for this review. It was done at d6550cc5 by two independent reviewers: me, and Codex reading cold. I checked every finding below against the source, mostly by running probes. This is comment-only, because approving in this repo is J's call.

The registry itself looks solid. There is one thing to fix before merging, which is the rebase, and two mediums in validation. I've included a tested resolution for the rebase.

What was verified

• Generated code. tools/regenerate.sh on Linux (xscgen 3.0.1356) reproduces src/Objects/ with no diff, and it does not recreate ElementType.cs.
• Build and tests. The library builds on net8, net9 and net10. All 164 tests pass against ceridwen HEAD.
• Schema constraints. Codex ran mutation tests on the xs:key, xs:unique and xs:keyref constraints, and each one rejects what it should: duplicate keys, ids, sources and targets, and undefined references.

Before merge: the rebase onto main (found by Imbas; resolution reviewed by Codex)

main now includes #11 (HS-1552). A trial merge of d6550cc5 onto origin/main shows two problems:

1. src/Extensions/Variant.cs conflicts. Taking this branch's side would silently drop every #11 fix: the Physical precedence fix, the SmallMax scaling (this branch still computes SmallMin twice), the flag union, the tag dedupe, the guards for an item with no Equipment, no Damage or no Restrictions, the damage clamp, and the level default.
2. tests/VariantResolution.cs merges without a conflict, then fails to compile. It still uses ElementType, so the build has eight CS0103/CS0246 errors.

The resolution below starts from main's Variant.cs and changes only the element checks to string form. With it, the merged tree passes 181/181 (164 from here plus 17 from #11). Codex reviewed it and found that it keeps both branches' intent.

--- a/src/Extensions/Variant.cs
+++ b/src/Extensions/Variant.cs
@@ public Item ResolveVariant(Item originalItem)
         if (Properties.StatModifiers is { } modifiers)
         {
             variantItem.Properties.StatModifiers += modifiers;
-            // The generated element properties have no Specified flag, so None is the only way to tell an
-            // element the variant leaves unset; the clone already holds the base item's.
-            if (modifiers.BaseDefensiveElement != ElementType.None)
+            // An element the variant leaves unset is null; the clone already holds the base item's.
+            if (!string.IsNullOrEmpty(modifiers.BaseDefensiveElement))
                 variantItem.Properties.StatModifiers.BaseDefensiveElement = modifiers.BaseDefensiveElement;
-            if (modifiers.BaseOffensiveElement != ElementType.None)
+            if (!string.IsNullOrEmpty(modifiers.BaseOffensiveElement))
                 variantItem.Properties.StatModifiers.BaseOffensiveElement = modifiers.BaseOffensiveElement;
         }
--- a/tests/VariantResolution.cs
+++ b/tests/VariantResolution.cs
@@
-    [InlineData("""<StatModifiers BonusHp="5" />""", ElementType.Fire, ElementType.Water)]
-    [InlineData("", ElementType.Fire, ElementType.Water)]
-    [InlineData("""<StatModifiers BaseDefensiveElement="Earth" />""", ElementType.Earth, ElementType.Water)]
+    [InlineData("""<StatModifiers BonusHp="5" />""", "Fire", "Water")]
+    [InlineData("", "Fire", "Water")]
+    [InlineData("""<StatModifiers BaseDefensiveElement="Earth" />""", "Earth", "Water")]
     public void ResolveVariant_KeepsBaseElements_UnlessTheVariantSetsItsOwn(string variantProperties,
-        ElementType defensive, ElementType offensive)
+        string defensive, string offensive)

The Variant.cs file is CRLF with a BOM, so keep both when you edit it. A text-mode rewrite turns the whole file into the diff.

Medium: a missing required attribute loads as a default, and Validate() doesn't catch it (found by Codex; confirmed by Imbas)

World data is loaded through a plain XmlReader (HybrasylEntity.LoadFromFile, line 90), so the schema's use="required" is never enforced at runtime. Two cases:

• An <Element Key="Fire"/> with no Id deserializes to Id = 0. Validate() reports nothing, because 0 is a legal id and is unique as long as no other element uses it, so Fire goes out on the wire as id 0.
• A <Group> with no Key, and a table with no <Source> rows, load cleanly in the same way.

My probe printed Fire.Id=0; Source rows=0; Validate: no errors.

Id is the field that goes on the wire, and this PR depends on load-time validation, so this is the place to close the gap. There are two options, and the choice is yours:

• Schema-validate production loads. This is the broader change: it touches the loader for every type.
• Keep a record of whether each required attribute was present (for example, make Id optional in the XSD so that the generator emits IdSpecified), and have Validate() reject anything missing.

A related case: an explicit empty element string, such as BaseDefensiveElement="", also gets past the schema at load. The IsNullOrEmpty check in the resolution above treats it as "not set", which is the same behaviour as this branch's own Variant.cs. Whichever option you choose, it should reject empty element strings too.

Medium: duplicate group members weight the random pick (found by both reviewers independently)

Members="Fire Fire Water" passes both the schema and Validate(), because xs:list cannot enforce uniqueness. The reservoir sampling in TryResolve then goes through the list entries, not the distinct keys. Out of 30,000 picks of Random:Dup, Fire came up 19,941 times and Water 10,059. So a typo in content silently changes the balance, and the comment's "uniform over the defined members" doesn't hold.

The fix belongs in Validate(): report duplicate members, compared with KeyComparer, in the same way it already reports duplicate keys. Deduplicating only inside TryResolve would leave the bad data unreported.

Checked and fine

• GetResultSummaries now reads GetValidationResult, which fixes the bug where validation results were swallowed.
• Every element comparison goes through KeyComparer. CreatureBehaviorSet.ImmuneToElement no longer depends on the current culture.
• TryResolve never picks an undefined member, and it refuses creature selectors.
• GetMultiplier falls back from the target, to the row default, to the table default.

[imbas-forosnai]

Imbas again (@imbas-forosnai). I'm withdrawing one finding from my review above.

J and Angelique have confirmed that duplicate group members are intentional: listing a member twice gives it twice the weight, so Members="Fire Fire Water" meaning about 2:1 is a feature.

To make the code say that rather than contradict it:

• The comment at ElementTable.cs:147-148 says each defined member is kept "with probability 1/n, so the pick is uniform over the defined members". It's per listed entry. Something like "each listed entry is kept with probability 1/n, so a member listed twice is picked twice as often" would say so.
• A test that pins the weighting, or the README/XSD annotation for Members, would keep a later cleanup from deduplicating it by accident. This review almost suggested doing exactly that.

The rebase item and the required-attribute medium still stand.

[Caeldeth]

Thank you, @imbas-forosnai. The branch is rebased onto main and force-pushed. 184 tests pass.

Rebase

I used your resolution. Variant.cs starts from main (#11), and only the two element checks change to string.IsNullOrEmpty. The file stays CRLF with its BOM. The element theory in tests/VariantResolution.cs uses keys. I removed the note about Variant from the message of the review-fixes commit, because that commit no longer changes Variant.cs.

Medium: missing required attributes load as defaults

This is a real gap, and you found the cause: the loader never checks a file against the schema, for any type. A trait with no IconSet loads as Skill in the same way. The correct fix is your first option, so I filed it as its own card, HXML-35: ship the schemas with the library and check each file at load. It changes the loader for every type and needs a pass over world and ceridwen first, so it is not in this PR.

In this PR, Validate now also reports a group with no key and a table with no Source rows (commit bc6166d). A missing Id is different: Validate cannot tell it from Id="0" without a change to the schema. In a normal table, the default element has id 0, so a missing Id repeats that id and Validate reports it. That is not guaranteed, and HXML-35 closes it. An empty element string, such as BaseDefensiveElement="", means "not set" everywhere in this PR, and HXML-35 makes the schema pattern reject it at load.

Medium: duplicate group members

This behaviour is intentional. We discussed it and keep it as a feature: content weights a group by listing a member more than once, for example Members="Fire Fire Water" picks Fire two times in three. So Validate does not report duplicate members. The commit bc6166d documents it in the schema, in the TryResolve comments and in the PR description, and a test checks the weight. Your probe gave the expected result.

[baughj]

@Caeldeth and i discussed Fire Fire Water, we're keeping it