-
Notifications
You must be signed in to change notification settings - Fork 40
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
libtrx/anims/common: move ANIMs to TRX #2230
Conversation
This makes a common accessor function to retrieve animations, although a temporary global g_Anims remains until all calls are migrated. Utility functions are also added to retrieve Item and Object animations easily.
This replaces all direct references to g_Anims (other than level and injection loading) to use Anim_GetAnim or Item_GetAnim as appropriate.
This replaces all direct references to g_Anims (other than level and injection loading) to use Anim_GetAnim or Item_GetAnim as appropriate.
This eliminates the g_Anims global for both games and moves and moves reading from level/injection files to libtrx.
This moves Item_SwitchToObjAnim, Item_TestFrameEqual and Item_TestFrameRange to TRX now that the dependency on g_Anims is removed.
Download the built assets for this pull request: |
@@ -49,7 +49,7 @@ void Statue_Initialise(int16_t item_num) | |||
Item_Initialise(centaur_item_num); | |||
|
|||
Item_SwitchToAnim(centaur, CENTAUR_REARING_ANIM, CENTAUR_REARING_FRAME); | |||
centaur->current_anim_state = g_Anims[centaur->anim_num].current_anim_state; | |||
centaur->current_anim_state = Item_GetAnim(centaur)->current_anim_state; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Am I crazy or is this doing nothing? It's setting centaur->current_anim_state
to centaur
's current_anim_state
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's setting the item's state to that of the animation, which is 6 in this case so it's in the rearing state.
ASSERT(interpolation <= 0xFF); | ||
anim->interpolation = interpolation & 0xFF; | ||
anim->frame_size = 0; | ||
#else |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these else
consistent with all later TRs? Is it worth explicitly saying #if TR_VERSION == 2
to set up for potentially a TR3X
or later that is different?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TR3 uses the same format as TR2. Arguably, TR1 is the same in terms of struct size, it's just that frame size was introduced in TR2 to deal with the way rotations are packed.
TR4 and 5 have extra (unrelated) properties (lateral speed and acceleration IIRC).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's OK – we'll have to go through every single if TR_VERSION
when thinking to support more games anyway.
} else { | ||
anim_base = g_Objects[O_LARA_FLARE].anim_idx + 4; | ||
anim_idx = 4; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it worth adding enums for 1-4 for flares as well as pistols? I guess the frame enums in the if test kinda tell you what these anim_idx
are setting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should. I will follow it up separately though if we're OK with that as I've just realised I didn't make use of Anim_TestAbsFrameRange
in Gun_Pistols_SetArmInfo
in #2224. Makes sense to do these together.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed and it's OK to do this in a follow-up PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks. Can't wait to see the ANIM_FRAME arrive to TR2 as well!
ASSERT(interpolation <= 0xFF); | ||
anim->interpolation = interpolation & 0xFF; | ||
anim->frame_size = 0; | ||
#else |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's OK – we'll have to go through every single if TR_VERSION
when thinking to support more games anyway.
} else { | ||
anim_base = g_Objects[O_LARA_FLARE].anim_idx + 4; | ||
anim_idx = 4; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed and it's OK to do this in a follow-up PR
Checklist
Description
This migrates
ANIM
storage to libtrx and introduces accessors rather than using globals. We have three functions essentially for getting anANIM
:Anim_GetAnim(n)
- gets an anim by global indexItem_GetAnim(item)
- gets the given item's current animObject_GetAnim(obj, n)
gets thenth
anim of the given objectAnimations are read commonly in libtrx for both games, plus TR1 injections. Frame setup is still as it was, but this will be refactored later separately. All anim and frame switching/testing functions are also now in libtrx.