Skip to content

Fix Node Power giving nodes another node's score - #2559

Open
juddisjudd wants to merge 1 commit into
PathOfBuildingCommunity:devfrom
juddisjudd:fix/power-cache-key
Open

juddisjudd wants to merge 1 commit into
PathOfBuildingCommunity:devfrom
juddisjudd:fix/power-cache-key

Conversation

@juddisjudd

Copy link
Copy Markdown
Contributor

Description of the problem being solved:

PowerBuilder caches one calculation per modKey and gives that result to every node with the same modKey. Nodes with the same modKey can still calculate differently:

  • a radius jewel only applies to the nodes in its radius
  • small and notable passives are scaled separately (incSmallPassiveSkill, JewelSmallPassiveSkillEffect, JewelNotablePassiveSkillEffect)
  • a weapon set node gets a WeaponSet condition on its mods, which matters for the _remove entries

So a Time-Lost Emerald with "Small Passive Skills in Radius also grant 1% increased Lightning Damage" makes every small passive with the same stat text show the damage of the one inside the radius, or none of them show it. Which one depends on which node pairs reaches first.

This change keys the cache on the modKey plus the node's type, attribute flag, weapon set and the radius jewels that cover it. Nodes that calculate the same still share one calculation.

Steps taken to verify a working solution:

  • Level 94 Invoker with that Time-Lost Emerald, Node Power on Combined DPS. Before: two "3% of Damage taken Recouped as Life" small passives (one inside the radius, one outside) and a "15% increased Evasion Rating" small passive outside it all showed +385.5. After: +385.5 for the one inside the radius, 0 for the two outside. Adding each node on its own through GetMiscCalculator gives the same numbers as after.
  • Scored every node a second time in a different order (sorted by cache key and split across separate calc instances, so a different node fills each cache entry) and compared singleStat, pathPower, offence and defence. Before the change the two runs differed on 101 of 3,743 nodes for the Invoker. After it they match on 9 builds, 3 of them with Time-Lost jewels.
  • Node Power on the Invoker took 24.6 s, against 22.5 s before, because nodes inside a radius no longer share a calculation with the rest.

Link to a build that showcases this PR:

Any build with a Time-Lost jewel that grants small passives in its radius a stat the build scales. Compare Node Power on two small passives with the same text, one inside the radius and one outside.

Before screenshot:

Both "Life Recoup" small passives: +385.5 Combined DPS.

After screenshot:

Inside the radius: +385.5. Outside: 0.

PowerBuilder caches one calculation per modKey and reuses it for every node
with that modKey. Nodes with the same modKey can still calculate differently:
a radius jewel only applies to the nodes in its radius, small and notable
passives are scaled separately, and weapon set nodes get a weapon set
condition. A Time-Lost jewel that gives small passives in its radius extra
damage made every same-stat small passive on the tree show that damage, or
none of them, depending on which one was scored first.

Key the cache on the modKey plus the node's type, attribute flag, weapon set
and the radius jewels that cover it.
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