search

misyltoad / VPhysics-Jolt

Volt (VPhysics Jolt) is a replacement physics module for the Source Engine.
MIT License
1.26k stars 72 forks source link

Fixed a crash #194

Closed RaphaelIT7 closed 3 months ago

RaphaelIT7 commented 1 year ago

[#] Fixed a crash

Related issues: fixes #137 fixes #85

misyltoad commented 3 months ago

Let's come back to this after 0.20, do you have a repro case?

RaphaelIT7 commented 3 months ago

I'll can get one in a few hours

YUCLing commented 3 months ago

Let's come back to this after 0.20, do you have a repro case?

Maybe check the related issues? It looks like it's simple to repro in GMod with an expanding hydraulic.

RaphaelIT7 commented 3 months ago

Latest Build:

https://github.com/user-attachments/assets/11c861d7-ffd2-4bc7-ad82-c437e9eb188c

RaphaelIT7 commented 3 months ago

I did a bit more testing, and it seems like JoltPhysicsSpring::OnJoltPhysicsObjectDestroyed will be called and then it breaks. If a prop is connected to the world, it will crash in JoltPhysicsSpring::SetSpringLength because the m_pObjectStart was deleted, but there is no null check so it'll try to use it.
I'll try to test it future again tomorrow, but it seems like the crash in the video is a different one? (I don't remember it ever crashing in one of the JoltPhysicsSpring functions when I made the pr)

misyltoad commented 3 months ago

Is it not set up to have an OnDestroyedListener?

RaphaelIT7 commented 3 months ago

It has and OnJoltPhysicsObjectDestroyed will be called to set m_pObjectStart to nullptr but it's still used / the engine still calls SetSpringLength and in that function you don't check if it's null

misyltoad commented 3 months ago

I guess MR that NULL check? :-)

RaphaelIT7 commented 3 months ago

It'll solve the world -> prop crash but not the prop -> prop crash sadly

I'll look into it tomorrow again to try and solve it

RaphaelIT7 commented 3 months ago

So It seems like I found the cause, and it should now be fixed for all cases :D

I also reverted the original two commits, since it seems like they didn't really do anything. I wonder why they fixed it on the dupe months ago.