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

Add calculation boosts for Light Ball and Sandstorm #3387

Merged
merged 4 commits into from
Oct 8, 2023
Merged

Add calculation boosts for Light Ball and Sandstorm #3387

merged 4 commits into from
Oct 8, 2023

Conversation

Soul-8691
Copy link

In Gen 4+, Light Ball multiplies the base power of physical moves by 2, and Sandstorm multiplies the Special Defense of Rock types by 1.5. Added constants to be able to reverse these effects in the damage calc.

Description

#define B_LIGHT_BALL_ATTACK_BOOST  GEN_LATEST // In Gen4+, Light Ball multiplies the base power of physical moves by 2.
#define B_SANDSTORM_SPDEF_BOOST    GEN_LATEST // In Gen4+, Sandstorm multiplies the Special Defense of Rock types by 1.5.

Relevant Issues

Part of the overall concept of #1401

Discord contact info

yakattack_

include/battle.h Outdated Show resolved Hide resolved
include/config/battle.h Outdated Show resolved Hide resolved
@LOuroboros
Copy link
Collaborator

I have some issues with the description of that commit you just pushed.

In the meantime, I changed Light Ball to do IS_MOVE_SPECIAL(move) if the Gen is 3, even though that's technically false because of the lack of the PSS in Gen 3.

It's not false. The fact is that there is a split in Gen. 3, it's just type based instead of move based, but it's still a separation of categories for moves regardless.

I noticed there isn't really a way to account for the lack of the Physical/Special/Status Split in the damage calc functions without adding a bit more code.

For example, Choice Band's damage calculation code is IS_MOVE_PHYSICAL(move) without a check for the PSS.

There is no need for that to have a PSS related check. The Choice Band boosts the Attack stat specifically. Its check is a few lines above the Light Ball's in the vanilla codebase, which I linked for you to see in one of my review comments.

If you could just git commit --amend to remove that description full of misleading information and then force push, that'd be nice imo.

LOuroboros
LOuroboros previously approved these changes Oct 7, 2023
include/config/battle.h Outdated Show resolved Hide resolved
src/battle_util.c Outdated Show resolved Hide resolved
src/battle_util.c Outdated Show resolved Hide resolved
@AsparagusEduardo
Copy link
Collaborator

By the way, don't force-push if it's not necessary. It makes it harder to review new changes.
We tend to squash commits anyway, so no need to try to keep history clean.

@Soul-8691
Copy link
Author

I'm assuming a test needs to be updated for

    // snow def boost for ice types
    if (IS_BATTLER_OF_TYPE(battlerDef, TYPE_ICE) && IsBattlerWeatherAffected(battlerDef, B_WEATHER_SNOW) && usesDefStat)
    ```

@AsparagusEduardo
Copy link
Collaborator

AsparagusEduardo commented Oct 7, 2023

I'm assuming a test needs to be updated for

I fucked up in my suggestion, accidentally removed the line that adds the multiplier :D
image

@AsparagusEduardo AsparagusEduardo dismissed their stale review October 7, 2023 23:23

Forgot to point to upcoming

@AsparagusEduardo
Copy link
Collaborator

Looks good, I forgot to say to point this to upcoming

@Soul-8691 Soul-8691 changed the base branch from master to upcoming October 7, 2023 23:27
@AsparagusEduardo AsparagusEduardo merged commit fd19c26 into rh-hideout:upcoming Oct 8, 2023
1 check passed
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