Skip to content
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

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

ohlidalp
Copy link
Member

Fixes #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
Copy link
Collaborator

CuriousMike56 commented Apr 22, 2024

Almost perfect. Found one crash scenario:

  • Spawn Liebherr LTM 1050-3.1 and any load, such as the Rock pack
  • Press O to attach ties to the load
  • Delete the load, disconnecting ties first doesn't matter.
  • Spawn a copy of the load with CTRL+PERIOD, or spawn a different load
  • Try to attach ties to other load -> crash

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

RoR_2024-04-22_19-22-25.mp4

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

@ohlidalp
Copy link
Member Author

ohlidalp commented May 27, 2024

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.

Removed `isBeingReset()`, exposed variable `m_ongoing_reset` and renamed to `ar_`
The problem was introduced by myself in 9431e1b - I cleaned up duplicate syncing code but oversimplified the logic to reset unlinked actors. As result, the physics paused state and skeletonview state could only be active on player's actor and connected actor, not on any free standing one.

This is quite a significant remake of actor-linking code; a direct follow-up to d36c4ec which introduced ✉️ MSG_SIM_ACTOR_LINKING_REQUESTED. A remake was needed because there was no single spot to correctly reset the skeleton/physicspause on actor unlink - there was simply no concept of "actors were just linked/unlinked". While researching how to add it, I realized scripts may want to know when that happens, so I added a script event `SE_GENERIC_TRUCK_LINKING_CHANGED` for it. Keep in mind there can be multiple interlinking beams at the same time, so not every added/removed beam means linking changes. The new event also helps navigate the codebase and documents how stuff works.

Note that only code changed; all data remained the same - code handling hooks/ties/ropes/slidenodes got only cosmetic edits.

Code changes:
* ScriptEvents.h - added SE_GENERIC_TRUCK_LINKING_CHANGED ~ args: #1 action (1=linked, 0=unlinked), #2 link type `ActorLinkingRequestType` (will be INVALID on savegame load or forced unlink when deleting actor) #3 master ActorInstanceID_t, #4 slave ActorInstanceID_t
* Actor.h/cpp - Removed functions `Add/RemoveInterActorbeam()` - using new ones in ActorManager.
* ActorManager.h/cpp -  Added newly implemented functions `Add/RemoveInterActorBeam()`.
💡 This introduces the mechanism of grouping beams in `Actor::ar_beams` by their type and origin. See `struct BeamRangesByOrigin` in file 'SimData.h'.
@ohlidalp
Copy link
Member Author

ohlidalp commented May 28, 2024

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.

Problem `a` and `b` were just references-to-smartpointer, not full smartpointers, the actual data lived in `inter_actor_links` - we deleted that data and then strill tried using `a`.

Additional fix: faulty assert and debug check (operators were swapped).
@CuriousMike56
Copy link
Collaborator

CuriousMike56 commented May 30, 2024

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

8mb.video-lNy-61XoPei1.mp4

vs master:

RoR_2024-05-30_16-16-36.mp4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Paused actor physics resume when switching to another actor
2 participants