Lock Simulation weak_ptr in all visitors to avoid use after free.
And swap order for weak_ptr locking with locking collision world mutex to avoid
deadlock when underlying object tries to lock the same mutex in the destructor.
Add SimulationImpl type to avoid use of FrameData without locking weak_ptr.
- properly initialize mSimulationPosition in the constructor. Unlucky thread scheduling can cause processHits to be called before the first simulation run, causing the projectile to vanish to whatever value the variable happens to contains.
- don't continue moving the projectile after a hit. The position would continue to be updated to some senseless value.
1. Reorder unlock and notify_all calls to avoid notifying when not all worker
threads are waiting.
2. Make sure main thread does not attempt to exclusively lock mSimulationMutex
while not all workers are done with previous frame.
3. Replace mNewFrame flag by counter to avoid modification from multiple
threads.
simulation
The purpose of weak_ptr is to avoid performing the simulation on deleted
Actor by promoting it to a shared_ptr via a lock() call. This clutter
the code with a lot of branches, whereas the overwhelmingly common case is for the call to succeed.
Since the simulation is decoupled from the engine state, we can use a shared_ptr instead of a weak_ptr.
This allow us to ignore (ie not handle) the rarer case where the actor is delete from the scene. This means that the simulation
will run for one frame more than before for each actor, whereas the rest of the engine
will be ignorant of this.
MWWorld::Ptr:
- they are equivalent
- btCollisionObject* is readily available from the simulation, it saves
a call to a mutex
- btCollisionObject* is smaller
- compute the swimming state instead of storing it, it changes as part of the simulation and was not updated, so it was wrong anyway.
- store the swim level in ActorFrameData, it is constant per Actor so no need to compute it inside the simulation
ActActorFrameData structure. It makes it easier to reason about the
simulation (and hopefully simplify it).
Remove atomics from Actor class as a side effect.
Rename mFloatToSurface to mInert to make is explicit what it represent, not what it is used for
Store the Actor rotation (1 Vec2) instead of the whole ESM::Position (2 Vec3)
- inline PhysicsSystem::applyQueuedMovements() into PhysicsSystem::stepSimulation()
- rename PhysicsTaskScheduler::moveActors() to PhysicsTaskScheduler::applyQueuedMovements()
- move the actor movement code from World::doPhysics() to
PhysicsSystem::moveActors() (analogically to the projectile manager)
with a TTL of 0 frame. It helps performance when several subsystems
request the same LOS in the same frame (combat, headtracking, etc).
Except it doesn't work if the cache is never trimmed.
- default to 1 thread
- default to always use defered aabb update, remove option
- always keep a cache of LOS request for at least the current frame.
This decreases number of raycast, especially when a lot of actors are
involved and "NPCs avoid collisions" is on
In this case, the actor mPreviousPosition is not updated, so the actor
position is interpolated between an old (stucked) position and the new
(unstucked) position. The new position is most likely "stucked", so the
unstuck code strikes again, making the actor "vibrates".
That's exactly what the sync code path does, and it doesn't exhibit this
behavior.
What happened is that the last handle to an Actor shared_ptr was a
promoted weak_ptr. When the shared_ptr goes out of scope, the Actor dtor
is invoked. That involves removing the Actor collision object after
exclusively locking mCollisionWorldMutex. In this case, the lock was
already held in the outter scope of the promoted weak_ptr.
Reduce the scope of the mCollisionWorldMutex to never encompass the
lifetime of a promoted weak_ptr.
