Skip to content

Add masteries to power report#9604

Open
ksnyder9801 wants to merge 1 commit intoPathOfBuildingCommunity:devfrom
ksnyder9801:dev
Open

Add masteries to power report#9604
ksnyder9801 wants to merge 1 commit intoPathOfBuildingCommunity:devfrom
ksnyder9801:dev

Conversation

@ksnyder9801
Copy link
Copy Markdown

Fixes #3704 .

Description of the problem being solved:

Masteries are not in the power report

Steps taken to verify a working solution:

  • Manually tested with my build and cross-checked with stat differences tooltips
  • Added spec

Link to a build that showcases this PR:

literally any build

Before screenshot:

image

After screenshot:

image

Comment thread src/Classes/CalcsTab.lua
for nodeId, node in pairs(self.build.spec.nodes) do
wipeTable(node.power)
if node.type == "Mastery" then
node.power.masteryEffects = { }
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NB: adding a new table to node.power since a single power per node can't represent unallocated mastery options

Comment thread src/Classes/CalcsTab.lua
end
end
end

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

opted to duplicate the power calc in the loop here instead of a bigger refactor for readability

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer to not duplicate this logic, actually.

Copy link
Copy Markdown
Member

@Wires77 Wires77 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is some opportunity to combine things here, but I like the idea, and love the added test

Comment thread src/Classes/TreeTab.lua
Comment on lines +1108 to +1127
local nodePower = (effectPower.singleStat or 0) * ((displayStat.pc or displayStat.mod) and 100 or 1)
local pathPower = ((effectPower.pathPower or effectPower.singleStat or 0) / pathDist) * ((displayStat.pc or displayStat.mod) and 100 or 1)
local nodePowerStr = s_format("%"..displayStat.fmt, nodePower)
local pathPowerStr = s_format("%"..displayStat.fmt, pathPower)

nodePowerStr = formatNumSep(nodePowerStr)
pathPowerStr = formatNumSep(pathPowerStr)

if (nodePower > 0 and not displayStat.lowerIsBetter) or (nodePower < 0 and displayStat.lowerIsBetter) then
nodePowerStr = colorCodes.POSITIVE .. nodePowerStr
elseif (nodePower < 0 and not displayStat.lowerIsBetter) or (nodePower > 0 and displayStat.lowerIsBetter) then
nodePowerStr = colorCodes.NEGATIVE .. nodePowerStr
end
if (pathPower > 0 and not displayStat.lowerIsBetter) or (pathPower < 0 and displayStat.lowerIsBetter) then
pathPowerStr = colorCodes.POSITIVE .. pathPowerStr
elseif (pathPower < 0 and not displayStat.lowerIsBetter) or (pathPower > 0 and displayStat.lowerIsBetter) then
pathPowerStr = colorCodes.NEGATIVE .. pathPowerStr
end

local effectLabelParts = isAlloc and not node.allMasteryOptions and node.sd or effect.stats or effect.sd
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is all the same code as the section above, I think we can combine these sections. At least have a function that they share.

Comment thread src/Classes/CalcsTab.lua
end
end
end

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer to not duplicate this logic, actually.

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.

power report: add passive masteries

2 participants