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

Skill rebalance from kro #3200

Draft
wants to merge 31 commits into
base: master
Choose a base branch
from

Conversation

gbasso666
Copy link

  • I have followed [proper Hercules code styling][code].
  • I have read and understood the [contribution guidelines][cont] before making this PR.
  • I am aware that this PR may be closed if the above-mentioned criteria are not fulfilled.

Implemented the skill changes from this patch: https://www.divine-pride.net/forum/index.php?/topic/3453-kro-mass-skills-balance-1st-2nd-and-transcendent-classes-skills/

All skills that were reworked were tested and seem to be working correctly. If I miss anything let me know

NOT IMPLEMENTED:
-Basilica (completely new skill);
-AMP (completely new skill);
-Harmonic Lick (completely new skill);
-Classical Pluck (todo: add CONFUSION to the aoe effect);
-New Falcon Assault formula (IRO formula seems weird/wrong and I couldnt find another source).

Also, Bowling Bash should only have 4 hits on 4 or more targets with 2h sword. Atm I only implemented 4 hits with 2h sword at all instances (which I think it's fair).

Additional content: Some skill were later (2022) adjusted again in kro, I might have already updated them (like songs lasting 180sec).

PS: btw, Hindsight skill from Professor seems very convoluted on skill.c, large portions of code doesnt seem to do anything, I recommend someone clean it up.

From this patch: https://www.divine-pride.net/forum/index.php?/topic/3453-kro-mass-skills-balance-1st-2nd-and-transcendent-classes-skills/

All skills that were reworked were tested and seem to be working correctly.
If I miss anything let me know

NOT IMPLEMENTED:
-Basilica (completely new skill)
-AMP (completely new skill)
-Harmonic Lick (completely new skill)
-Classical Pluck (todo: add CONFUSION to the aoe effect)

Also, Bowling Bash should only have 4 hits on 4 or more targets with 2h sword. Atm I only implemented 4 hits with 2h sword at all instances (which I think it's fair).

Additional content: Some skill were later (2022) adjusted again in kro, I might have already updated them (like songs lasting 180sec).
src/map/battle.c Outdated Show resolved Hide resolved
src/map/battle.c Outdated Show resolved Hide resolved
src/map/battle.c Outdated Show resolved Hide resolved
src/map/battle.c Outdated Show resolved Hide resolved
src/map/battle.c Outdated Show resolved Hide resolved
src/map/battle.c Outdated Show resolved Hide resolved
src/map/battle.c Outdated Show resolved Hide resolved
src/map/battle.c Outdated Show resolved Hide resolved
src/map/status.c Outdated Show resolved Hide resolved
src/map/status.c Outdated Show resolved Hide resolved
src/map/status.c Outdated Show resolved Hide resolved
src/map/status.c Outdated Show resolved Hide resolved
src/map/status.c Outdated Show resolved Hide resolved
src/map/status.c Outdated Show resolved Hide resolved
src/map/status.c Outdated Show resolved Hide resolved
src/map/status.c Outdated Show resolved Hide resolved
src/map/status.c Outdated Show resolved Hide resolved
src/map/status.c Outdated Show resolved Hide resolved
src/map/status.c Outdated Show resolved Hide resolved
src/map/status.c Outdated Show resolved Hide resolved
src/map/status.c Outdated Show resolved Hide resolved
src/map/status.c Outdated Show resolved Hide resolved
src/map/status.c Outdated Show resolved Hide resolved
src/map/status.c Outdated Show resolved Hide resolved
src/map/battle.c Outdated Show resolved Hide resolved
src/map/battle.c Outdated Show resolved Hide resolved
src/map/battle.c Outdated Show resolved Hide resolved
src/map/battle.c Outdated Show resolved Hide resolved
gbasso666 and others added 2 commits April 24, 2023 10:25
Co-authored-by: Evil Puncker <[email protected]>
Co-authored-by: Evil Puncker <[email protected]>
@MishimaHaruna
Copy link
Member

Hello, thank you for your contribution!

It would be nice if this was split into commits so that if in the future we'll need to debug an issue near the lines of code edited here, we'll be able to figure out if a change was intentional or accidental by reading the commit messages.

I'm scheduling this to be merged in the next release - I can lend a hand splitting this if you can't or don't know how

@MishimaHaruna MishimaHaruna added this to the Release v2023.06.14 milestone May 11, 2023
@MishimaHaruna MishimaHaruna added the status:inprogress Issue is being worked on / the pull request is still a WIP label May 11, 2023
@gbasso666
Copy link
Author

Thanks Mishima, I've been terribly busy with work (and sick with covid also), also I'm not particularly good at github, so for now that's all I can contribute.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:inprogress Issue is being worked on / the pull request is still a WIP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants