Add Battle Animations module - #352
Conversation
…euristics for finding animation count. Attempt to optimise
…animations may need it
|
Still missing one feature: patching the game code to accept higher battle animation IDs than usual. Oops |
tolmar
left a comment
tolmar
left a comment
There was a problem hiding this comment.
Looks good!
I read through everything and only had a few comments.
Note that I just read through everything, I did not build or test it myself.
| NAME = "Battle Animations" | ||
|
|
||
| # Animations config, arrangements, arrangement pointers, tilesets, and palettes | ||
| FREE_RANGES = [(0x0C2E19, 0x0CF617)] |
There was a problem hiding this comment.
Sick, that's almost a whole bank of new space mapped out.
| # Now we need to guess the length of things. | ||
| guessed_length = 0 | ||
| while True: | ||
| # Keep going until any of these happen: |
There was a problem hiding this comment.
Just for my curiosity - do any of these trigger more than the others? Did different projects you sample get hit by different heuristics? (If you didn't record it, don't check on my account, I'm just curious.)
There was a problem hiding this comment.
For vanilla ROMs and ones with expanded animations ROMs following the usual method, it'll hit an invalid/null arrangement pointer before anything else. If you were to fill up all the expanded animations and have the palette of the first animation configured in a certain way it'd probably run into the decompressed arrangement data check and fail
| tilesets_max_ID_used = [] | ||
|
|
||
| for animation_id in range(self.battle_animation_table.num_rows): | ||
| try: |
There was a problem hiding this comment.
this try block is very large, can you shrink the area it covers? I assume it's just the resource_opens that require it?
There was a problem hiding this comment.
I figure if anything at all goes wrong here, the user would want to know which animation was the problem one. Is there any particular downside to a large try block?
There was a problem hiding this comment.
Yeah, overly-large try blocks can end up hiding more errors than you meant to. It's generally best practice to scope them down to things that are really supposed to break out of your control sometimes (like file access).
I guess in this case it's going to be reported to the user as an error anyway, so it's probably not covering anything up? It's just something I look for reflexively.
There was a problem hiding this comment.
As far as I'm aware, this wouldn't get in the way of any other error handling, it just attaches a nice message for the user to it. So it'll be fine. It still bubbles back up the stack
| message = "Encountered an error while reading battle animation #{}.".format(animation_id) | ||
| raise CoilSnakeTraceableError(message, e) | ||
|
|
||
| # Trim tilesets past max ID |
…ated per-animation
|
OK, after some discussion on the discord server, I've changed the way we're dumping so that tilesets are no longer duplicated when decompiling and deduplicated when compiling. This makes it easier for external editors to handle sharing tilesets across animations. I'll update the PR description to reflect this |
2 participants
This introduces the Battle Animations module, which is responsible for decompiling+recompiling data related to battle animations (aka PSI animations). To support dumping edited data from the current usual method of editing battle animations, which requires direct edits to a project's base ROM plus a CCScript repointing of the data, the project upgrade process now also optionally accepts an existing compiled ROM either as a prompt in the GUI or the new CLI flag
--existing_rom/-e.Battle animations are dumped into the new
BattleAnimationsdirectory, which includes thebattle_animations.ymlconfiguration file, theTilesetssubdirectory which includes grayscale PNGs of each of the tilesets, and theArrangementssubdirectory which includes the individual frame tilemaps for every animation. Like the vanilla game, tilesets compiled into the ROM do not contain extraneous tiles.Brand-new animations can be added by creating a new
.maparrangement file with the appropriate number filename and adding a corresponding entry tobattle_animations.yml. If there are more animations than vanilla, then a small code patch will be applied so that the function at C3F981 can process them. Additional battle animation IDs start at 55 (56 in CCScript) due to battle animations and the HDMA-based enemy animations sharing the same ID space.