Fix ES Bypass not overriding sources of negative bypass#6965
Merged
LocalIdentity merged 5 commits intoPathOfBuildingCommunity:devfrom Dec 11, 2023
Merged
Fix ES Bypass not overriding sources of negative bypass#6965LocalIdentity merged 5 commits intoPathOfBuildingCommunity:devfrom
LocalIdentity merged 5 commits intoPathOfBuildingCommunity:devfrom
Conversation
Wires77
reviewed
Dec 11, 2023
Member
Wires77
left a comment
Wires77
left a comment
There was a problem hiding this comment.
This isn't really the right approach, since you're reusing a completely separate flag just because that's how it's coded. Changing those mods to OVERRIDE, adding Chaos, and changing the calculation code to care about OVERRIDE mods is the better solution.
Member
|
I went ahead and pushed a commit that fixes this using the original mods, but haven't extensively tested it yet. Ideally this gets a test in the testing suite, too. |
Contributor
Author
|
That's fair! I tried making as few changes as possible, but understand why OVERRIDE is a better approach. |
LocalIdentity
approved these changes
Dec 11, 2023
deathbeam
added a commit
to deathbeam/PathOfBuilding-1
that referenced
this pull request
Dec 12, 2023
* upstream-dev: (31 commits) Release 2.37.0 (PathOfBuildingCommunity#7019) Add support for new Uniques + fix parsing for changed mod names (PathOfBuildingCommunity#7016) Fix ES from Tricksters Escape Artist when using Oath of the Maji (PathOfBuildingCommunity#7018) Fix Necromancer Offering charm not working (PathOfBuildingCommunity#7014) Update Query mods (PathOfBuildingCommunity#7011) Fix projectile count being 1 higher on all skills (PathOfBuildingCommunity#7006) Fix Herald of Agony quality not working (PathOfBuildingCommunity#7017) Fix Pyroclast Mine Aura Effect scaling Maximum Added Flat Damage (PathOfBuildingCommunity#7005) Fix Ascendant nodes counting towards allocated passive skill total (PathOfBuildingCommunity#7002) Change Manastorm config option to not overrun options box (PathOfBuildingCommunity#7008) Release 2.36.1 Release 2.36.1 (PathOfBuildingCommunity#6996) Fix pathing and mastery nodes (PathOfBuildingCommunity#6989) Export from game files Fix Crash when opening Timeless Jewel search (PathOfBuildingCommunity#6995) Fix negative bypass being ignored (PathOfBuildingCommunity#6992) Release 2.36.0 Release 2.36.0 (PathOfBuildingCommunity#6988) Add support for Tincture Implicits Fix ES Bypass not overriding sources of negative bypass (PathOfBuildingCommunity#6965) ...
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
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.
Fixes issue brougth up on reddit .
Description of the problem being solved:
all damage taken bypasses energy shieldwas being parsed as Base ES Bypass, which made it interact with sources of "does not bypass" when it shouldn't. Reusing the flagUnblockedDamageDoesBypassESfrom Emperor's Vigilance fixes the issue. Emperor's is however not getting parsed correctly at the moment, but that's another issue.Steps taken to verify a working solution:
Link to a build that showcases this PR:
Before screenshot:
After screenshot: