Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed dissolution of the flesh changing maximum hit pools #6791

Merged
merged 4 commits into from
Dec 6, 2023

Conversation

ProFrenchToast
Copy link
Contributor

Added a flag to the modifier on dissolution of the flesh to change recoverable life to full life pool rather than just unreserved life.

Description of the problem being solved:

Reserving life with dissolution of the flesh incorrectly reduces maximum hit pools. In game any skills which reserve life are automatically disabled when too much life is reserved through dissolution of the flesh.

Steps taken to verify a working solution:

  • Created a build that uses dissolution of the flesh .
  • Added an aura that reserves life.
  • Checked that maximum hit pool is uneffected buy adding the aura.

Link to a build that showcases this PR:

https://pobb.in/Eniw8-R3U0Ro

Before screenshot:

BuggedDissolution
This screenshot shows the maximum hit pool is based on the unreserved life pool when dissolution of the flesh is equipped.

After screenshot:

FixedDissolution
This screenshot shows the maximum hit pool is based on the total life pool when dissolution of the flesh if equipped.

Added a flag to the modifier on dissolution of the flesh to change recoverable life to full life pool rather than just unreserved life.
@ProFrenchToast ProFrenchToast marked this pull request as ready for review October 6, 2023 17:32
@Regisle
Copy link
Member

Regisle commented Oct 6, 2023

Thanks for the contribution, there are a few things I would like tested with this, how does it interact with Midnight Bargain as I assume that life doesnt get unreserved from too much damage, same with Relic of the Pact and its skill, as if I remember correctly that is a major issue for that skill

I think overall this change is good, but would like those 2 edge cases tested (and preferably fixed if they are an issue)

@ProFrenchToast
Copy link
Contributor Author

Tested Midnight Bargin and Relic of the Pact neither is disabled by taking damage meaning that life reserved by these means do effect max hit pool even with dissolution of the flesh. I understand why Midnight Bargin cannot be disabled but the skill granted by Relic of the Pact I would expected to just detonate similar to how auras get disabled.

I will look into how best to cover these two edge cases.

Added a new stat "Uncancellable_<pool>Reservation" this holds the percent of a given pool that cannot be unreserved for example by blood sacrament skill or midnight bargain item.
@ProFrenchToast
Copy link
Contributor Author

Added a new stat that contains the % of a given pool that cannot be unreserved, for example by Midnight Bargin or Blood Sacrament. This new stat uses the mod ExtraLifeReserved or ExtraManaReserved as a base to cover items with a fixed reservation such as Midnight Bargin and any skill with the flag HasUncancellableReservation has its reservation (post reservation efficiency calculations) added.

This will need to be changed if any uncancellable flat reservations are added to the game.

Build showing edge cases covered: https://pobb.in/hCEhMznR-1sV

Screenshot showing original behaviour. Max hit is uneffected by reserving an aura on life.
image

Screenshot showing Midnight Bargin does effect max hit while aura reserved on life does not.
image

Screenshot showing Blood Sacrament does effect max hit and stacks with Midnight Bargin and aura reserved on lilfe does not effect max hit.
image

Screenshot showing Blood Sacrament effect on max hit is effected by reservation modifiers.
image

@ProFrenchToast
Copy link
Contributor Author

After some testing in trial of the acestors this behavior seems bugged in that mode. Sometimes during a trial a player will die when they still have auras that can be cancelled. I'm guessing this is due to how the death system in trial of the ancestors works. No issues like this were found in sanctum.

Also, need to test if the reservation from mines is cancellable like with auras. From my experience I have never died with mines still reserved but should be tested properly

POB: https://pobb.in/-fiA4FMloBRb

@ProFrenchToast
Copy link
Contributor Author

tried to test mines reserved on life. It seems that the reservation for the mines gets cancelled rather than killing the player however the mines are not removed or detonated. No idea why this happens but at least it means no more changes.

@Paliak Paliak added the bug: calculation Numerical differences label Oct 27, 2023
@LocalIdentity LocalIdentity merged commit d66a1e4 into PathOfBuildingCommunity:dev Dec 6, 2023
deathbeam added a commit to deathbeam/PathOfBuilding-1 that referenced this pull request Dec 9, 2023
* upstream-dev: (369 commits)
  Update spec
  Export bases + tincture + stat descriptions
  Fix IsSupport Spec
  Fix skillgems
  Fix Skill gems dat
  Tincture export start
  Additional Files
  Update Spec
  Adding gemeffects.dat (PathOfBuildingCommunity#6974)
  Add support for toggling ineligible configurations (PathOfBuildingCommunity#5950)
  Add support for search input to Configuration tab (PathOfBuildingCommunity#6178)
  Fix spelling
  Fix Variable casing
  Ngahamu -> Ngamahu (PathOfBuildingCommunity#6955)
  Release 2.35.5
  Release 2.35.5 (PathOfBuildingCommunity#6951)
  Exclude extra ascendancies from node counts (PathOfBuildingCommunity#6949)
  Fixed dissolution of the flesh changing maximum hit pools (PathOfBuildingCommunity#6791)
  Release 2.35.4
  Release 2.35.4 (PathOfBuildingCommunity#6947)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug: calculation Numerical differences
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants