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

2-2 jobs skill rebalance - Sage (2018 patch/Renewal) #3230

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

Conversation

guilherme-gm
Copy link
Member

@guilherme-gm guilherme-gm commented Aug 24, 2023

Pull Request Prelude

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

src/map/skill.c Outdated Show resolved Hide resolved
@guilherme-gm guilherme-gm changed the title 2-2 jobs skill rebalance (Part 2 - Sage) (2018 patch/Renewal) and Auto Spell refactor (WIP) 2-2 jobs skill rebalance (Part 2 - Sage) (2018 patch/Renewal) and Auto Spell refactor Oct 8, 2023
@guilherme-gm guilherme-gm changed the title (WIP) 2-2 jobs skill rebalance (Part 2 - Sage) (2018 patch/Renewal) and Auto Spell refactor (WIP) 2-2 jobs skill rebalance (Part 2 - Sage) (2018 patch/Renewal) Oct 17, 2023
@guilherme-gm guilherme-gm changed the title (WIP) 2-2 jobs skill rebalance (Part 2 - Sage) (2018 patch/Renewal) 2-2 jobs skill rebalance (Part 2 - Sage) (2018 patch/Renewal) Oct 27, 2023
@guilherme-gm guilherme-gm changed the title 2-2 jobs skill rebalance (Part 2 - Sage) (2018 patch/Renewal) (WIP) 2-2 jobs skill rebalance (Part 2 - Sage) (2018 patch/Renewal) Oct 27, 2023
@guilherme-gm guilherme-gm force-pushed the old-job-rebalances-4 branch 5 times, most recently from f734d17 to 4289131 Compare October 28, 2023 18:28
@guilherme-gm guilherme-gm changed the title (WIP) 2-2 jobs skill rebalance (Part 2 - Sage) (2018 patch/Renewal) 2-2 jobs skill rebalance (Part 2 - Sage) (2018 patch/Renewal) Oct 28, 2023
@guilherme-gm guilherme-gm force-pushed the old-job-rebalances-4 branch 2 times, most recently from 03526a2 to 0da5993 Compare February 10, 2024 03:41
@violent01
Copy link

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.

@guilherme-gm
Copy link
Member Author

guilherme-gm commented Feb 14, 2024

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

@violent01
Copy link

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.

@guilherme-gm guilherme-gm force-pushed the old-job-rebalances-4 branch 2 times, most recently from 0639e05 to 3bedeb0 Compare April 27, 2024 00:32
@guilherme-gm guilherme-gm changed the base branch from master to rebalance May 30, 2024 00:02
- Requirement changed: Yellow Gemstone -> Blue Gemstone
- Effect changed:
  - Old: ATK +(SkillLevel x 10)
  - New: ATK/MATK +(5 + (Skill Level x 5))

From massive skills rebalance (1st/2nd/transclass) (2018.10.31)
- Requirement changed: Yellow Gemstone -> Blue Gemstone

From massive skills rebalance (1st/2nd/transclass) (2018.10.31)
- Fixed casting time reduced: 3s -> 1s
- Variable casting time added: 1s
- Requirement changed to Indigo Point/Yellow Wish Point/Lime Green
  Point/Scarlet Point (based on skill element)
- Now increase correspond element magical damage by 1% per skill level
- Removes skill failure
- Changes duration
  - Old: 20 minutes (Level 1 ~ 4), 30 minutes (Level 5)
  - New: 5 x (Skill Level + 1) minutes

From massive skills rebalance (1st/2nd/transclass) (2018.10.31)
- Autocast level changed
  - Old: The Bolt skills will vary in level from level 1 to 3
         The level 1 version will occur 50% of the time, level 2 35%,
           and level 3 at 15%
  - New: Levels of autocasted spells is the half of Hindsight level.
         If autocasted spells has lower level than half of Hindsight levels,
            actual skill level will be autocasted instead.
- Autocast chance changed
  - Old: 7% - 25%
  - New: (Skill Level x 2)%
- Usable skills changed
  - Old: Napalm Beat, Fire Bolt, Cold Bolt, Lightning Bolt,
         Soul Strike, Fire Ball, Frost Diver
  - New: Fire Bolt, Cold Bolt, Lightning Bolt, Soul Strike,
         Fire Ball, Earth Spike, Frost Diver, Thunderstorm, Heaven's Drive

From massive skills rebalance (1st/2nd/transclass) (2018.10.31)
@guilherme-gm guilherme-gm marked this pull request as ready for review May 30, 2024 00:19
this loads pre-re autospell_db even on RENEWAL,
and is offered as an alternative for RE servers that
doesn't support more than 7 skills at once.
@guilherme-gm guilherme-gm force-pushed the old-job-rebalances-4 branch 2 times, most recently from fc807fc to ede28c5 Compare June 1, 2024 01:17
@guilherme-gm guilherme-gm changed the title 2-2 jobs skill rebalance (Part 2 - Sage) (2018 patch/Renewal) 2-2 jobs skill rebalance - Sage (2018 patch/Renewal) Jun 2, 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.

None yet

3 participants