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.

Sign up for GitHub

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

Jump to bottom

Fling fixes and tests #3191

Merged
merged 13 commits into from
Sep 5, 2023
Merged

Fling fixes and tests #3191

merged 13 commits into from
Sep 5, 2023

Conversation

DizzyEggg
Copy link
Collaborator

Fixes #3001

  • Tests for Fling
  • Changed the Fling various to a call native
  • removed unnecessary lines from the Fling battlescript
  • made kee berry and maranga berry be affected by fling

AsparagusEduardo
AsparagusEduardo requested changes Aug 1, 2023
View reviewed changes
test/move_effect_fling.c Outdated Show resolved Hide resolved
test/move_effect_fling.c Outdated Show resolved Hide resolved
test/move_effect_fling.c Outdated Show resolved Hide resolved
test/move_effect_fling.c Outdated Show resolved Hide resolved
test/move_effect_fling.c Outdated Show resolved Hide resolved
test/move_effect_fling.c Outdated Show resolved Hide resolved
test/move_effect_fling.c Outdated Show resolved Hide resolved
test/move_effect_fling.c Outdated Show resolved Hide resolved
include/battle_scripts.h Outdated Show resolved Hide resolved
src/battle_script_commands.c Show resolved Hide resolved
DizzyEggg and others added 9 commits August 2, 2023 09:02
@DizzyEggg @AsparagusEduardo
Update test/move_effect_fling.c
ad31bd3 
Co-authored-by: Eduardo Quezada D'Ottone <[email protected]>
@DizzyEggg @AsparagusEduardo
Update test/move_effect_fling.c
04b9cc4 
Co-authored-by: Eduardo Quezada D'Ottone <[email protected]>
@DizzyEggg @AsparagusEduardo
Update test/move_effect_fling.c
88e6e17 
Co-authored-by: Eduardo Quezada D'Ottone <[email protected]>
@DizzyEggg @AsparagusEduardo
Update test/move_effect_fling.c
afc9987 
Co-authored-by: Eduardo Quezada D'Ottone <[email protected]>
@DizzyEggg @AsparagusEduardo
Update test/move_effect_fling.c
046b05f 
Co-authored-by: Eduardo Quezada D'Ottone <[email protected]>
@DizzyEggg @AsparagusEduardo
Update test/move_effect_fling.c
fa5b514 
Co-authored-by: Eduardo Quezada D'Ottone <[email protected]>
@DizzyEggg @AsparagusEduardo
Update test/move_effect_fling.c
1f045c6 
Co-authored-by: Eduardo Quezada D'Ottone <[email protected]>
@DizzyEggg @AsparagusEduardo
Update test/move_effect_fling.c
2321797 
Co-authored-by: Eduardo Quezada D'Ottone <[email protected]>
@DizzyEggg
scripts names change
d2a7f4d
ghoulslash
ghoulslash previously requested changes Aug 6, 2023
View reviewed changes
data/battle_scripts_1.s Outdated Show resolved Hide resolved
include/constants/battle_script_commands.h Show resolved Hide resolved
@DizzyEggg
Copy link
Collaborator Author

What is blocking this PR from getting merged?

@AsparagusEduardo
Copy link
Collaborator

What is blocking this PR from getting merged?

Haven't received a response from ghoul from my replies to their comments ^^;

@ghoulslash
Copy link
Collaborator

What is blocking this PR from getting merged?

Haven't received a response from ghoul from my replies to their comments ^^;

It's fine to merge. I disagree with the battle script labeling but it's not worth blocking the pr over

@AsparagusEduardo
Copy link
Collaborator

It's fine to merge. I disagree with the battle script labeling but it's not worth blocking the pr over

Let me get a third opinion over a PR on Pret (I kept forgetting to do it ^^)

@AsparagusEduardo
Copy link
Collaborator

PR opened over at pret: pret#1906 👀

@AsparagusEduardo AsparagusEduardo marked this pull request as draft August 24, 2023 01:05
@AsparagusEduardo AsparagusEduardo marked this pull request as ready for review August 24, 2023 01:05
@AsparagusEduardo
Copy link
Collaborator

@ghoulslash how about pret's current labels?

@ghoulslash
Copy link
Collaborator

@ghoulslash how about pret's current labels?

Seems fine to me

@DizzyEggg
Copy link
Collaborator Author

Anything left to change here? 👀

@AsparagusEduardo
Copy link
Collaborator

Anything left to change here? 👀

Applying pret's current labels.
Also, I think that the "removing item and then fail" should have a goto and be next to the other fling scripts

@DizzyEggg
Copy link
Collaborator Author

Sorry, I'm kinda lost here. Which labels should be changed to what exactly?
BattleScript_ButItFailedPpReduceAtkString back to BattleScript_ButItFailedAtkStringPpReduce? BattleScript_ButItFailedPpReduceAtkStringRemoveItem to BattleScript_ButItFailedAtkStringPpReduceRemoveItem?

@AsparagusEduardo
Copy link
Collaborator

Sorry, I'm kinda lost here. Which labels should be changed to what exactly? BattleScript_ButItFailedPpReduceAtkString back to BattleScript_ButItFailedAtkStringPpReduce? BattleScript_ButItFailedPpReduceAtkStringRemoveItem to BattleScript_ButItFailedAtkStringPpReduceRemoveItem?

  • BattleScript_ButItFailedPpReduce -> BattleScript_FailedFromPpReduce (current label)
  • BattleScript_ButItFailedPpReduceAtkString -> BattleScript_FailedFromAtkString (current label)
  • Move BattleScript_ButItFailedPpReduceAtkStringRemoveItem to be below BattleScript_FlingEnd
BattleScript_FlingEnd:
	tryfaintmon BS_TARGET
	trysymbiosis
	goto BattleScript_MoveEnd

+BattleScript_FlingFailConsumeItem
+	removeitem BS_ATTACKER
+	goto BattleScript_FailedFromAtkString

BattleScript_FlingFlameOrb:

@DizzyEggg
Copy link
Collaborator Author

@AsparagusEduardo @ghoulslash Fixed, hopefully the way you wanted

@AsparagusEduardo AsparagusEduardo dismissed ghoulslash’s stale review September 5, 2023 09:56

Ghoul approved the merge

@AsparagusEduardo AsparagusEduardo merged commit cfd8024 into rh-hideout:master Sep 5, 2023
1 check passed
@AsparagusEduardo AsparagusEduardo mentioned this pull request Sep 27, 2023
Merged
@DizzyEggg DizzyEggg deleted the fling branch October 1, 2023 21:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ghoulslash ghoulslash ghoulslash left review comments

@AsparagusEduardo AsparagusEduardo AsparagusEduardo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Fling doesn't check if move is successful before consuming item.
3 participants
@DizzyEggg @AsparagusEduardo @ghoulslash