Skip to content

Conversation

@FileEX
Copy link
Member

@FileEX FileEX commented Dec 31, 2024

Fix #472

This is my next attempt to fix this issue.

  • Fixed the crash.
  • Fixed the bug where the ped would still die.

The hook in CRenderer::RenderEverythingBarRoads is necessary because when an entity is frequently re-created, a crash occurs due to m_pRwObject being nullptr. This happens, for example, when shooting a ped with a minigun. In my opinion, this should not cause any synchronization or desynchronization issues, but this will become clear during testing if this PR gets merged.

function handleDamage(attacker,weapon,bodypart,loss)
    setElementHealth(source,1)
end
addEventHandler("onClientPedDamage",root,handleDamage)

I believe this is my last attempt to resolve this issue, as I have run out of ideas for fixing it.

@botder
Copy link
Member

botder commented Dec 31, 2024

I did similiar crash fixes before, but if you're looking for a cleaner solution, using a queue for entity re-creation (caused for example by changing model or respawning) and processing it after the game update, should fix all of these crashes.

@FileEX
Copy link
Member Author

FileEX commented Dec 31, 2024

I did similiar crash fixes before, but if you're looking for a cleaner solution, using a queue for entity re-creation (caused for example by changing model or respawning) and processing it after the game update, should fix all of these crashes.

What do you mean by processing it after the game update? These crashes are usually caused by the entity pointer becoming invalid during code execution, so it still needs to be replaced with the correct one using hooks, right?

@botder
Copy link
Member

botder commented Dec 31, 2024

I did similiar crash fixes before, but if you're looking for a cleaner solution, using a queue for entity re-creation (caused for example by changing model or respawning) and processing it after the game update, should fix all of these crashes.

What do you mean by processing it after the game update? These crashes are usually caused by the entity pointer becoming invalid during code execution, so it still needs to be replaced with the correct one using hooks, right?

Usually these crashes are caused by a hook that triggers an event, which then calls to functions that cause the invalidation. My suggestion is to delay whatever actions these "invalidating" functions do after the game update, where no chunk of code holds a pointer to an invalidated entity.

@FileEX
Copy link
Member Author

FileEX commented Dec 31, 2024

I did similiar crash fixes before, but if you're looking for a cleaner solution, using a queue for entity re-creation (caused for example by changing model or respawning) and processing it after the game update, should fix all of these crashes.

What do you mean by processing it after the game update? These crashes are usually caused by the entity pointer becoming invalid during code execution, so it still needs to be replaced with the correct one using hooks, right?

Usually these crashes are caused by a hook that triggers an event, which then calls to functions that cause the invalidation. My suggestion is to delay whatever actions these "invalidating" functions do after the game update, where no chunk of code holds a pointer to an invalidated entity.

I understand what you mean now, but I’m not entirely sure how this could be achieved. The event needs to be triggered at the appropriate moment, as it is now, to allow it to be canceled correctly. So, how could this be done while still preserving the ability to cancel the event? Additionally, how would it even be possible to delay the code executed in a event function?

@botder
Copy link
Member

botder commented Dec 31, 2024

I understand what you mean now, but I’m not entirely sure how this could be achieved. The event needs to be triggered at the appropriate moment, as it is now, to allow it to be canceled correctly. So, how could this be done while still preserving the ability to cancel the event? Additionally, how would it even be possible to delay the code executed in a event function?

I didn't talk about the event callback function, but whatever function the user decides to call (setElementHealth that respawns dead peds), that cause this effect: entity invalidation. And I don't mean we should delay setting the health, but rather the ped recreation.

@FileEX
Copy link
Member Author

FileEX commented Jan 1, 2025

I understand what you mean now, but I’m not entirely sure how this could be achieved. The event needs to be triggered at the appropriate moment, as it is now, to allow it to be canceled correctly. So, how could this be done while still preserving the ability to cancel the event? Additionally, how would it even be possible to delay the code executed in a event function?

I didn't talk about the event callback function, but whatever function the user decides to call (setElementHealth that respawns dead peds), that cause this effect: entity invalidation. And I don't mean we should delay setting the health, but rather the ped recreation.

Done

@botder
Copy link
Member

botder commented Jan 1, 2025

Did you test the code/crash?

@FileEX
Copy link
Member Author

FileEX commented Jan 1, 2025

Did you test the code/crash?

Yeah, multiple times even with minigun.

@botder
Copy link
Member

botder commented Jan 1, 2025

If this works as intended, then we should use this approach for vehicles and other things too.

@FileEX
Copy link
Member Author

FileEX commented Jan 1, 2025

If this works as intended, then we should use this approach for vehicles and other things too.

You're right, but since this PR also touches the ped damage logic, it would be better to do it in a separate PR soon, so that if a revert is needed due to potential synchronization issues, it won't undo other changes.

@botder botder merged commit 2d3397d into multitheftauto:master Jan 1, 2025
6 checks passed
MTABot pushed a commit that referenced this pull request Jan 1, 2025
2d3397d Fix crash by delaying ped recreation (PR #3914, Fixes #472)
@botder botder added the bugfix Solution to a bug of any kind label Jan 1, 2025
@botder botder added this to the 1.6.1 milestone Jan 1, 2025
@FileEX FileEX deleted the bugfix/onClientPedDamage-crash2 branch January 1, 2025 22:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Solution to a bug of any kind

Projects

None yet

Development

Successfully merging this pull request may close these issues.

setElementHealth in onClientPedDamage can cause crash

2 participants