Skip to content

Conversation

@Rezzemy
Copy link

@Rezzemy Rezzemy commented Jan 20, 2025

Fixes #426.

Description of the problem being solved:

Shock min-max limits were changed in commit 0b90f10 causing any shock applied to be applied at a magnitude of 20%, this pull request reverts the changes to those limits.

EDIT 1/19:

When manually configuring the Effect of Shock in the Configuration panel, that number is processed and subject to the maximum and minimum % amounts for the shock ailment, so 35% will be processed and turned into 20% for damage calculation, which isn't correct.

Link to a build that showcases this PR:

https://maxroll.gg/poe2/pob/4h8t50yo

Configuration screenshot

image

Before screenshot:

image

After screenshot:

image

@QuickStick123
Copy link
Contributor

I don't think this is the correct way to go about it it seems you should instead be scaling the maximum shock by shock magnitude in this case it comes from the overcharge support gem.
image

@Rezzemy
Copy link
Author

Rezzemy commented Jan 20, 2025

I don't think this is the correct way to go about it it seems you should instead be scaling the maximum shock by shock magnitude in this case it comes from the overcharge support gem.

Forgive me because i believe i misrepresented this pull request

When manually configuring the Effect of Shock in the Configuration panel, that number is processed and subject to the maximum and minimum % amounts for the shock ailment, so 35% will be processed and turned into 20% for damage calculation, which isn't correct.

This change is just reverting the change that causes this issue. The build provided is just for used for demonstration, with the manually configured value being the proper amount provided by overcharge on shock.

You are correct though, scaling maximum shock would be better in the future, my only thoughts on this are that skills such as tempest flurry do not count as a generator of shock, so when not configuring any Effect of Shock and just having "Is the enemy Shocked?" configured as true, it doesn't get calculated.

@Rezzemy Rezzemy closed this Jan 20, 2025
@ConKou
Copy link

ConKou commented Jan 20, 2025

I think the base behavior should change because poe 2 calculates shock differently than poe 1.

In poe 2 shock has a baseline dmg taken increase of 20% that only gets scaled by modifiers (such as magnitude) and is not varied depending on the number of the hit proportionate to the mob's hp. The thing that is proportionate to mob hp is the chance to apply the shock. It's exactly the opposite than poe 1.

So imo the correct behavior should be, have the "Is shocked" stay as is. If it's enabled then don't allow for manual shock value, instead calculate 20% base * the correct modifiers (magnitude, less / more effect from gems etc). If it's a niche case like you said with Tempest Flurry, then have the input shock value box enable like you do with Stormweaver's Strike Twice for example.

Probably you are already doing this in #178 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Effect of Shock Configuration is ignored and Magnitude increases aren't calculated

3 participants