Added dynamic ped ID allocating(based on lopezloo pull request) #151

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

Jump to bottom

Merged

qaisjp merged 15 commits into multitheftauto:master from Neproify:dynamicidallocation

Aug 23, 2018

Merged

Added dynamic ped ID allocating(based on lopezloo pull request) #151

qaisjp merged 15 commits into multitheftauto:master from Neproify:dynamicidallocation

Aug 23, 2018

Conversation

@Neproify

Copy link

Contributor

@Neproify Neproify commented Jun 28, 2017 •

edited by qaisjp

Loading

Authors: @lopezloo, @Neproify, @ArranTuna

Dynamic ped ID allocation based on @lopezloo pull request - #65
With api proposed by @sbx320

Functions:

engineRequestModel(string type)

type: {"ped"}(Other things can be added in future?)
Returns: bool or int
bool: false if model wasn't allocated
int: ID of allocated model

engineFreeModel(int modelID)

Returns: bool
true if model was deallocated
false if it wasn't

How it looks like?
mta-screen_2017年06月28日_20-43-37
mta-screen_2017年06月28日_20-43-42

Test resource:
testresource.tar.gz

@Neproify


 Added dynamic ped ID allocating(based on lopezloo pull request)

8e9cf02

sbx320

sbx320 previously requested changes

Jun 28, 2017

View reviewed changes

Copy link

Member

@sbx320 sbx320 left a comment

Choose a reason for hiding this comment

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

good work 👍

Some minor things:

(void) is pointless, just remove the void there
nullptr should be preferred to NULL

Client/mods/deathmatch/logic/luadefs/CLuaEngineDefs.cpp Outdated

if ( !argStream.HasErrors () )

{

eClientModelType eModelType;

if ( stricmp( strModelType, "ped" ) == 0 )

Copy link

Member

Choose a reason for hiding this comment

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

strModelType == "ped" would be better here. No real need to make it case-insensitive or use C functions.

Client/mods/deathmatch/logic/luadefs/CLuaEngineDefs.cpp Outdated

CClientModel * pModel = new CClientModel( m_pManager, iModelID, eModelType );

pModel->Allocate ();

m_pManager->GetModelManager ()->Add ( pModel );

Copy link

Member

Choose a reason for hiding this comment

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

You are already doing m_pModelManager->Add ( this ); in the constructor of CClientModel.

Ideally we'd eliminate the use of new entirely and just use unique_ptrs for the managers in general to avoid memleaks entirely.

Client/game_sa/CModelInfoSA.cpp Outdated

m_dwModelID <= 288) ||

(m_dwModelID >= 290 &&

m_dwModelID <= 312));

return ( GetInterface() && GetInterface()->pColModel && GetInterface()->pColModel == ( CColModelSAInterface * ) VAR_CTempColModels_ModelPed1 );

Copy link

Member

Choose a reason for hiding this comment

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

Did you check whether all peds always have a colmodel VAR_CTempColModels_ModelPed1? The 1 seems to imply there might be more.

Copy link

Contributor Author

@Neproify Neproify Jun 28, 2017

Choose a reason for hiding this comment

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

Not sure about that. That was based on lopez research.

Copy link

Contributor Author

@Neproify Neproify Jun 28, 2017

Choose a reason for hiding this comment

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

Looks like there is only one collision? Seems weird...

http://gtaforums.com/topic/809134-restoring-the-pedscol/

Copy link

Contributor Author

@Neproify Neproify Sep 26, 2017

Choose a reason for hiding this comment

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

Just to end the topic - GTA is using hardcoded ped collision, so all peds are using the same collision. @sbx320

@Neproify


 Don't add model to manager twice, use SString == operator, not stricmp.

8414eca

@qaisjp qaisjp added the enhancement label

Jul 2, 2017

@Necktrox Necktrox dismissed sbx320’s stale review

February 18, 2018 13:19

"GTA is using hardcoded ped collision, so all peds are using the same collision"

@Necktrox

Copy link

Necktrox commented Feb 18, 2018

During testing I noticed that you can leak model IDs when you execute the /allocate commands multiple times. Restarting the resource doesn't deallocate the allocated IDs, which should be done by MTA, not the resource.

@Necktrox Necktrox added feedback pr:needs-testing labels

Feb 18, 2018

@Neproify


 Merge branch 'master' into dynamicidallocation

8d232b5

@botder botder added refactor review-required and removed enhancement pr:needs-testing labels

Jul 9, 2018

@qaisjp qaisjp removed the review required label

Jul 14, 2018

Neproify added 2 commits

July 30, 2018 16:15

@Neproify


 Merge branch 'master' of https://github.com/multitheftauto/mtasa-blue ...

404fb7f

...into dynamicidallocation

@Neproify


 Deallocate models that were allocated by resource that is unloading

eb27178

@Neproify

Copy link

Contributor Author

Neproify commented Jul 31, 2018

@Necktrox Fixed.

BTW. It is almost year later... I'm lazy guy. :P

qaisjp

qaisjp reviewed

Jul 31, 2018

View reviewed changes

Client/mods/deathmatch/logic/CClientModel.h Outdated

eClientModelType GetModelType ( void ) { return m_eModelType; };

bool Allocate ( void );

bool Deallocate ( void );

void SetResourceThatAllocatedIt(CResource* pResource) { m_pOwnedByResource = pResource; }

Copy link

Contributor

@qaisjp qaisjp Jul 31, 2018

Choose a reason for hiding this comment

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

a better name might be SetParentResource with a comment "This is the resource that allocated this model"

qaisjp

qaisjp reviewed

Jul 31, 2018

View reviewed changes

Client/mods/deathmatch/logic/CClientModel.cpp Outdated

pModelInfo->DeallocateModel ();

this->SetResourceThatAllocatedIt(NULL);

Copy link

Contributor

@qaisjp qaisjp Jul 31, 2018

Choose a reason for hiding this comment

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

we use nullptr wherever possible now

@Neproify


 Just some minor code changes.

6343aca

@Neproify

Copy link

Contributor Author

Neproify commented Aug 3, 2018

@qaisjp Is everything fine now?

qaisjp added 8 commits

August 23, 2018 23:18

@qaisjp


 Merge branch 'master' into dynamicidallocation

f05130d

@qaisjp


 Revert changes to files that break git diff

c52a118

@qaisjp


 Reapply changes to files that broke git diff

0d36dc0

@qaisjp


 Fix formatting

2af7982

@qaisjp


 Fix formatting (2)

ced9120

@qaisjp


 Fix formatting (3)

681d15b

@qaisjp


 Replace NULL with nullptr

ebe37d7

@qaisjp


 Use lt not lte for MAX_MODEL_ID

b60d7e8

@qaisjp


 Fix model ID check

bfdaecb

@qaisjp qaisjp merged commit 34d4eed into multitheftauto:master

Aug 23, 2018

qaisjp added a commit that referenced this pull request

Aug 29, 2018

@qaisjp


 Revert "Add dynamic ped ID allocating FOR TESTING ( #151 )"

242684e

Needs further testing. Lets make custom animations the big 1.5.6
feature.
This reverts commit 34d4eed.

@dretax

Copy link

dretax commented Sep 1, 2018

Heh. Nice one.

qaisjp added a commit that referenced this pull request

Sep 8, 2019

@qaisjp @lopezloo @Neproify @ArranTuna


 Re-add dynamic ped ID allocating ( #349 )

475544f

This reverts commit 242684e.
Original PR can be found at #151.
Co-authored-by: lopezloo <lopez@multitheftauto.pl>
Co-authored-by: Neproify <Neproify@users.noreply.github.com>
Co-authored-by: Arran <arrantuna@hotmail.co.uk>
Co-authored-by: Qais Patankar <qaisjp@gmail.com>

Labels

feedback refactor

6 participants

@Neproify @Necktrox @dretax @qaisjp @sbx320 @botder

Uh oh!

Added dynamic ped ID allocating(based on lopezloo pull request) #151

Added dynamic ped ID allocating(based on lopezloo pull request) #151

Uh oh!

Conversation

@Neproify Neproify commented Jun 28, 2017 • edited by qaisjp Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

@sbx320 sbx320 left a comment

Choose a reason for hiding this comment

Uh oh!

@sbx320 sbx320 Jun 28, 2017

Choose a reason for hiding this comment

Uh oh!

@sbx320 sbx320 Jun 28, 2017

Choose a reason for hiding this comment

Uh oh!

@sbx320 sbx320 Jun 28, 2017

Choose a reason for hiding this comment

Uh oh!

@Neproify Neproify Jun 28, 2017

Choose a reason for hiding this comment

Uh oh!

@Neproify Neproify Jun 28, 2017

Choose a reason for hiding this comment

Uh oh!

@Neproify Neproify Sep 26, 2017

Choose a reason for hiding this comment

Uh oh!

Necktrox commented Feb 18, 2018

Uh oh!

Neproify commented Jul 31, 2018

Uh oh!

@qaisjp qaisjp Jul 31, 2018

Choose a reason for hiding this comment

Uh oh!

@qaisjp qaisjp Jul 31, 2018

Choose a reason for hiding this comment

Uh oh!

Neproify commented Aug 3, 2018

Uh oh!

dretax commented Sep 1, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

@Neproify Neproify commented Jun 28, 2017 •

edited by qaisjp

Loading