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

Multi-hit refactor and Parental Bond #1676

Merged
merged 43 commits into from
Nov 2, 2022

Conversation

BuffelSaft
Copy link
Collaborator

@BuffelSaft BuffelSaft commented Sep 23, 2021

Port multi-hit system from the CFRU, implement Parental Bond, refactor certain moves as required and implement all remaining multi-hit moves except Dragon Darts and Scale Shot.

Description

This PR does the following:

  • Implements Parental Bond
  • Replaces the existing script-based multi hit system with the CFRU's version, which sets up the number of hits in the attack canceller and executes the follow up hits as a looping move end effect.
  • Implements the following move effects: Triple Axel, Surging Strikes, Double Iron Bash
  • Implements Triple Kick/Skill Link interaction (no accuracy check on follow up hits if if user has Skill Link)
  • Updates Beat Up to its modern effect and adds a config option to switch between this and its old effect
  • Refactors the following move effects to be move end effects (like Knock Off) so that they happen after both strikes of Parental Bond:
    • Smack Down
    • Burn Up
    • Wake Up Slap/Smelling Salts/Sparkling Aria
  • Adds FLAG_TWO_STRIKES. Any move with this flag will strike twice and its secondary effect will occur on both strikes (see the changes to Twineedle for an example).

The following Parental Bond-affected edge cases are accounted for:

  • Fury Cutter and Echoed Voice increase their damage counter on the second strike of Parental Bond, so that they don't get boosted damage after the first hit.
  • If Present heals the foe, it doesn't strike twice. If Present damages the foe, the second hit will always do damage.
  • Knock Off and Wake-Up Slap deal bonus damage on both strikes, as their effects happen after both hits
  • Burn Up receives STAB on both hits
  • Smack Down knocks down the target after both hits
  • Items are stolen after both hits, so items can activate after the first hit (and not be stolen) if applicable
  • Recoil damage happens after both strikes and is based on the total damage of both hits
  • Bug Bite's berry stealing effect happens after the final hit

Some things aren't handled yet:

  • Relic Song should change Meloetta's form after both hits if it has Parental Bond (Relic Song effect not done yet)
  • Secret Power's secondary effect happens only on the second strike. I had some trouble with this one, and I'm not sure if it can just be made into a move end effect like the Smack Down et al.
  • Struggle does not deal 50% of the user's HP if it has Parental Bond. This is because Struggle has not been updated to its modern effect, so that can be done in a separate PR.

Notes for testing:
I've had to rearrange the order that some move end effects happen, and while it seems fine in my testing so far, it's a major change and will need thorough testing.

Gifs:
Recharge moves and fixed damage moves:
PB recharge fixed damage
Recoil after both hits (Double-Edge changed to fighting type for testing):
PB Recoil
Target smacked down after both hits:
PB Smack Down
Spread moves hit once if there are two targets, twice otherwise:
PB spread move 2 targets
Status is cured after both hits for Smelling Salts etc.:
PB cure status
Knock Off removes items after both hits:
PB knock off
Burn Up removes Fire typing after both hits:
PB burn up

Addresses the following issues:
#351
#1102
#1107
#1402

Discord contact info

Buffel Saft#2205

Port multihit system from the CFRU, implement Parental Bond, refactor certain moves as required and implement all remaining multi hit moves except Dragon Darts.
@LOuroboros
Copy link
Collaborator

There's conflicts that need to be solved 👀

src/battle_util.c Outdated Show resolved Hide resolved
src/battle_util.c Outdated Show resolved Hide resolved
src/battle_util.c Outdated Show resolved Hide resolved
@BuffelSaft
Copy link
Collaborator Author

Conflicts resolved and tabs replaced!

@AsparagusEduardo AsparagusEduardo added the category: battle-mechanic Pertains to battle mechanics label Oct 1, 2021
The "x was hit with recoil!" message was appearing when the target was immune due to protection or an ability - this should fix that and handle any other cases where 0 damage is dealt.
@ultima-soul ultima-soul linked an issue Oct 4, 2021 that may be closed by this pull request
Make these effects endure one hit of a multi strike move, not all of them.
@LOuroboros
Copy link
Collaborator

More conflicts to solve 👀

@BuffelSaft
Copy link
Collaborator Author

More conflicts to solve 👀

Done! There are some problems with Thief's move effect, so I'll need to fix those too (haven't figured out what the cause is yet).

Thief doesn't work as a move end effect - the item steal animation only works if it runs before the target faints. This restores its original code and adds a simple check to handle its interaction with Parental Bond.
Bug Bite's effect should only occur on the final strike of Parental Bond (which can also be the first strike if it KO's the target).
Copy link
Collaborator

@AsparagusEduardo AsparagusEduardo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Parental Bond notes

Cases:

  • Causes most damaging moves used by the Pokémon to become two-strike moves with the second strike having its damage halved
    • Unless it is a set-damage move (such as Seismic Toss and Dragon Rage), which deal the full amount of damage for both strikes
    • Check Gen 6 damage multiplier.
  • It does not affect multi-strike moves
  • If a move could hit multiple targets (including allies), such as Earthquake and Rock Slide, it will not strike twice
    • If a move could hit multiple Pokémon but only hits one due to missing the other Pokémon, it will only strike once.
    • however, if it can only hit a single Pokémon, such as in a Single Battle it will strike twice.
  • Since Parental Bond turns moves into two-hit multi-strike moves
    • each strike has a separate chance to be a critical hit
    • items that trigger upon strike or contact such as Rocky Helmet occur for each strike
    • Abilities that trigger upon strike or contact such as Cursed Body occur for each strike
  • there is only one accuracy check, so either both strikes hit or both strikes miss.
  • Any attack which has a secondary effect has the same secondary effect on both strikes (such as Power-Up Punch);
    • except Secret Power's secondary effect that can only occur after the final strike.
    • each strike has an independent chance of activating that effect.
  • Even if the Pokémon's Ability is changed to Mummy after the first strike, it will continue to make a second strike regardless.
  • Pay Day scatters coins after the first strike only.
  • Incinerate destroys applicable held items after each strike.
  • If a move has recoil damage, the recoil will be based on the damage dealt by both strikes, but will be taken after the final strike.
  • Struggle will inflict recoil damage equal to half the user's maximum HP (after the final strike)
  • Moves that switch the user out strike twice, then force a Pokémon to switch out after both strikes are conducted.
  • Thief, Covet, Bug Bite, and Pluck do not steal or eat the target's held item until after the final strike, so if the target could use its item after the first strike (e.g. due to low HP), it will use it before the attacker can steal or eat it.
  • Smelling Salts, Wake-Up Slap, and Knock Off do not cure the target's status condition or remove its held item (respectively) until after the final strike, so both strikes get the increased power.
  • Fire-type moves, Scald, and Steam Eruption thaw a frozen target after the final strike (so a frozen target cannot be thawed and then burned by the same move).
  • Smack Down and Thousand Arrows only cause the target to fall to the ground after the final strike.
  • If Meloetta has Parental Bond and uses Relic Song, it will change Forme only once, after the final strike.
  • Burn Up does not remove the user's Fire type until after the second strike (so both strikes receive same-type attack bonus).
  • If Present heals the target it will only strike once, but if it damages the target it will strike twice (the second strike will always damage the target).
  • The damage dealt by Psywave is generated separately for each strike, and the second strike's damage is not halved.
  • Each strike of Super Fang halves the target's HP (effectively quartering it if HP is not changed between strikes).
  • Counter, Mirror Coat, Metal Burst deal the full amount of damage for both strikes.
  • The first strike of Assurance counts as previously taking damage for the second strike, giving it increased power.
  • Fury Cutter and Echoed Voice only consider uses of the move rather than hits, so the second strike's power is not boosted by the first strike.
  • Natural Gift strikes twice.
  • Spit Up strikes twice.
  • Moves that require recharging after use strike twice, but the user only needs to recharge for one turn.
  • One-hit knockout moves only strike once.
  • Fling only strikes once.
  • Self-Destruct and Explosion only strike once.
  • Final Gambit only strikes once.
  • Uproar only strikes once.
  • Rollout and Ice Ball only strike once.
    • Other consecutively executed moves, such as Outrage, can strike twice.
  • Moves with a charging turn (such as Fly and Solar Beam) only strike once, even if the Pokémon becomes fully charged in one turn (such as with a Power Herb).
  • Endeavor also only strikes once, even if the user or target's HP is changed after it strikes (such as by Iron Barbs or the Sitrus Berry).
  • Confusion damage only occurs once.
  • Spirit Shackle and Anchor Shot only trap the target after the final strike.

Failed tests

  • If using a move that can hit multiple targets when only one target is in range, it will strike twice.
    earthquake

  • Spiky Shield and King's Shield only damage and decrease Attack (respectively) once if they protect a Pokémon from a contact move used by a Pokémon with Parental Bond.
    kingshield spikyshield

  • Moves that switch the target out strike twice, then force a Pokémon to switch out after both strikes are conducted.
    dragontail

  • Parental Bond does not affect Z-Moves.
    zmove

Suggested improvements:

  • Using constants for parentalBondOn (PARENTAL_BOND_1ST_HIT, PARENTAL_BOND_2ND_HIT, PARENTAL_BOND_OFF). Rename parentalBondOn to parentalBondState.
  • Adding a function to get the random 2-5 multi-hit count.
    • use ifdef instead of if
  • Self-destruct and Explosion don't need the forbidden flag, as the user faints before a second hit, likely canceling the second strike. Should we fix this so that they do hit twice and they need the flag? 👀
    • And if we do, we'll need to add Misty Explosion to the ban list.

Not tested, requires unimplemented feature:

  • Grass Pledge, Fire Pledge, and Water Pledge strike twice, even when used as a combination move.
  • Parental Bond does not affect Max Moves.

To be fixed in a later PR:

  • Bide deals the full amount of damage for both strikes.

Beat Up Notes:

Suggestion:

  • B_UPDATED_MOVE_DATA for Beat Up should be replaced by B_BEAT_UP_DMG.

- Dragon Tail and Circle Throw strike twice and then switch the target
- Z moves are unaffected by Parental Bond
- Renamed parentalBondOn to parentalBondState
- Added constants for possible values of parentalBondState
- Fixed broken stat change animations and removed unused MOVE_EFFECT_SKY_DROP
- Parental Bond spread moves should now strike twice whenever there is a single target, even if the target is the user's ally.
- Random chance to hit 2-5 times is now set in SetRandomMultiHitCounter().
Probably not needed, but just in case!
@BuffelSaft
Copy link
Collaborator Author

@AsparagusEduardo I've fixed the following:

  • If using a move that can hit multiple targets when only one target is in range, it will strike twice (but actually fixed this time, I hope!)
  • Spiky Shield and King's Shield only damage and decrease Attack (respectively) once if they protect a Pokémon from a contact move used by a Pokémon with Parental Bond.
  • Moves that switch the target out strike twice, then force a Pokémon to switch out after both strikes are conducted.
  • Parental Bond does not affect Z-Moves.
    I've also changed parentalBondOn to parentalBondState, added constants for each state, and moved the random hit counter to a separate function.

I left Explosion in the ban list just in case something wacky gets done to it in future; this way it will always work properly with Parental Bond. IMO it's also nice to have all the unaffected moves documented in the code.

Oddly I'm not seeing the Beat Up delay in the gen 5+ version, though it should be there for gen 3 and 4 as the vanilla version waits ages for the "mon's attack!" string on each hit.

I think Beat Up is worth a separate config because it's essentially a whole new move effect with very different mechanics, rather than a simple data change like most updated moves get. Could move it to other move settings if that fits better?

Copy link
Collaborator

@AsparagusEduardo AsparagusEduardo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pause seems to be happening for every multi-hit iteration, not only Beat Up. It's between the times the mon takes damage.
Are you sure you don't notice it?

PauseMultiHit.mp4

src/battle_util.c Outdated Show resolved Hide resolved
src/battle_main.c Outdated Show resolved Hide resolved
src/battle_script_commands.c Outdated Show resolved Hide resolved
src/battle_script_commands.c Outdated Show resolved Hide resolved
src/battle_script_commands.c Outdated Show resolved Hide resolved
@BuffelSaft
Copy link
Collaborator Author

The pause seems to be happening for every multi-hit iteration, not only Beat Up. It's between the times the mon takes damage. Are you sure you don't notice it?

PauseMultiHit.mp4

I can definitely see it on Parental Bond, but not on other multihit moves:
PB pause
pause

- Various style fixes
- Add BATTLE_ALIVE_EXCEPT_ATTACKER case to CountAliveMonsInBattle
@BuffelSaft
Copy link
Collaborator Author

Suggested changes done!

@AsparagusEduardo
Copy link
Collaborator

I can definitely see it on Parental Bond, but not on other multihit moves

Yeah, that was my bad wording. I meant to say PB moves were also affected.

@AsparagusEduardo
Copy link
Collaborator

I think Beat Up is worth a separate config because it's essentially a whole new move effect with very different mechanics, rather than a simple data change like most updated moves get. Could move it to other move settings if that fits better?

I forgot to say that I agree with this.

This is more of a mechanical change like gen 8's change to Teleport, so it should go in the same block of settings.
@BuffelSaft
Copy link
Collaborator Author

I've moved B_BEAT_UP_DMG to other move settings. Can't see anything that would be causing the pause between hits, as that should just be flushing the message box like multi target moves do. Could tackle that as a separate issue if you don't mind? It's a little annoying but not game breaking.

include/battle.h Outdated Show resolved Hide resolved
include/constants/battle_config.h Outdated Show resolved Hide resolved
include/constants/battle_config.h Outdated Show resolved Hide resolved
@AsparagusEduardo
Copy link
Collaborator

I will take a look at look at the delay issue later today. After that and the other changes I requested now, we should be good to go :D

Rename constants, remove white space, improve B_BEAT_UP description.

* Rename B_BEAT_UP_DMG to B_BEAT_UP
* Shorten B_PARENTAL_BOND_DAMAGE to B_PARENTAL_BOND_DMG
* Remove trailing whitespace
* Replace boolean 1 with TRUE in MOVEEND_MULTIHIT_MOVE
@BuffelSaft
Copy link
Collaborator Author

Changes done!

@AsparagusEduardo
Copy link
Collaborator

So far I've noticed that the delay is happening on double battles when there's only the mon with PB and the target.

@AsparagusEduardo
Copy link
Collaborator

Found a fix to the delay. It seems to be happening every time stringId not set, not only for multi-hit moves with no effectiveness message:

    if (stringId)
+   {
        PrepareStringBattle(stringId, gBattlerAttacker);
+       gBattlescriptCurrInstr++;
+   }
+   else
+       gBattlescriptCurrInstr += 2; //Skips the following waitmessage, as there's nothing to print.
-   gBattlescriptCurrInstr++;

Copy link
Collaborator

@AsparagusEduardo AsparagusEduardo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seeing as the only missing thing is that delay fix that we already found a solution for,
I'll call this PR complete and submit the fix separetly, as it'll be better than risking getting new conflicts.

@AsparagusEduardo AsparagusEduardo merged commit b837150 into rh-hideout:upcoming Nov 2, 2022
@LOuroboros LOuroboros mentioned this pull request Nov 2, 2022
@AsparagusEduardo AsparagusEduardo mentioned this pull request Nov 10, 2022
Pokabbie added a commit to Pokabbie/pokeemerald-rogue that referenced this pull request Jul 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: battle-mechanic Pertains to battle mechanics
Projects
None yet
4 participants