Skip to content

Conversation

ghost
Copy link

@ghost ghost commented Apr 3, 2018

EDIT: Github automatically closed my PR while I was trying to fix it, so I'm creating a new one.

I've implemented custom IFP animations into MTA while touching the current animation system as little as possible. I've written my own IFP loader that will allow scripters to load their own IFP file using engineLoadIFP and play animations using setPedAnimation, or replace and restore them using engineReplaceAnimation and engineRestoreAnimation respectively. The code is client-side only.

There are three new Lua functions added:

// loads IFP with a custom block name 
ifp engineLoadIFP ( string ifp_file_path, string custom_block_name ) 

// this will replace an internal GTA animation with custom one, it's a light-weight operation,
// don't worry too much about performance. Different peds can have different running, walking,
// crouching, shooting, etc. animations all running simultaneously because we are not actually
// replacing animation hierarchies, we are merely storing everything in std::map which is in
// CClientPed. When an animation triggers, we get the ped by clump, and play the animation
// we wish to play. setPedAnimation works a little different than this, but the idea is same.
bool engineReplaceAnimation ( ped thePed, string block_name, string anim_name, string custom_block_name, string custom_anim_name )

// This will restore animation replaced using engineReplaceAnimation, if only 1 parameter 
// is provided which is ped, then it will restore all animations, if  block name is also provided,
// then it will restore animations of that block only, if animation name is provided as well,
// it will restore that specific animation only.
bool engineRestoreAnimation ( ped thePed [, string block_name, string anim_name ] )

To unload IFP, use destroyElement, and to play the custom animation simply use setPedAnimation. Also, I've tested everything countless times, but if you find something that is not the way it should be then please let me know.

This is stable because animation loading, playing, and unloading is handled by my custom IFP loader, so we know exactly how it works. I did not use any hackish techniques in my code. I've tried my best sticking to KISS principle, so anyone can maintain it.

…rn type of StaticAddAnimationHandler and AddAnimationHandler to CAnimBlendAssociation *
…pAddanimationHandler is nullptr/NULL then normal function flow executes to avoid crash
…mapOfPedClumps and new clump is inserted into the map
…ck to whom the clump belongs and what animation to play, it can be a custom animation as well.
…ing missing vTable, nNumBlendNodes, and pAnimBlendNodesSequeceArray
…ciation, also implemented isGateWayAnimationHierarchy
…added gateway animation constant, value is set to "run_wuzi" for gateway animation
…dler for checking and playing custom animation, but it is not tested because IFP reader needs to be added asap.
…ass. Also, renamed it to m_kGateWayAnimationName.
CFileReader is using ifstream to load file into std::vector. No more dynamic memory allocation.
&
replaced std::vector::push_back with std::vector::emplace_back in CElementDeleter::OnClientIFPElementDestroy
& Removed empty destructor from CClientIFP and empty constructor from CIFPAnimations
- Suggested by Necktrox

Used for (auto& pPed : m_setOfPedPointers) for iterating in CClientGame::OnClientIFPUnload.
&
using std::find_if in CElementDeleter::DeleteIFP with a lambda function
- Suggested by sbx320

Removed #pragma once in CClientIFP.h and CIFPAnimations.h
- Suggested by sbx320 and Necktrox
Replaced uint32_t, int32_t, and int16_t with std::* from <cstdint>
- Suggested by Necktrox

Used std::find_if in CClientIFP::GetAnimationHierarchy
& improved iterator loops by using
for (auto& Animation : m_pIFPAnimations->vecAnimations)  in CClientIFP::ReadIFPVersion2 and CClientIFP::ReadIFPVersion1
- earlier suggested by sbx320

Replaced std::string with SString.

Changed CIFPAnimations::SIFPAnimation::pSequencesMemory type to BYTE*. Removed casting where this member of struct was accessed.

Used const where applicable in CClientIFP.cpp.

Renamed ConvertStringToMapKey to ConvertStringToKey.

Applied source formatting again
@ghost
Copy link
Author

ghost commented Apr 5, 2018

Something important about engineReplaceAnimation:

I forgot to mention that partial animations are not supported for replacing, if you replace them, MTA won't crash, but the animation will get glitched, most probably not what you want. Those are animations which usually run parallel with other animations like punching while running can't be replaced. You can replace running, punching, crouching, shooting with weapons, swimming, and hundreds of more similar animations, but there are few that you can't replace properly, and those few animations are partial animations.

The only reason why I did not fix this is because it will require me to rewrite the entire animation system including GTA task system. The amount of bones to be moved for every animation is hardcoded in GTA:SA for every task, and it will literally take me 4-5 more months to completely reverse it, and a few more to write code properly for MTA. That's roughly 1 year :)

I do have plans to add support for partial animations and the ability to have any amount of bones. Currently, bone limit is 32. I want to lift this limit and add a few more functions like setPedBonePosition and setPedBoneRotation. Hopefully, next year.

@@ -11,6 +11,9 @@

#include "StdInc.h"
#include <net/SyncStructures.h>
#include <../Client/game_sa/CAnimBlendAssocGroupSA.h>
#include <../Client/game_sa/CAnimBlendAssociationSA.h>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GameSA headers should not be used here. Instead, add an interface to the Game SDK (/Client/sdk/game).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. I forgot to ask you what to do with them. I'll fix them.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

…ments

Replaced GameSA includes with interfaces
- Suggested by Jusonex

Removed #pragma from CIFPEngine.h
eRestoreType is now passed by const reference in CIFPEngine::EngineRestoreAnimation
@multitheftauto multitheftauto deleted a comment from neves768 Apr 6, 2018
@Pirulax
Copy link
Contributor

Pirulax commented Apr 10, 2018

Look pretty nice, this is what i was waiting for, thanks for it, u did an awesome job, even do, i never wrote a single line of C++ :D

They were not needed. I used them because I was in doubt whether GTA:SA is using a secondary thread. I confirmed that it's not using another thread with the help of SharedUtil::IsMainThread() method.
- Suggested by Jusonex
Rewrote HOOK_CAnimBlendAssocGroup_CopyAnimation in C++ for more clarity, it is shorter now, and thus more readable.

Moved HOOK_CAnimBlendAssoc_destructor to CMultiplayerSA_HookDestructors.cpp
Visual studio was generating a warning when calling delete operator on hierarchy and sequnece interface in Client Deathmatch, so replaced it with CAnimManager::DeleteCustomAnimHierarchyInterface and CAnimManager::DeleteCustomAnimSequenceInterface in CIFPAnimations::DeleteAnimations and CClientIFP::MoveSequencesWithDummies

Also, applied source formatting
@neves768
Copy link

neves768 commented Apr 30, 2018

Thx for your hard work on it, @codenulls and @saml1er. You're building a new high-attractive thing to MTA:SA. I hope it gets into the game soon. Also, thanks for the two reviewers that are taking their times to analyze and test it. ❤️

@botder botder added enhancement New feature or request todo labels Apr 30, 2018
@@ -0,0 +1,1079 @@
#include <StdInc.h>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

File is missing the copyright/etc. comment at the top

Copy link
Author

@ghost ghost May 3, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the help. This is how the current copyright comment looks like:

 *
 *  PROJECT:     Multi Theft Auto v1.0
 *  LICENSE:     See LICENSE in the top level directory
 *  FILE:        <DIRECTORY>/<FILENAME>.cpp
 *  PURPOSE:     <PURPOSE HERE>
 *
 *  Multi Theft Auto is available from http://www.multitheftauto.com/
 *
 *****************************************************************************/

Should I change Multi Theft Auto v1.0 to Multi Theft Auto v1.5? I'll do the same for other files.

Copy link
Contributor

@qaisjp qaisjp May 3, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, that's much better.

if (bAnp2 || bAnp3)
{
m_bVersion1 = false;
ReadIFPVersion2(bAnp3);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if the IFP file is corrupt? Shouldn't this function call return a boolean to indicate failure? See also ReadIFPVersion1

Copy link
Author

@ghost ghost May 3, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is how GTA:SA was reading IFP files. It's easy to check for corruption in case of DFF, it has multiple headers throughout the file, but IFP has only 1 header which is at the top, we can use the first 8 bytes to check:

struct SIFPHeader 
{
    char         Version [ 4 ]; 
    std::int32_t EndOfFileOffset; 
};

We can add two checks:

Version
It can have three possible strings, "ANPK," "ANP2," and "ANP3," if we don't find these strings, we can assume that it's corrupt, and return false.

EndOfFileOffset
We can do if ( (LengthOfFile - 8bytes) == EndOfFileOffset ) if condition is false, we can assume it's corrupt.

The client can still crash, if the header is fine, but the data inside the IFP file is corrupt. Nonetheless, adding these two checks will certainly help.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like IFP files modified with KAM's script have incorrect EndOfFileOffset, so we cannot rely on that, I just confirmed it. We can only check the first four bytes.

@@ -0,0 +1,285 @@
#ifndef __CCLIENTIFP_H
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

File is missing the copyright/etc. comment at the top.
Also, use #pragma once

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

About #pragma once, I remember using it for the header files, but I was told to remove it. It does no harm, I'll add it back.

@@ -0,0 +1,51 @@
#include <StdInc.h>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

File is missing the copyright/etc. comment at the top

@@ -0,0 +1,36 @@
#ifndef CFILEREADER_H
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

File is missing the copyright/etc. comment at the top.
Also, use #pragma once

@@ -0,0 +1,22 @@
#ifndef __CIFPANIMATIONS_H
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

File is missing the copyright/etc. comment at the top.
Also, use #pragma once

@@ -0,0 +1,91 @@
#include <StdInc.h>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

File is missing the copyright/etc. comment at the top

@@ -0,0 +1,21 @@
#ifndef __CIFPENGINE_H
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

File is missing the copyright/etc. comment at the top.
Also, use #pragma once

@@ -0,0 +1,283 @@
#include "StdInc.h"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

File is missing the copyright/etc. comment at the top

@@ -111,6 +119,6 @@ void _declspec(naked) HOOK_GetAnimHierarchyFromSkinClump()
//////////////////////////////////////////////////////////////////////////////////////////
void CMultiplayerSA::InitHooks_FixBadAnimId(void)
{
EZHookInstall(CAnimBlendAssocGroupCopyAnimation);
// EZHookInstall ( CAnimBlendAssocGroupCopyAnimation );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any reason not to remove this line completely?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch. It should be removed completely.

@qaisjp qaisjp changed the base branch from master to feature/custom-ifp-anims June 7, 2018 08:15
@qaisjp qaisjp changed the base branch from feature/custom-ifp-anims to master June 7, 2018 08:16
@qaisjp qaisjp changed the base branch from master to feature/custom-ifp-anims June 7, 2018 08:16
@qaisjp qaisjp changed the base branch from feature/custom-ifp-anims to master June 7, 2018 08:17
Merge master into custom_ifp_animations
@jushar
Copy link
Contributor

jushar commented Jun 18, 2018

See 15f99b2

@jushar jushar closed this Jun 18, 2018
@patrikjuvonen patrikjuvonen assigned ghost Sep 4, 2018
@ghost ghost deleted the custom_ifp_animations branch October 6, 2020 05:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants