Skip to content

fix: correct Runic Ward import, double-count, and EHP operator precedence#2058

Closed
jay9297 wants to merge 47 commits into
PathOfBuildingCommunity:devfrom
jay9297:fix/runic-ward-import-double-count
Closed

fix: correct Runic Ward import, double-count, and EHP operator precedence#2058
jay9297 wants to merge 47 commits into
PathOfBuildingCommunity:devfrom
jay9297:fix/runic-ward-import-double-count

Conversation

@jay9297
Copy link
Copy Markdown

@jay9297 jay9297 commented Jun 2, 2026

Summary

  • ImportTab:ImportItem missing return item — The function parsed the GGG API item correctly but never returned it, so all API-import callers received nil. Items imported via the character API were silently dropped.
  • Ward double-counting on authoritative items — When armourData.Ward is pre-set from a paste or API property line (Runic Ward: 104), that value is the game's final post-quality result. The old code still applied wardInc rune mods on top of the already-final value, and left unconsumed flat BASE mods to leak into CalcDefence. Fixed with a wardIsAuthoritative flag: both BASE and INC local mods are consumed (removed from modList) but discarded; the property-line value is used verbatim.
  • Operator precedence zeroing EHP pooloutput[...TotalHitPool] + output.Ward or 0 parsed as (... + output.Ward) or 0, silently zeroing the entire hit pool when Ward was nil. Fixed with explicit parentheses: (output.Ward or 0).

Test plan

  • All 392 existing tests pass: docker compose run --rm tests
  • Paste a Runeforged item with Runic Ward: X — displayed Ward equals X (no quality re-scaling)
  • Import a character via GGG API wearing a Runeforged body armour — item appears in the Items tab with correct Ward value
  • Character with Ward equipped shows correct EHP (Life + Ward, not zeroed)
  • Item with rune mods in runeModLines#runeModLines reflects rune count, mods not double-applied

jay9297 and others added 30 commits May 25, 2026 18:23
- Add scripts/ai-agent.sh with 2-tier fallback (Claude Pro → OpenCode Go)
- Add CLAUDE.md and AGENTS.md with project-specific agent instructions
- Add .github/workflows: ci.yml, ai-fix.yml, ai-review.yml, auto-merge.yml
- Model slugs verified against opencode v1.15.7:
  review: opencode-go/glm-5.1
  fix: opencode/deepseek-v4-flash-free → opencode-go/qwen3.6-plus → opencode-go/deepseek-v4-flash
- .busted: enable coverage = true
- .luacov: configure include/exclude paths and report file
- .luacheckrc: lua51+luajit std, ignore unused args/loop vars
- Dockerfile: install luacheck via luarocks
Remove the build step (pre-built image from GHCR needs no local build)
and replace all `tests` service references with `busted-tests` to match
the name defined in docker-compose.yml.
docker compose run args replace 'command' but append to 'entrypoint',
so --tags and --coverage flags were being treated as executables.
Updated AI fix workflow to extract issue metadata and improve branch handling.
Added concurrency control to cancel in-progress AI reviews and improved diff handling.
Added caching and image management steps to the verify job.
Updated AI fix workflow to improve error handling and timeout settings.
Increased timeout for AI review job from 15 to 30 minutes and updated review prompt structure.
Update commit message format and error handling in AI workflow.
- Make 'rating' optional in evasion suffix patterns (r?a?t?i?n?g?)
- Remove dead-code keystone pattern with wrong parameter order
- Add bounds check for hollowPalmAddedPhys table (levels 1-40)
- Add descriptive line patterns for Hollow Palm Technique node
- Add Spectral Ward pattern for 'per X item evasion on equipped body armour'
- Update ModCache entries for all affected patterns
* Chore:  Bump action version numbers (PathOfBuildingCommunity#1993)

* Add support for Legacy Grand Spectrum mods (PathOfBuildingCommunity#1994)

Some old mods don't require the socketed in text

Co-authored-by: LocalIdentity <localidentity2@gmail.com>

* Add support for Ancestral Empowerment, Ancestral Call, Crescendo II and III, Infernal Cry, and "Ancestrally Boosted Attacks" passive nodes (PathOfBuildingCommunity#1839)

* add support for Ancestral Call, Crescendo II and III, ancestral boost tree nodes

* fix ancestral call inc dmg from tree

* big ole refactor so we stop duplicating code
one commit so I can revert if it goes boom

* added test
update CalcSection from "Exerted Warcries" to "Ancestral Boosts"

* add data map for gemName given a modSource

* boosted attacks configOption

* add support for Ancestral Empowerment, including combining Ancestral Empowerment with Fist of War
fix Fist of War/ancestrally boosted slams more AOE to inc AOE
add test for Combined logic for Ancestral Empowerment and Fist of War

* comment spelling

* better label

* fix avg aoe from fist of war

* forgot AOE portion for Crescendo III

* add support for Infernal Cry's empowered buff

* refactor away from specific baseMod for FistOfWar
fix Ancestral Empowerment not getting the double from fist of war III

* empowered condition for all Ancestrally Boosted

* ancestralBoostEffect

* multiplier instead

* pr comments
also refactored Crescendo III to use the new logic instead of an extra isolated block of logic by introducing a way to conditionally max uptime for Boosted or Empowered attacks

* Fix issues

Move the Infernal Cry buff to calc offence so it doesn't rely on stale calc output
Fix Ancestral Empowerment uptime calculation
Move gain table so Infernal Cry still works

* Fix spelling

---------

Co-authored-by: LocalIdentity <localidentity2@gmail.com>

* Add support for 0.5 JSON skill tree (PathOfBuildingCommunity#1984)

* Passive tree parser from ggg web data

* enhanced escapeGGGString

* Fix ascendancy unlock constraint check in node search parameters

* Refactor DrawQuadAndRotate to handle different data lengths for vertex calculations

* Add node overlay for Blighted Notable Frames in passive tree

* Fix typos and improve comments in passive tree script

* Fix typos in ascendancy handling and update inner radius calculations in passive tree script

* Refactor passive tree node replacement logic and add switchable options for Druid nodes

* Add support for alias notable maps and enhance jewel socket handling

- Enhanced passivetree_ggg.lua to support aliasNotableMap for jewel sockets.

* Rename aliasNotableMap to aliasPassiveSocket for clarity in passive tree data

* Enhance jewel socket handling by adding noRadius property and updating related logic in passive tree

* Enable Abyssal Lich parser

* Fix tests

Export ModCache
Fix crash with class Ids
Fix unarmed data for Ids

* Add legacy class ID remap

Old builds from 0.1 to 0.3 used the wrong id for the character class and opening them now would convert them to the wrong class. This maps the old builds so they now open correctly

* Formatting + typos

* AliasPassiveSocket fix

Was using the wrong name from a previous commit

* Export fixes

Uses the tree data folder to look for the ggg assets and json files
Fixes the tree file changing each time on export due to pairs usage

* Use copy file command to speed up export

Now runs in 0.1s instead of 8s

---------

Co-authored-by: justjuangui <servicios@juacarvajal.com>
Co-authored-by: LocalIdentity <localidentity2@gmail.com>

* Remove aliasPassiveSocket temp code

* Fix Hollow Palm Technique parsing and add Spectral Ward support

- Make 'rating' optional in evasion suffix patterns (r?a?t?i?n?g?)
- Remove dead-code keystone pattern with wrong parameter order
- Add bounds check for hollowPalmAddedPhys table (levels 1-40)
- Add descriptive line patterns for Hollow Palm Technique node
- Add Spectral Ward pattern for 'per X item evasion on equipped body armour'
- Update ModCache entries for all affected patterns

---------

Co-authored-by: Nighty <Nightblade@users.noreply.github.com>
Co-authored-by: LocalIdentity <31035929+LocalIdentity@users.noreply.github.com>
Co-authored-by: LocalIdentity <localidentity2@gmail.com>
Co-authored-by: Peechey <92683202+Peechey@users.noreply.github.com>
Co-authored-by: Juangui <80857657+justjuangui@users.noreply.github.com>
Co-authored-by: justjuangui <servicios@juacarvajal.com>
Co-authored-by: jay9297 <jay9297@users.noreply.github.com>
jay9297 and others added 17 commits May 29, 2026 11:05
# Conflicts:
#	spec/System/TestSkills_spec.lua
#	src/Data/ModCache.lua
#	src/Export/Scripts/passivetree_ggg.lua
#	src/TreeData/0_5/tree.lua
- Regenerate ModCache.lua to match current ModParser (removes stale
  combined modifier entries from pre-upstream Hollow Palm changes)
- Fix spellcheck workflow: remove stale dictionary cache that was
  causing upstream words to be missed, align with upstream config
- Never open PRs against upstream
- Default push target is fork (remote.pushDefault=fork)
- Upstream sync via local merge, not PR
Fix typo in specialModList: the GGPK stat ID prefix was 'cover_x%_'
but the real stat ID is 'recover_x%_of_maximum_ward_on_persistent_minion_death'.
Also add display-text pattern so customMods input can parse this modifier.
Add TestWard_spec.lua with 13 tests covering Ward mechanics.
Add Ward pool computation (base/inc/more), WardRegen from base regen
rate, WardRechargeDelay, WardBypass, WardRecoverOnBlock, and ward
config conditions (LowWard, FullWard) to CalcDefence. Add sidebar
display entries for Ward stats in BuildDisplayStats. Add Ward
breakdown panel rows in CalcSections. Add WardRechargeDelay and
WardRegenRatePercentPerMinute constants to Misc.lua. Add Ward-related
config options (conditionLowWard, conditionFullWard, etc).
Add SkillStatMap entries for base_maximum_ward, maximum_ward_+%,
ward regeneration, ward cost efficiency, recover_x_ward_on_block,
and recover_x_ward_on_charm_use. Add corresponding stat description
entries for all ward-related stat IDs.
Strip "Runeforged " prefix from item base names in Item.lua so that
Runeforged variants resolve to their underlying base type. Fix
ImportTab to handle Runic Ward import correctly.
…ArmourBreak)

- Add ward_regeneration_rate_+% and ward_rune_maximum_ward_+%_final
  stat-ID mappings to SkillStatMap
- Route WardRegen into TotalNetRegen and ComprehensiveTotalNetRegen
  with breakdown display
- Apply RuneWardDamageTaken multiplier in ward absorption formula
  so runic ward drains faster per point absorbed
- Add WardRecoverOnBlock to EHP GainWhenHit loop including both
  restore blocks and accelerated damage forwarding
- Route WardAttackHitPercent into ArmourBreakPerHit in CalcOffence
- Add test for WardRegen computation
…Regen test

- BuildDisplayStats.lua: WardBypass/WardRecoverOnBlock/WardCoverOnMinionDeath sidebar entries already present
- CalcSections.lua: Ward breakdown rows (Bypass, Recover on Block/Charm/Minion Death, Rune stats) already present
- TestWard_spec.lua: rename test and add nil-safe TotalNetRegen >= WardRegen assertion
…alNetRegen test

- ModParser: rename ward_rune_maximum_ward_ to ward_rune_maximum_ward_+%_final
  and change INC to MORE (_final suffix always denotes a MORE multiplier in GGPK)
- CalcDefence: read WardRegen BASE sum so flat per-minute bonuses are no longer
  silently ignored; convert from per-minute to per-second and include in breakdown
- TestWard: replace non-falsifiable TotalNetRegen assertion (== nil or ...) with a
  degen-triggered test that guarantees TotalNetRegen is computed and correct;
  add new test verifying Ward MORE stacks multiplicatively with INC
Implements the three core mechanics of the Stonefist ascendancy node:

- GloveBaseTypeTransform: overwrites equipped glove armour/evasion/ES values
  with the Fists of Stone base stats and injects the three per-level implicit
  mods (+2 Evasion, +1 ES, +1 Ward per player level) into modDB each calc pass.
  Item state is restored after each pass via env.stonefistRestore.

- IgnoreAttributeRequirementsForGloves: scoped flag that skips attribute
  requirement checks only for glove-slot items, without zeroing global
  attribute requirements.

- GloveExplicitModTransform stub: flag parsed and ready for the mod-equivalency
  lookup phase (Data/ModEquivalencies.lua export script included).

Also adds the Fists of Stone base type to Data/Bases/gloves.lua (armour 44,
evasion 40, ES 15, quality 20), ModParser patterns for all three ascendancy
mod strings, and a full Busted test suite in spec/System/TestStonefist_spec.lua.
…ence

Three bugs in the Runic Ward implementation:

1. ImportTab.ImportItem missing return statement
   ImportItem built and wired the item correctly but never returned it,
   so all callers received nil. Fixed by adding `return item` at the end.

2. Ward double-counting when property line is authoritative
   When armourData.Ward is pre-set from a pasted or API-imported property
   line ("Runic Ward: 104"), that value is the game's final post-quality
   result. The old code still ran calcLocal for INC rune mods on top of
   the authoritative value, inflating Ward and leaking unconsumed BASE
   mods to CalcDefence. Introduced wardIsAuthoritative flag: in that path
   both BASE and INC local mods are consumed (removed from modList) but
   their values are discarded; the authoritative value is used verbatim.

3. Operator precedence zeroing EHP pool
   `output[...TotalHitPool] + output.Ward or 0` evaluated as
   `(... + output.Ward) or 0`, which zeroed the entire pool when Ward was
   nil. Fixed with explicit parentheses: `(output.Ward or 0)`.

All 392 tests pass.
@jay9297
Copy link
Copy Markdown
Author

jay9297 commented Jun 2, 2026

Opened against wrong repo — re-submitting against fork.

@jay9297 jay9297 closed this Jun 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant