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] Standardize "damage taken" modifier usage #7130

Open
wants to merge 2 commits into
base: base
Choose a base branch
from

Conversation

Xaver-DaRed
Copy link
Contributor

I affirm:

  • I understand that if I do not agree to the following points by completing the checkboxes my PR will be ignored.
  • I understand I should leave resolving conversations to the LandSandBoat team so that reviewers won't miss what was said.
  • I have read and understood the Contributing Guide and the Code of Conduct.
  • I have tested my code and the things my code has changed since the last commit in the PR and will test after any later commits.

What does this pull request do?

This is current usage:

  • Physical SDT modifiers: Defined as a float in database, fetched as an uint, stored in a base 1000 modifier where base 100% damage = value of 1000
  • Elemental SDT modifiers: Defined as an int in database, fetched as an int, stored in a base 10000 modifier where base 100% damage = value of 0 (sane) BUT 50% less damage = 5000 and 50% more damage = -5000
  • Other SDT modifiers: Defined as an int in database, fetched as an int, stored in a base 10000 modifier where base 100% damage = value of 0, 50% less damage = -5000 and 50% more damage = 5000

This is new usage:

  • All SDT modifiers are defined as an int in database, fetched as an int, stored in a base 10000 modifier where base 100% damage = value of 0, 50% less damage = -5000 and 50% more damage = 5000

Steps to test these changes

Fight mobs. Cast spells.

@Xaver-DaRed Xaver-DaRed force-pushed the sdt-normalization branch 2 times, most recently from 9907d21 to 4e6c064 Compare February 23, 2025 22:49
@UmeboshiXI
Copy link
Contributor

Just thought of some not functional but for documentation. The mod enum has comments relating to the old physical SDTs not being the same base values as the others.

@zach2good
Copy link
Contributor

I don't know if this ever came up in the past, but should we rename these mods to imply that they're 1/10000 1e4-based, etc.

SLASH_SDT -> SLASH_SDT_BASE_10K? Just really hammer it home forever? I wouldn't be against us marking other things with types too: ..._FLOAT, _PERCENT, _something that indicates 0.0 to 1.0, etc.

@Xaver-DaRed
Copy link
Contributor Author

I wouldnt be opposed to changing the modifier naming conventions. Or implementing a convention in the first place.

With this change, now all _SDT mods use 10k as base and all function the same (positive -> more damage, negative .> less damage)
This was more something that was bound to happen. We started this a year or 2 ago and we just didnt finish it.

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.

3 participants