Hercules
Hercules copied to clipboard
HerculesWS
2-2 jobs skill rebalance (Part 2 - Sage) (2018 patch/Renewal)
Dependency
This PR depends on #3229, all commits from #3229 are included in this one. The first commit original to this PR is:
Rebalance of SA_VOLCANO (Volcano)
Pull Request Prelude
- [x] I have followed proper Hercules code styling.
- [x] I have read and understood the contribution guidelines before making this PR.
- [x] I am aware that this PR may be closed if the above-mentioned criteria are not fulfilled.
Draft Reason
This PR by itself is ready and I don't think there will be further changes, unless someone finds an issue or a review asks for some change.
BUT, the rebalance in main official servers came at once, so I believe the right thing to do is to merge this PR together with the rest of the rebalance (that will be in other PRs).
So I will keep this as draft for now, but feel free to give an early review :)
Changes Proposed
The main purpose of this PR is to introduce the rebalance of Sage job skills. This change affects Renewal-only.
On official servers this came along with rebalances of 1st, 2nd jobs and transclass too. I am working in additional PRs for the remaining 2-2, including bard/dancer, being in separate PRs in order to keep those PRs in a reasonable size.
The implementation in this PR is based on kRO and kRO zero patch notes, iRO Wiki, rAthena and divine pride info, along with some in-game testing. I can't say everything is 100% accurate because there were discrepancies between different sources, and I could not test everything in kRO, but should be quite close.
Since clients before 2018-10 could only support up to 7 skills in Auto Spell, I added a define that allows people in Renewal to load pre-re autospell db and use the old skill list (in a mix of new/old behavior as documented in general.h) in case they can't migrate now.
I won't list all the rebalance changes in the PR description, but it may be checked in each commit text. Also, the commits are in the same order as they appear in the references below.
Affected skills
- SA_VOLCANO
- SA_DELUGE
- SA_VIOLENTGALE
- SA_FLAMELAUNCHER
- SA_FROSTWEAPON
- SA_LIGHTNINGLOADER
- SA_SEISMICWEAPON
- SA_AUTOSPELL
References:
Since AutoSpell is tricky to test, I made some sort of "unit tests" for Auto Spell in a separate branch to try to cover as many cases. You can check the testing code in this PR in my fork: https://github.com/guilherme-gm/Hercules/pull/9/files#diff-57e0249af7272bf23858146fc95e4674d48e2ffdf89e0824333cb54e232dd56d
It is kinda hackish, but this testing PR only extracts the functions to smaller ones with the same ones as in this main PR so I can test it using a plugin to run as many cases as possible. The same plugin can be used in this PR and it should pass for both PRE-RE and RE.
Issues addressed: Part of #2727
There's a small problem with Hindsight. I discovered this yesterday upon implementing this draft on my fork. This is what's happening. Use Hindsight-> Select Heavens Drive or Earth Spike-> Recast Hindsight Now select any spells beside those two->Recast Hindsight again and choose Heavens Drive or Earth Spike again-> Hindsight will not auto-cast Heavens Drive or Earth spike during this time but instead it will auto-cast the last spell you selected instead. Finally, you need to wait for the hindsight buff to end and reuse it for the Heavens drive or Earth spike to work again.
There's a small problem with Hindsight. I discovered this yesterday upon implementing this draft on my fork. This is what's happening. Use Hindsight-> Select Heavens Drive or Earth Spike-> Recast Hindsight Now select any spells beside those two->Recast Hindsight again and choose Heavens Drive or Earth Spike again-> Hindsight will not auto-cast Heavens Drive or Earth spike during this time but instead it will auto-cast the last spell you selected instead. Finally, you need to wait for the hindsight buff to end and reuse it for the Heavens drive or Earth spike to work again.
Thank you! There was an issue created by #3237 that ended up breaking here. I made a separate PR but also included the commit here (last commit in this PR). The fix in #3282 should be merged earlier since this PR is big, so I wanted to get this fix out ASAP because master is breaking too.
Thanks for testing :D
No worries! So far all of your rebalance changes up to MO_BLADESTOP has been nothing but fantastic. I'll keep testing every single one of your rebalance changes and I'll let you know again if I see something broken.