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

Document battle animation scripts #2070

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

mrgriffin
Copy link
Collaborator

@mrgriffin mrgriffin commented Nov 28, 2024

Local aliases for gBattleAnimArgs

Uses a CMD_ARGS macro similar to rh-hideout#2529, which allows the developer to name the arguments. CMD_ARGS(x, y); is similar in purpose to our #define tX data[0], #define tY data[1] pattern for naming task data.

Example:

 static void AnimHitSplatBasic(struct Sprite *sprite)
 {
-    StartSpriteAffineAnim(sprite, gBattleAnimArgs[3]);
-    if (gBattleAnimArgs[2] == ANIM_ATTACKER)
+    CMD_ARGS(x, y, relativeTo, animation);
+
+    StartSpriteAffineAnim(sprite, cmd->animation);
+    if (cmd->relativeTo == ANIM_ATTACKER)
         InitSpritePosToAnimAttacker(sprite, TRUE);
     else
         InitSpritePosToAnimTarget(sprite, TRUE);

Wrapper macros for createsprite/createvisualtask/createsoundtask

Creates wrappers with the same names as the arguments used in CMD_ARGS. The arguments are passed by name name=... rather than by position; exceptions: anim_battler and subpriority_offset for createsprite wrappers are passed by position.

createsprite wrappers which really represent tasks rather than sprites (e.g. simplepaletteblend) try to default unused_anim_battler and unused_subpriority_offset to whatever is used in the majority of cases. Sometimes GF decided to specify those values, which forces me to specify them for matching purposes (see also the mess of shakemonorterrain). Downstream projects such as RHH Expansion could consider removing these arguments and parameters.
Question: should I introduce warnings for specifying any parameters which have no purpose except matching? Possibly gated behind some define?

createvisualtask wrappers which are always called with the same priority specify that priority as a default (e.g. shakebattleterrain defaults to priority=2).

The usages of the macros do not reorder the arguments so that it's simpler for downstream users to migrate to the wrapper macros if they want to.

Example:

+       .macro createconfusionducksprite anim_battler:req, subpriority_offset:req, x:req, y:req, wave_offset:req, wave_period:req, duration:req
+       createsprite gConfusionDuckSpriteTemplate, \anim_battler, \subpriority_offset, \x, \y, \wave_offset, \wave_period, \duration
+       .endm
-       createsprite gConfusionDuckSpriteTemplate, ANIM_TARGET, 2, 0, -15, 0, 3, 90
+       createconfusionducksprite ANIM_TARGET, 2, x=0, y=-15, wave_offset=0, wave_period=3, duration=90

Special cases

TODO: Document how these don't fit the usual pattern.

  • shakemonorterrain: optional argument.
  • tintpalettes: takes color and splits it up.
  • metallicshine: no useColor, optional color.
  • setgrayscalepal & setoriginalpal: split up from AnimTask_SetGrayscaleOrOriginalPal.

TODO

A second pass over everything to make the names consistent. In particular, I think a lot of xs and ys will become x_offsets and y_offsets, and I want to verify that relative_to is only used in contexts where all the coordinates are relative to that battler (and if not, the names of parameters should specify which battler argument they're relative to).

@mrgriffin
Copy link
Collaborator Author

Just battle_anim_normal.c atm, but I'll update this PR with similar changes across the rest of the animation files on my commutes.

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.

1 participant