Conversation
mcagnion
marked this pull request as ready for review
May 12, 2026 21:41
mcagnion
force-pushed
the
feature/full-dps-auto-max-totems
branch
2 times, most recently
from
August 22, 2026 07:29
fad0bfb to
c376c5d
Compare
mcagnion
force-pushed
the
feature/full-dps-auto-max-totems
branch
from
August 29, 2026 06:23
f270bc5 to
6883e2d
Compare
Contributor
Author
|
Updated the implementation to keep |
When enabled, use TotemsSummoned or ActiveTotemLimit when the manual Count is 1. Manual counts still win, and multiple Totem sources keep their existing counts. Explosive Arrow remains excluded from automatic scaling because its DPS calculation already accounts for active Totems. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Use project terminology for the case where two Totem skills share one global limit.
Count only enabled Full DPS sources, resolve automatic Totem counts in one place, and cover the Explosive Arrow exclusion explicitly.
Keep calcFullDPS generic by passing resolved counts through its Count context. Totem-specific pool rules stay in the caller.
The private table is an identity key for one shared Totem slot pool; its contents are intentionally unused.
The existing Full DPS test already verifies that the option starts disabled.
mcagnion
force-pushed
the
feature/full-dps-auto-max-totems
branch
from
September 27, 2026 11:31
6883e2d to
11fdd5b
Compare
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, anoint sorting, trade scoring, and similar comparison tools use Full DPS to evaluate a change. For Totem skills, a source of
+1 to maximum number of Summoned Totemscurrently has no effect on those comparisons unless the user also updates the socket group's manual Count.This PR adds an opt-in Configuration option, Auto-count Totems in Full DPS?. For a single enabled Totem source included in Full DPS at Count 1, it uses the current
TotemsSummonedvalue when available, thenActiveTotemLimit. Manual Count values greater than 1 always win, and the option is off by default.Because
ActiveTotemLimitis a shared slot pool, multiple Totem sources fall back to their manual Counts rather than applying the same global limit to each. Explosive Arrow Ballista still occupies a source slot but is not scaled again because its custom DPS calculation already models active totems. Disabled Vaal variants do not block auto-counting for the remaining active source.The Full DPS aggregator resolves this through a generic, pool-aware Count policy boundary.
calcFullDPSonly builds and resolves generic Count context; Totem detection, shared-pool participation, opt-in handling, Count priority, and the Explosive Arrow exception stay inside the private Totem policy. No second skill family or per-skill Count persistence mode is introduced in this PR.Steps taken to verify a working solution:
Build used for the screenshots below:
Example totem build (Hierophant): https://pobb.in/-3AWBE9QE4FU. Any build with a single Totem skill marked Include in Full DPS exhibits the same behavior; builds with multiple included Totem sources keep their manual Counts.
Anoint Item dropdown sorted by Full DPS, option OFF (baseline — "Watchtowers" not in the top 6):
Same Anoint Item dropdown, option ON ("Watchtowers" jumps to #1):
Configuration tab — new option in the Totem section: