Fix Soul Balls in newer clients #3291
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
guilherme-gm
force-pushed
the
fix-soulball
branch
2 times, most recently
from
71d33f4 to
dc9bd31
Compare
guilherme-gm
changed the title
guilherme-gm
commented
Apr 27, 2024
| case BALL_TYPE_NONE: | ||
| break; | ||
| } | ||
| clif->send(&p, sizeof(struct PACKET_ZC_SPIRITS), ((bl == NULL && spirit == BALL_TYPE_SOUL) ? &sd->bl : bl), (spirit == BALL_TYPE_SPIRIT ? AREA : target)); |
There was a problem hiding this comment.
the logic here in previous version is somewhat confusing to me, but:
nullpo_retv(bl);at the start, so(bl == NULL && ..)is always false, thusblis always the target (Well, it would be anyway)- the
targetfor SPIRIT is alwaysAREA, this is given by the caller already.
so simply changing to bl, target is equivalent.
There was a problem hiding this comment.
- I agree
targetis not alwaysAREA
guilherme-gm
force-pushed
the
fix-soulball
branch
from
dc9bd31 to
002346c
Compare
MrKeiKun
reviewed
Apr 27, 2024
src/map/clif.c
Outdated
| @@ -9761,7 +9751,7 @@ static void clif_refresh(struct map_session_data *sd) | |||
| if (sd->charm_type != CHARM_TYPE_NONE && sd->charm_count > 0) | |||
| clif->charm_single(sd->fd, sd); | |||
| if (sd->soulball > 0) | |||
| clif->spiritball(&sd->bl, BALL_TYPE_SOUL, SELF); | |||
| clif->soulball(&sd->bl, sd->soulball, SELF); | |||
There was a problem hiding this comment.
oh, what I tried to say is that for BALL_TYPE_SPIRIT, clif->spiritball is already called with AREA every time, no need to "hardcode" it in the ternary (thus, simply using target is enough). Just trying to justify that removing the ternary was safe, because clif->spiritball was already called with AREA every time it was called for "spirit ball" instead of "soul ball".
This one was not AREA, but it was also not BALL_TYPE_SPIRIT
MishimaHaruna
requested changes
Apr 30, 2024
- newer clients uses separate packets, making a mess to handle both in the same function - those functions now expect caller to give the number of balls, since most of the time there is a "generalization" to be "specialized" again
spirit spheres and soul balls are handled separately in newer clients, and thus, using ZC_SPIRITS always show spirit balls, even for classes that would show soul balls before
MishimaHaruna
approved these changes
May 1, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull Request Prelude
Changes Proposed
Fixes soul balls not being properly displayed in newer clients. (2020+).
2020+ clients split the logic for spirit spheres and soul balls, players may now have both at the same time, and a new packet was introduced for soul balls only (being separated from the spirit spheres one).
Implementation based on rAthena logic/packet. also huge thanks to 4144 for packet dates.
I decided to split
clif_spiritballinto 2 functions and leave the number of balls to the caller so we can simplify those functions. There were 2 reasons for that:Issues addressed:
None, I think (someone mentioned it in discord)