Repository navigation
Conversation
The second conversion step applied all conversions in one pairs() loop over tempConversions, where an entry for a converted type sets that type's share and every entry adds its conversions to other types. When a skill's own conversion and a global conversion both target the same type, the addition to that type is lost if the type's own share is set after it. pairs() order over string keys varies between LuaJIT processes, so the same build could give two results (e.g. 20% skill Physical-to-Fire plus 75% global to Fire: Fire 0.2 or 0.8). Set all shares first, then add all conversions, both in dmgTypeList order. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.
Description of the problem being solved:
In
CalcOffence.lua, the second conversion step ("Apply all conversions simultaneously") loops overtempConversionswithpairs(). Inside that loop, an entry for a converted type sets that type's share (= data.base), while every entry adds its conversions to other types. When a skill's own conversion and a global conversion both point at the same damage type, the addition to that type is overwritten if that type's own entry is visited afterwards.pairs()order over string keys is not stable across LuaJIT processes (the string hash is seeded per process), so the same build can show two different results depending on the process.Example: a skill with 20% of Physical converted to Fire, plus a global "75% of Damage Converted to Fire Damage" (e.g. Avatar of Fire). The Physical row should end up as Physical 0.2 / Fire 0.8 (skill conversion first, then 75% of the remaining 80%). Depending on iteration order it came out as either that or Physical 0.8 / Fire 0.2. In one build I measured CombinedDPS 7,725 vs 5,736 across fresh processes, about 3:5.
The fix keeps the intended "simultaneous" semantics: set every remaining share first, then add every conversion, both in
dmgTypeListorder, so the result no longer depends on table order.Steps taken to verify a working solution:
TestOffence_spec.lua(skill conversion to Fire + global 75% conversion to Fire) asserting Physical row Fire = 0.8, mult = 0.2.pathofbuilding-testscontainer: 904 successes / 0 failures (903 ondevwithout the new spec).Link to a build that showcases this PR:
Any build with a skill that converts part of its Physical damage to Fire (e.g. Cluster Grenade's 20%) plus Avatar of Fire.
Before screenshot:
N/A (result varied between runs of the same build).
After screenshot:
N/A
🤖 Generated with Claude Code