fix: correct misplaced parens in generated regen formula defaults (regen was silently 0) - #5
Merged
Merged
Conversation
ServerFormulas.cs is <auto-generated> by Xsd2Code++ and had drifted from the XSD. Commit 5a1a080 ("fix misplaced parens in HpRegenPerTick/MpRegenPerTick formulas") corrected src/XSD/ServerConfig.xsd but did not regenerate the C#, so the generated defaults kept the earlier, wrong parenthesization from a5759ab: Min( MAXHP * (0.1 + Max(CON, CON-LVL) * 0.01, MAXHP * 0.20) ) ^ should close after * 0.01 Min() therefore received a single argument and NCalc threw "Min() takes exactly 2 arguments". Hybrasyl's FormulaParser.Eval catches all exceptions and returns 0.0, so HP/MP regen was silently zero for any world whose config omits a Formulas section (ceridwen's config.xml has none, so the hardcoded defaults were in use). The XSD is already correct; a future regeneration will now agree with this file. Both literals verified byte-identical to the XSD defaults by extracting and comparing them programmatically, and evaluated through NCalc: regen now yields 200 for a level-10/CON-30/1000-maxHP character instead of 0. Note this changes live behaviour: with the cap restored, Min() binds at CON >= 10, so regen is 20% of max HP per tick for most characters. That is what the XSD specifies, but it has effectively been 0, so the swing is large. Bumps package version to 0.9.7. Co-Authored-By: Imbas <imbas@eris.co>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5 +/- ##
=======================================
Coverage 61.24% 61.24%
=======================================
Files 40 40
Lines 1538 1538
Branches 350 350
=======================================
Hits 942 942
Misses 475 475
Partials 121 121 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
HP/MP regen has been silently zero for any world whose config omits a
Formulassection — which includes ceridwen, whose
xml/serverconfigs/config.xmlcontains noFormulaelements at all, so the hardcoded C# defaults are in use.src/Objects/ServerFormulas.csis<auto-generated>by Xsd2Code++ and had driftedfrom the XSD:
Min()therefore received a single argument. NCalc throwsNCalcEvaluationException: Min() takes exactly 2 arguments; Hybrasyl'sFormulaParser.Evalcatches all exceptions and returns0.0;World.cscasts thatto
(uint)— so regen is 0.How the drift happened
a5759abfixed the parens in both the XSD andServerFormulas.cs, but to thewrong parenthesization (
Minstill got one argument).5a1a080("fix misplaced parens in HpRegenPerTick/MpRegenPerTick formulas")corrected the XSD only —
ServerFormulas.cswas never regenerated.So the XSD has been right since April and the generated C# has been wrong since.
Fix
Regenerate-equivalent hand-patch of the two literals to match the XSD. The XSD is
unchanged and already correct, so a future Xsd2Code++ run will now agree with this
file.
Verification
default=attributes byextracting and comparing them programmatically (not by eye — an eyeballed paren in a
wrapped string literal is how this was introduced).
NCalcSync 6.3.1using
FormulaParser.Eval's semantics, for a level-10 character with CON/WIS 30 and1000 max HP/MP:
dotnet buildacross all four TFMs:0 Error(s).Behaviour change — please read
This restores a cap that has not been functioning. With correct parens the
Minbindsat
CON >= 10, so HP regen becomes a flat 20% of max HP per tick for essentiallyany character. That is exactly what the XSD specifies, but since regen has effectively
been 0, the swing is large and worth a balance look before this reaches players.
Release note
Package version bumped
0.9.6→0.9.7. Publishing is triggered by pushing av*tag (or manual
workflow_dispatch), not by merging this PR — the package won'texist on nuget.org until tagged. hybrasyl-server pins
Hybrasyl.Xmlat0.9.6viaPackageReference, so it will not pick this up until the tag is pushed and thereference bumped (or it is built with
UseLocalXml=true).Context
Found while investigating a claim that NCalc truncates integer division. That claim was
measured false —
25 / 100→0.25 [Double]on 6.3.1, 5.8.0, and master — and theregen symptom traced entirely to these parens. Full write-up in comhaigne
(
docs/plans/hybrasyl-server/ncalc-integer-division.md).Generated with Imbas imbas@eris.co