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

Fixed switch-in abilities activating on terrain change #2881

Merged
merged 2 commits into from
Apr 7, 2023

Conversation

AgustinGDLV
Copy link
Collaborator

@AgustinGDLV AgustinGDLV commented Apr 3, 2023

Description

Fixes #2880. This PR removes an erroneous call to activateswitchinabilities in the TerrainSeed loop and combines both the TerrainSeedLoop and TerrainAbilitiesLoop into one loop for terrain effects so you don't have to call each individually (and so it doesn't have to loop over every battler twice).

This PR also adds tests for every terrain effect! This includes some edge cases for Psychic Terrain that appear to be inaccurate at the moment, so they have been marked with KNOWN_FAILING. These bugs have been pointed out in #2882, but are not fixed in this PR.

@ghoulslash
Copy link
Collaborator

ghoulslash commented Apr 3, 2023

I think it should instead be replaced with a call to ActivateTerrainChangeAbilities. On mobile so can't confirm right now but that's my intuition

@AgustinGDLV
Copy link
Collaborator Author

I think it should instead be replaced with a call to ActivateTerrainChangeAbilities. On mobile so can't confirm right now but that's my intuition

BattleScript_EffectMistyTerrain:
BattleScript_EffectGrassyTerrain:
BattleScript_EffectElectricTerrain:
BattleScript_EffectPsychicTerrain:
	attackcanceler
	attackstring
	ppreduce
	setterrain BattleScript_ButItFailed
	attackanimation
	waitanimation
	printfromtable gTerrainStringIds
	waitmessage B_WAIT_TIME_LONG
	playanimation BS_ATTACKER, B_ANIM_RESTORE_BG
	call BattleScript_ActivateTerrainAbilities
	call BattleScript_TerrainSeedLoop
	goto BattleScript_MoveEnd

The terrain seed script and the terrain abilities script are called separately in all the terrain effect scripts, so it doesn't need to be swapped. This bug is probably a leftover from these scripts being changed when Quark Drive was added.

@ghoulslash
Copy link
Collaborator

I think it should instead be replaced with a call to ActivateTerrainChangeAbilities

Those loops ought to be merged then. No reason to loop twice when you can check seeds and terrain change abilities in the same loop iteration

@AgustinGDLV
Copy link
Collaborator Author

Oh huh, good idea. I will commit that when I can.

@AgustinGDLV
Copy link
Collaborator Author

Updated and ready for review!

@ghoulslash ghoulslash merged commit 3a710a6 into rh-hideout:upcoming Apr 7, 2023
@AsparagusEduardo AsparagusEduardo mentioned this pull request May 31, 2023
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.

Terrain changes activate all switch-in battle abilities
2 participants