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

Convert 8 various to callnatives #5172

Merged
merged 3 commits into from
Sep 1, 2024
Merged

Convert 8 various to callnatives #5172

merged 3 commits into from
Sep 1, 2024

Conversation

AsparagusEduardo
Copy link
Collaborator

Description

Title.
Because Revival Blessing didn't have finished tests, I checked that it still worked in-game.

Also, set commented test to TODO test

Discord contact info

AsparagusEduardo

AlexOn1ine
AlexOn1ine reviewed Aug 14, 2024
View reviewed changes
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
@AsparagusEduardo
Copy link
Collaborator Author

Addressed review comments

AlexOn1ine
AlexOn1ine reviewed Aug 15, 2024
View reviewed changes
Copy link
Collaborator

@AlexOn1ine AlexOn1ine left a comment

Choose a reason for hiding this comment

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

If BS_ATTACKER or BS_TARGET are passed down we usually leave out the argument and reference gBattlerAttacker and gBattlerTarget in the callnative

asm/macros/battle_script.inc Show resolved Hide resolved
asm/macros/battle_script.inc Show resolved Hide resolved
asm/macros/battle_script.inc Show resolved Hide resolved
asm/macros/battle_script.inc Show resolved Hide resolved
asm/macros/battle_script.inc Show resolved Hide resolved
@AlexOn1ine AlexOn1ine added category: battle-mechanic Pertains to battle mechanics type: cleanup labels Aug 17, 2024
@pkmnsnfrn pkmnsnfrn marked this pull request as draft August 28, 2024 02:59
@AsparagusEduardo
Copy link
Collaborator Author

If BS_ATTACKER or BS_TARGET are passed down we usually leave out the argument and reference gBattlerAttacker and gBattlerTarget in the callnative

Doesn't it make more sense for the functions to be allow any battler so they're more flexible in scripting?

@AlexOn1ine
Copy link
Collaborator

If BS_ATTACKER or BS_TARGET are passed down we usually leave out the argument and reference gBattlerAttacker and gBattlerTarget in the callnative

Doesn't it make more sense for the functions to be allow any battler so they're more flexible in scripting?

No, this doesn't matter here. It is just wasting cycles.

AlexOn1ine
AlexOn1ine reviewed Aug 31, 2024
View reviewed changes
Copy link
Collaborator

@AlexOn1ine AlexOn1ine left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Actually I double checked and the battler argument here is correct.

Please solve conflicts

@AlexOn1ine AlexOn1ine marked this pull request as ready for review August 31, 2024 11:43
@AsparagusEduardo
Copy link
Collaborator Author

Conflicts solved

@AlexOn1ine AlexOn1ine merged commit 484acdc into rh-hideout:upcoming Sep 1, 2024
1 check passed
@AsparagusEduardo AsparagusEduardo deleted the _RHH/pr/upcoming/variousToCallNative branch September 1, 2024 13:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AlexOn1ine AlexOn1ine AlexOn1ine approved these changes

Assignees
No one assigned
Labels
category: battle-mechanic Pertains to battle mechanics type: cleanup
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@AsparagusEduardo @AlexOn1ine