Change skills to use new stat set format #62
Merged
Conversation
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
deathbeam
suggested changes
Jan 6, 2025
LocalIdentity
approved these changes
Jan 8, 2025
Contributor
LocalIdentity
left a comment
LocalIdentity
left a comment
There was a problem hiding this comment.
Still has some issues but skills at least work now so we can start implementing them on dev
Wires77
pushed a commit
that referenced
this pull request
Jan 19, 2025
majochem
added a commit
to majochem/PathOfBuilding-PoE2
that referenced
this pull request
Aug 25, 2025
`addFlags` functionality was accidentally broken in PathOfBuildingCommunity#62 because `mainSkillFlags` and `calcSkillFlags` variables were introduced but seemingly later made redundant to use the original `skillFlags`. However the `addFlags` section still checked the old variables which were now `nil`. This wasn't a problem until now, because the PoE2 skills had not used `addFlags` yet.
LocalIdentity
pushed a commit
that referenced
this pull request
Sep 2, 2025
* Add 'totem' flag to Vile Effusion (Dark Effigy) The 'totem' flag was missing for the damaging part of the skill * Fix `addFlags` parsing `addFlags` functionality was accidentally broken in #62 because `mainSkillFlags` and `calcSkillFlags` variables were introduced but seemingly later made redundant to use the original `skillFlags`. However the `addFlags` section still checked the old variables which were now `nil`. This wasn't a problem until now, because the PoE2 skills had not used `addFlags` yet. * Remove "Base" mod for "ActiveTotemLimit" `CalcSetup` previous added +1 to "ActiveTotemLimit" as a base, but totem skills in PoE2 always come with "base_number_of_totems_allowed" values, which led to the values always being 1 too high * Let "Ancestral Warrior Totem" add the `totem` flag * Get totem stats from "base" totem skill This fixes previous problems that were caused by totem skills consisting of multiple parts, such as inability to get correct skillTotemId and level requirement for the "active" portion of a totem skill. The skill (or support) that provides the baseTotem data is now identified within `CalcActiveSkill` Examples: - "Shockwave Totem" -> "Wave" would not show correct totem stats - "Sunder" attached to "Ancestral Warrior Totem" would not show any totem stats at all * Reword "totemification" to "totemified" * Get rid of "totemified" * Add support for Mortar Cannon Adds support for a few stats on mortar cannon and ancestral totem too --------- Co-authored-by: majochem <majochem@users.noreply.github.com> Co-authored-by: LocalIdentity <localidentity2@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Update skill export to following schema
Currently not ready for merge as I'm working through logic in the rest of the app to handle the new format of skills