RigsOfRods / rigs-of-rods

Main development repository for Rigs of Rods soft-body physics simulator
https://www.rigsofrods.org
GNU General Public License v3.0
990 stars 175 forks source link

Fixed paused actor physics resuming after switching to another actor #3146

Closed ohlidalp closed 5 days ago

ohlidalp commented 3 months ago

Fixes https://github.com/RigsOfRods/rigs-of-rods/issues/3105.

This was quite an endeavor - it turned out there's no single spot to correctly reset the physics pausd state (or skeletonview state - related). While researching how to add it, I realized scripts may want to know when (un)linking of actors happens, so I added a script event SE_GENERIC_TRUCK_LINKING_CHANGED for it.

What changed: code changed signifficantly; all data remained the same - code handling hooks/ties/ropes/slidenodes got only cosmetic edits. There are many new sanity/integrity checks in the (un)linking logic which will trigger Asserts on Debug and just prevent damage on Release.

What to test: Operation of ties/hooks/ropes/slidenodes, including savegame and abrupt deleting of one of the actors.

CuriousMike56 commented 2 months ago

Almost perfect. Found one crash scenario:

Under Debug I get infinite asserts when trying to backspace with the crane:

https://github.com/RigsOfRods/rigs-of-rods/assets/46073351/73e0e22a-964d-4381-bb63-7e68dab1718a

And clicking retry gives: devenv_2024-04-22_19-22-55

ohlidalp commented 1 month ago

I fixed the asserts on backspacing the Liebherr crane, and added many more asserts in the process, because those that triggered were completely legit, SyncReset() was actually forcing hooks/ropes/ties to inconsistent state.

ohlidalp commented 1 month ago

obrazek I pressed L to attach the hook, then tried to delete via top menu. EDIT: turned out to be faulty assert. EDIT2: fixed the actual problem.

CuriousMike56 commented 1 month ago

Attached loads now move faster than the crane when using live repair mode (soft reset enabled):

https://github.com/RigsOfRods/rigs-of-rods/assets/46073351/327db4e2-d41e-4417-878e-b908cdcee454

vs master:

https://github.com/RigsOfRods/rigs-of-rods/assets/46073351/b1d91fb8-4a5f-4f88-b8a6-e1ded1b2b831

ohlidalp commented 2 weeks ago

@CuriousMike56 Ready for another test.

Turns out there was another bug in forced unlinking of inter-actor beams (both when deleting actor and hard-resetting) which left the actor data in inconsistent state. It's actually a very old bug in code written by Ulteq in ~2016/17 to fix multithreading problems related to inter-linking. It passed unnoticed until I introduced the RefCountingObjectPtr<> mechanic, which triggered the need for MSG_SIM_ACTOR_LINKING_REQUESTED and a bunch of asserts() to finaly kick the data into consistency.

CuriousMike56 commented 2 weeks ago

https://github.com/RigsOfRods/rigs-of-rods/pull/3146#issuecomment-2140796036 still happens

ohlidalp commented 2 weeks ago

@CuriousMike56 Mystery solved: why did this only manifest when using ties (your scenario) and not hook (my initial scenario)? The reason is - by accident, the movement sync between the crane and the load was done once for each interconnecting beam, instead of once per actor pair. And because there's always just one hook beam, it worked OK with a hooked load, but there may be multiple ties (in this case 4), the load moved faster (4x in this case).

Fixed by adding a cache of unique actor pairs, and updating it each time linking changes.

CuriousMike56 commented 2 weeks ago

Crane won't follow if you switch to the crate:

https://github.com/RigsOfRods/rigs-of-rods/assets/46073351/6aaebd10-d93b-475a-a983-a226552494b6

Also happens with a truck+trailer+load:

RoR_2024-06-18_02-33-34 RoR_2024-06-18_02-35-10

ohlidalp commented 2 weeks ago

@CuriousMike56 I apologize for all the trouble - it's all because I tried to automagically sync all state toggles in one place.

I'm rebooting this PR by reverting the offending commit. The paused physics problem is gone, as well as all the LiveRepair glitches, also the crash in https://github.com/RigsOfRods/rigs-of-rods/pull/3146#issuecomment-2071115449 is gone. However, now I'm observing these glitches:

ohlidalp commented 1 week ago

@CuriousMike56 I've refurbrished the prior commits without all the parts that touch LiveRepair (aka interactive reset). I tested all these cases:

CuriousMike56 commented 1 week ago

Seems the only remaining issue is the truck+trailer+load combo: RoR_2024-06-22_00-42-33 RoR_2024-06-22_00-42-48 Master (Viper N/B is visible, soft reset works as expected): RoR_2024-06-22_00-40-55

ohlidalp commented 1 week ago

@CuriousMike56 Ready for another test. I had to revert most of my new code, including the asserts. I re-tested all the above scenarios, including the crash scenario you found originally, including the truck/trailer/car combo.