Skip to content

Conversation

@Paliak
Copy link
Contributor

@Paliak Paliak commented Aug 31, 2025

Fixes #8971

Description of the problem being solved:

#8862 seems to have caused an issue where LIST type mods are duplicated due to how ModListClass:MergeMod handles them

function ModListClass:MergeMod(mod)
if mod.type == "BASE" or mod.type == "INC" or mod.type == "MORE" then
for i = 1, #self do
if modLib.compareModParams(self[i], mod) then
self[i] = copyTable(self[i], true)
self[i].value = self[i].value + mod.value
return
end
end
end
self:AddMod(mod)
end

This PR adds a switch to ModListClass:MergeMod causing mods which cannot be merged to not be added.

@Paliak Paliak added the bug: behaviour Behavioral differences label Aug 31, 2025
@Wires77
Copy link
Member

Wires77 commented Sep 4, 2025

Revisiting this code when looking over this PR makes me realize there are a few more hidden gotchas here.

  1. Making a mod unscalable in a LIST mod doesn't do anything. An example of this would be "banners also cause enemies to take (%d+)%% increased damage". This code only checks for unscalable at the LIST level, not for each subMod:
    function ModStoreClass:ScaleAddMod(mod, scale, replace)
    local unscalable = false
    for _, effects in ipairs(mod) do
    if effects.unscalable then
    unscalable = true
    break
    end
    end
    if scale == 1 or unscalable then
    self:AddMod(mod)
    else
    local scaledMod = copyTable(mod)
    local subMod = scaledMod
    if type(scaledMod.value) == "table" then
  2. Unscaleable mods get added twice. I tested "take no extra damage from critical strikes" and saw the value double (even though there is a hard cap on a lot of these unscalable mods
image

In short, I'll merge this PR but I needed to write this down somewhere so it can be refactored again, with proper test cases added for all these corner cases.

@Wires77 Wires77 merged commit dacbc90 into PathOfBuildingCommunity:dev Sep 4, 2025
2 checks passed
@Paliak Paliak deleted the issue-8971 branch September 4, 2025 09:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug: behaviour Behavioral differences

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Runegraft of the Bound bug

2 participants