-
Notifications
You must be signed in to change notification settings - Fork 135
chore: adjusted enums and fixed CPad functions #285
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
base: master
Are you sure you want to change the base?
Conversation
emil6strutui
commented
Jan 25, 2026
- added modern reversed names to eAnimationFlags
- fixed two CPad functions
- added ePedPieceTypes to CPed.h and removed the enum file
- adjusted CWeapon.h to use the new enum from CPed.h
- added modern reversed names to eAnimationFlags - fixed two CPad functions - added ePedPieceTypes to CPed.h and removed the enum file - adjusted CWeapon.h to use the new enum from CPed.h
| ANIMATION_FREEZE_TRANSLATION = 0x2000, | ||
| ANIMATION_BLOCK_REFERENCED = 0x4000, | ||
| ANIMATION_INDESTRUCTIBLE = 0x8000 | ||
| enum eAnimationFlags { |
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 do not think it is improvement. Old flags are still cleaner and easier to understand.
We do not have to blindly follow whatever guys from reversed project do.
I still did not got explanation about their policy about naming things gta-reversed/gta-reversed#1133, but seems they stick to leaked names even when these are bluntly incorrect.
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 do not agree to the old flags being easier to understand.
ANIMATION_FREEZE_LAST_FRAME -> ANIMATION_IS_BLEND_AUTO_REMOVE when it clearly is being removed when blendAmount reaches 0. How is the old name saying this from its name? From the old name I would expect the animation to freeze in the last frame, not be removed.
| ANIMATION_INDESTRUCTIBLE = 0x8000 | ||
| enum eAnimationFlags { | ||
| ANIMATION_DEFAULT = 0, //0x0, | ||
| ANIMATION_IS_PLAYING = 1 << 0, //0x1, |
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.
Why calculating flag then putting the value again in the comment? Just more visual noise. Plus comment message is fussed with //
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 found it clear to put shift operation and leave the old value as reference. What do you suggest?
| ANIMATION_IS_LOOPED = 1 << 1, //0x2, | ||
| ANIMATION_IS_BLEND_AUTO_REMOVE = 1 << 2, //!< (0x4) Automatically `delete this` once faded out (`m_BlendAmount <= 0 && m_BlendDelta <= 0`) | ||
| ANIMATION_IS_FINISH_AUTO_REMOVE = 1 << 3, //0x8, // Animation will be stuck on last frame, if not set | ||
| ANIMATION_IS_PARTIAL = 1 << 4, //0x10, // TODO: Flag name is possibly incorrect? Following the usual logic (like `ANIMATION_MOVEMENT`), it should be `ANIMATION_GET_IN_CAR` (See `RemoveGetInAnims`) |
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.
So now we are adding "possibly incorrect" names into SDK?
| ANIMATION_CAN_EXTRACT_VELOCITY = 1 << 6, //0x40, | ||
| ANIMATION_CAN_EXTRACT_X_VELOCITY = 1 << 7, //0x80, | ||
|
|
||
| // ** User defined flags ** |
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.
What does it mean? Does ANIMATION_WALK work in vanilla game?
| // ** User defined flags ** | ||
| ANIMATION_WALK = 1 << 8, //0x100, | ||
| ANIMATION_200 = 1 << 9, //0x200, | ||
| ANIMATION_DONT_ADD_TO_PARTIAL_BLEND = 1 << 10, //0x400, // Possibly should be renamed to ANIMATION_IDLE, see `CPed::PlayFootSteps()` |
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.
Again, adding TODO comments from reversing process into PSDK.
| enum eAnimationFlags { | ||
| ANIMATION_DEFAULT = 0, //0x0, | ||
| ANIMATION_IS_PLAYING = 1 << 0, //0x1, | ||
| ANIMATION_IS_LOOPED = 1 << 1, //0x2, |
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.
Why adding IS to bit flag makes it any better?
| }; | ||
|
|
||
| class CPed; | ||
| enum ePedPieceTypes : int; |
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.
Removing the include file and defining the enum base type here? This is spaghetti code.
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 needs to be forward declared due to circular dependency between CPed.h and CWeapon.h
| STYLE_ELBOWS | ||
| }; | ||
|
|
||
| enum PLUGIN_API ePedPieceTypes |
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.
Why not inside ePedPieceTypes.h?
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.
The enum is related to the ped itself, didn't seem logical to have it separated in another .h file while other enums are living in CPed.
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.
You already need it in CWeapon.