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

Implement m(atk) / m(def) percent system #3290

Merged
merged 33 commits into from
Apr 30, 2024

Conversation

skyleo
Copy link
Contributor

@skyleo skyleo commented Apr 15, 2024

Pull Request Prelude

Changes Proposed

Many skills use something called ATKPercentInfo & DEFPercentInfo in Aegis, the percentage bonuses are added up. (Both pre-re and re)

Every Bonus is in relation to it's skill and the strongest one according to abs(new_bonus) > abs(old_bonus) is chosen, when applying a new_bonus, if this is not met, then it won't be applied.

We're slightly deviating with the way we implement this, since Aegis keeps a map in order to keep track of the active Percent Buffs/Debuffs and their link to a skill, we won't do that.

Main purpose of this PR is to implement this system in order to be able to exclude these Percentage buffs from skills such as Asura Strike, which are not affected by Provoke or Curse for example.

This introduces values to the enum scb_flag which requires it to be 64 bit. This will only work MSVC 2022 with C23 or already in clang and gcc

In order to improve MSVC nontheless, last commits show a solution by using an ifdefed typedef.

Issues addressed: #1532

List of commits this is based on from my private fork, so I can compare if needed:

2309ba7d2efb30f043c1703aa69e58dc5c05279f
4a322ca9df8250f55af143382a8e95d46a0de084
c8f958f7ce842bb182d1918849a52ea81ee3a980
aa74964fb7248be46b6b342e3b8e1071abbcefca
5732261dc3642ac1cf058f9369787fc56b38a5ee
159d5167f9454ac4c1c8e34ae6e44bc90f201ec7
f94c2c68f62da10e72e10d70fa7be412204bbab7
bc744ff68f78e4c89c646d0dea00bcf24a961797
6d178d2188ebc1d706896d7b9f205f2169f2751f
69bebba647ffa8ca0a92dc901d18811a13c17196
35375883ee4edc15c21950041ab1ff0a95c7ee4d
f52ef302ad85266fc5989528142470b4e2ae0b46
5933b2c2f48c760e976d1f34bc1fe76978915c63
d3d33a560e01b88d2c14ba0cc5992f2d692ecf90
c2bf7a994767ad511fd37d6c9ab33d15259a3ea5
60b406dbb8c59d3b60a2ff5820f90abdcb7be544
907a3e15d95e652c2148d1cef5eb86478880be2d
8dfd03a0e3a8237725c9e2b98d83a6c69d96fff0
ce746187630b963af9aae377cb651ea40f13b5e2
0ee21c07a435f856ebb256fb93e371904058d762
6e619c64789691af1599a0c0aac1139882e052eb
b54ed44137e620a60cc333ec6d2cdc69efc0b9a3
d49ad9db026023eb2fec5526741576774e78b6e0
0865d2587f713454fb6e94a71a61becb3fa9851e
b11661a7794ec72476b16fbac4091d9fb85a11e0
a5ade750e3a977efa49a94dadde688203fb200b2
edaf3fd1882b8b80a8933b4003de2852598fa393
96cabeedc83ff57f4d5dc0708608f7e86722e425
9755566115e7a3cb40828fc869e3472d21b44706
05c812de9f5fab4b740c6b3f749ac22a511abb93
d044bf6629b3688e86bf2e5bde7bdeb5febbed9f
ad268787c070da7a4a82f8c15c03a5fa68505a99

@skyleo
Copy link
Contributor Author

skyleo commented Apr 15, 2024

Even VS 2022 apparently can't handle the enum no matter what settings for the compiler ...

Contemplating a solution atm that is least painful.

EDIT:
One solution is to use a typedef and depending on MSVC compiler or not we base it on an enum or uint64_t and the constants will be static const uint64_t then.

Applying that for now.

@skyleo
Copy link
Contributor Author

skyleo commented Apr 16, 2024

Will leave the last 6 commits unsquashed for now and rebase after review.

Please go ahead and review <3

@skyleo skyleo requested a review from guilherme-gm April 16, 2024 19:24
Copy link
Member

@guilherme-gm guilherme-gm left a comment

Choose a reason for hiding this comment

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

overall, code looks good to me. But I didn't test it nor compared to official. I plan to do that in a few days

src/map/status.c Outdated Show resolved Hide resolved
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