Add opt-in Full DPS auto-count for Totem skills#9849
Open
mcagnion wants to merge 1 commit into
Open
Conversation
Add a Configuration option "Auto-count Totems in Full DPS?" that lets Full DPS treat a Totem skill's count as the build's current TotemsSummoned (falling back to ActiveTotemLimit) when the skill's manual Count is 1. Behavior: - Default off: Full DPS uses the manual skill Count, unchanged. - Option on, Count == 1, skill has skillFlags.totem, not Explosive Arrow, and exactly one Totem source is included in Full DPS: Full DPS uses output.TotemsSummoned or output.ActiveTotemLimit. - Option on, Count > 1: manual Count wins. - Explosive Arrow is excluded from scaling because its custom DPS function already accounts for active totems, but it still counts as a Full DPS totem source for the multi-source guard. - Two or more Totem sources included in Full DPS (including Explosive Arrow): auto-count is suppressed and each skill keeps its manual Count, because ActiveTotemLimit is a global slot pool that cannot be allocated automatically across multiple sources. The implementation keeps two predicates intentionally separate: isIncludedFullDPSTotemSource (broader, used by the source counter) matches every included Totem skill that occupies a global totem slot; isFullDPSAutoTotemScalable (narrower, used by the per-skill scaling gate) additionally excludes Explosive Arrow. The option is gated by ifSkillFlag = "totem" so it only appears for builds containing at least one Totem skill, making it useful for comparison tools (Power Report, Compare tab, anoint sorting, trade query) to surface "+1 to maximum number of Summoned Totems" sources in single-totem-source Full DPS setups. Co-Authored-By: Claude Opus 4.7 (1M context) <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
1 participant
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:
Power Report, Compare tab, anoint sorting, trade query scoring and similar comparison tools all read Full DPS to evaluate "what does this change buy me?". For Totem skills, a source of
+1 to maximum number of Summoned Totemsonly contributes to Full DPS today if the user remembers to bump the manual skill Count on every relevant socket group. Otherwise totem-limit sources are invisible to comparison tooling -- e.g. the anoint dropdown sorted by Full DPS does not surface "Watchtowers" or other "+1 max totem" notables ahead of plain damage notables.This PR adds an opt-in Configuration option, "Auto-count Totems in Full DPS?". When enabled and the skill's manual Count is 1, the Full DPS loop treats the active count as
output.TotemsSummoned or output.ActiveTotemLimit. Manual Count > 1 always wins. Default is off, behavior for everyone else is unchanged. The option is only visible when the build has at least one Totem skill (ifSkillFlag = "totem").ActiveTotemLimitis a global slot pool, so auto-counting is only safe when a single Totem skill is included in Full DPS. With multiple Totem skills included, applying the global limit to each one would overcount the same slots; in that case auto-scaling is suppressed and each skill keeps its manual Count. The tooltip states this limitation, and a regression test covers the multi-source case.Steps taken to verify a working solution:
spec/System/TestFullDPSAutoTotems_spec.lua-- 6 successes, 0 failures, 0 errors. The spec covers: default-off keepsSkillDPS.count = 1; option-on scales byActiveTotemLimitfor a single Totem source; manual Count > 1 wins; theoutput.TotemsSummonedconfig override wins overoutput.ActiveTotemLimit; and auto-scaling is suppressed when more than one Totem socket group is included in Full DPS, so each skill keeps its manual Count.5x <skill>and toggling the option has no effect on the value -- manual Count wins.not activeSkill.activeEffect.grantedEffect.explosiveArrowFunc) is a single defensive guard so the existingexplosiveArrowFunccustom DPS path -- which already accounts for active totems internally -- is not double-counted. Verified by code review againstsrc/Data/Skills/act_dex.lua.Build used for the screenshots below:
Example totem build (Hierophant): https://pobb.in/-3AWBE9QE4FU. The change itself is not specific to any build -- any build with a single Totem skill marked Include in Full DPS will exhibit the same behavior. Builds with multiple Totem skills included in Full DPS leave auto-scaling suppressed, as documented above.
Anoint Item dropdown sorted by Full DPS, option OFF (baseline -- "Watchtowers" not in the top 6, only damage / crit notables surface):
Same Anoint Item dropdown, option ON ("Watchtowers" jumps to #1):
Configuration tab -- new option in the totem section: