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 original player animation names in gameplay_keep #1786

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

tom-overton
Copy link
Collaborator

This PR might seem a bit redundant, since the player animations are currently named after their original names (if the original name is available). The reason I want to do this, however, is that player animation names are probably going to change in the future, and I want to preserve these original names in the comments. This might seem silly:

<PlayerAnimation Name="gPlayerAnim_link_keirei" Offset="0xD9F8" /> <!-- Original name might be "link_keirei" -->

But it seems much more reasonable when the animation is renamed:

<PlayerAnimation Name="gPlayerSaluteAnim" Offset="0xD9F8" /> <!-- Original name might be "link_keirei" -->

I've described the history of how I derived these original animation names in previous PRs, but to summarize, Grezzo included almost all of the original animation data (just converted to little endian) for OoT's animations in OoT3D; for these animations, I wrote in the comment that the "original name is [name]", since we can be extremely confident in the names of these ones. For animations new to MM, I needed to look at the animation in MM3D and try to match it up with the N64 asset. Since I could be wrong about this (though I believe I got them correct; note how the original animation names follow a consistent alphabetical scheme), for these animations I wrote in the comment that the "original name might be [name]". A secondary benefit of this PR is that the repo itself now contains this distinction between original animation names that we know almost for certain and original animation names that might not be entirely accurate.

@hensldm hensldm added Merge-ready All reviewers satisfied, just waiting for CI and removed Needs-second-approval Second approval Needs-first-approval First approval labels Jan 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Asset Merge-ready All reviewers satisfied, just waiting for CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants