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

Refactor FormulaHelper to read racial resistances #2676

Merged
merged 4 commits into from
Oct 5, 2024

Conversation

numidium
Copy link
Collaborator

@numidium numidium commented Jul 7, 2024

FormulaHelper previously applied racial resistances by reading the player's race directly. With this change, the saving throw calculation will be modified by the contents of the player's RaceTemplate.

Addresses the following issue:
#2660

FormulaHelper previously applied racial resistances by reading the player's race directly. With this change, the saving throw calculation will be modified by the contents of the player's RaceTemplate.

Addresses the following issue:
Interkarma#2660
petchema
petchema previously approved these changes Jul 7, 2024
ajrb
ajrb previously approved these changes Jul 17, 2024
@KABoissonneault
Copy link
Collaborator

I'm not convinced with the changes in this PR. It feels incomplete.

  1. High Elf paralysis resistance is still hardcoded rather than going through immunity flags
  2. Only racial Resistance is checked, not Immunity, Low Tolerance, or Critical Weakness

I guess the main point is really missing Breton flag for Resistance, so maybe I should let this PR pass. It just feels like a wasted opportunity if we just do the data-driven formula halfway

Copy link
Collaborator

@KABoissonneault KABoissonneault left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Numidium told me he would rework this PR for completeness

Removes hardcoded high elf immunity from FormulaHelper and uses flags set by RaceTemplate instead. Moves application of player racial flags up in the file so that the saving throw can be assigned to 100 before modification.
@numidium numidium dismissed stale reviews from ajrb and petchema via 204e97a August 14, 2024 05:47
@numidium
Copy link
Collaborator Author

Added a new commit that uses RaceTemplate for all saving throw-related flags. See commit description for details.

@KABoissonneault
Copy link
Collaborator

Seems mostly good.

  1. I think a race that has resistance for both paralysis and the element should apply the effect twice. Not relevant with any of the vanilla race, so I'll keep that as a later improvement.
  2. The saving throw for High Elves never goes through. The EntityEffectManager blocks the spell immediately with IsEntityImmuneToParalysis. It does not make this PR wrong, just kind of pointless. Perhaps we should remove the race template check in the EntityEffectManager, and let the SavingThrow formula handle it

I'm gonna merge this in the meantime, no critical issues

@KABoissonneault KABoissonneault merged commit f55852d into Interkarma:master Oct 5, 2024
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.

4 participants