-
-
Notifications
You must be signed in to change notification settings - Fork 464
Fix #472 setElementHealth in onClientPedDamage can cause crash #3914
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
Fix #472 setElementHealth in onClientPedDamage can cause crash #3914
Conversation
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 |
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? |
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 |
Did you test the code/crash? |
Yeah, multiple times even with minigun. |
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. |
Fix #472
This is my next attempt to fix this issue.
The hook in
CRenderer::RenderEverythingBarRoads
is necessary because when an entity is frequently re-created, a crash occurs due tom_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.I believe this is my last attempt to resolve this issue, as I have run out of ideas for fixing it.