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

Implementation of the move Protect #6010

Open
ds-colo opened this issue Jan 12, 2025 · 2 comments
Open

Implementation of the move Protect #6010

ds-colo opened this issue Jan 12, 2025 · 2 comments
Labels
bug Bug category: battle-mechanic Pertains to battle mechanics status: unconfirmed This bug has not been reproduced yet

Comments

@ds-colo
Copy link

ds-colo commented Jan 12, 2025

Description

two points of note, both in src/battle_script_commands.c:

  1. in Cmd_setprotectlike, lines that increment protectUses don't bounds-check and will exceed the array length of sProtectSuccessRates on the 5th use and onwards, causing it to read incorrect values such as from sFinalStrikeOnlyEffects
  2. more of a suggestion, but sProtectSuccessRates has no config for gen 6 onwards where successive uses multiply the success rate by 1/3 instead of 1/2

Attached is game and log for the array index going OOB (threshold 67 corresponds to MOVE_EFFECT_BUG_BITE value from sFinalStrikeOnlyEffects)

protectratebug

Version

1.10.1 (Latest release)

Upcoming/master Version

No response

Discord contact info

No response

@ds-colo ds-colo added bug Bug category: battle-mechanic Pertains to battle mechanics status: unconfirmed This bug has not been reproduced yet labels Jan 12, 2025
@AlexOn1ine
Copy link
Collaborator

AlexOn1ine commented Jan 12, 2025

#define MAX_PROTECT_SUCCESS_RATE 4
static const u16 sProtectSuccessRates[MAX_PROTECT_SUCCESS_RATE]     = {USHRT_MAX, USHRT_MAX / 2, USHRT_MAX / 4, USHRT_MAX / 8};
static const u16 sProtectSuccessRatesGen6[MAX_PROTECT_SUCCESS_RATE] = {USHRT_MAX, USHRT_MAX / 3, USHRT_MAX / 9, USHRT_MAX / 27};

static inline u32 GetProtectSuccessRate(u32 protectUses)
{
    protectUses = (protectUses >= MAX_PROTECT_SUCCESS_RATE) ? MAX_PROTECT_SUCCESS_RATE - 1 : protectUses;
    if (B_CRITICAL_HIT_SUCCESS_RATE >= GEN_6)
        return sProtectSuccessRatesGen6[protectUses];
    return sProtectSuccessRates[protectUses];
}

Don't want to write tests right now so if anyone wants they can pick up from here. Otherwise I'll finish it sometime next week

@AlexOn1ine
Copy link
Collaborator

nevermind, started writing tests lol

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug category: battle-mechanic Pertains to battle mechanics status: unconfirmed This bug has not been reproduced yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants