Skipping the simulation, switching off collisions, and other approaches were not correct as they either broke some mods, or some core mechanics of the engine such as teleportation or waterwalking. As it turns out, the way to go is to simply do _nothing_ (modulo some gymnastics to account for the 1 frame difference in case of async).
Scripted movement and the unstucking logic tends to collide. Early out of unstuck in case the actor doesn't attempt to move. This means there is no AI package for NPC, which are the case for some boats and striders, or the player is content with their position.
Cylinder collision shape should give the best consistency between physics
simulation and pathfinding. Rotating box is already used by some actors, so
add it to have the same collision shape type for all actors.
Actors may have different collision shapes. Currently there are axis-aligned
bounding boxes and rotating bounding boxes. With AABB it's required to use
bounding cylinder for navmesh agent to avoid providing paths where actor can't
pass. But for rotating bounding boxes cylinder with diameter equal to the front
face width should be used to not reduce of available paths. For example rats
have rotating bounding box as collision shape because of the difference between
front and side faces width.
* Add agent bounds to navmesh tile db cache key. This is required to distinguish
tiles for agents with different bounds.
* Increase navmesh version because navmesh tile db cache key and data has changed.
* Move navmesh version to the code to avoid misconfiguration by users.
* Fix all places where wrong half extents were used for pathfinding.
Actors with disabled collisions still have physics simulations. Assuming they
should not be processed at all instead of disabling collision add a new flag to
make them inactive.
Actor::getOnGround and Actor::getOnSlope is used to initialize ActorFrameData.
After a physics simulation the result is copied back. But when actor is outside
processing range, Actor::mInternalCollisionMode is false and physics simulation
does not recalculate OnGround and OnSlope flags. So the flags are always set to
false that makes actor play landing animation when they exit and then enter
actors processing range.
Previous version skipped collision the frame immediately after a call to SetPos. It worked for one-off calls (teleports for instance) and continuous call along a pre-defined path (scenic travel). However, in the case of mod which uses SetPos to simulate a player-controlled movement, it is equivalent to using tcl.
Solution:
1/ skip update of mPosition and mPreviousPosition to avoid janky interpolation
2/ use back plain moveObject() instead of moveObjectBy() since we don't want physics simulation
3/ rework a little bit waterwalking influence on coordinate because of 1/
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)
mCanWaterWalk was set to false and updated during next frame's simulation
mOnGround is set to true but then was updated as part of the scene
loading logic.
Since actors can be active in 3x3 grid around the player, we need to
first load all objects in a 5x5 grid around the player.
Split load and unloading in 2 phases. Add an mInactiveCells set into the
scene, which contains all cells inside the aforementioned 5x5 grid.
These cells contains only heightfields and non-animated physics objects.
Animated objects are tied to the scene graph, which doesn't exists yet
in these cells, so we skip them.
Actor's position can be determined in 3 ways:
1/ as a result of physics simulation
2/ after a script require a relative position change (SetPos, Move)
3/ absolutely set from games mechanics event (teleport) or script
(PositionCell)
In case 1/, RefData::mPosition is updated with the physics simulation result
In case 2/, when RefData::mPosition is updated, physics simulation is informed of the change and update accordingly
In case 3/, when RefData::mPosition is updated, the physics simulation state is reset
In all 3 cases, we don't need to check the RefData::mPosition to get a
correct behaviour.
TSAN reported the following data race:
Read of size 4 at 0x7b50005b75b0 by thread T12 (mutexes: write M656173, write M84859534346343880):
#0 ESM::Position::asVec3() const /build/openmw/openmw/master2/.build/freebsd/TSAN/../../.././components/esm/defs.hpp:55:27 (openmw+0xb809d5)
#1 MWPhysics::Actor::updateWorldPosition() /build/openmw/openmw/master2/.build/freebsd/TSAN/../../../apps/openmw/mwphysics/actor.cpp:131:59 (openmw+0xb809d5)
#2 MWPhysics::Actor::setPosition(osg::Vec3f const&) /build/openmw/openmw/master2/.build/freebsd/TSAN/../../../apps/openmw/mwphysics/actor.cpp:177:5 (openmw+0xb809d5)
#3 MWPhysics::PhysicsTaskScheduler::updateActorsPositions() /build/openmw/openmw/master2/.build/freebsd/TSAN/../../../apps/openmw/mwphysics/mtphysics.cpp:524:28 (openmw+0xb91ac0)
#4 MWPhysics::PhysicsTaskScheduler::afterPostStep() /build/openmw/openmw/master2/.build/freebsd/TSAN/../../../apps/openmw/mwphysics/mtphysics.cpp:614:13 (openmw+0xb915e7)
#5 MWPhysics::PhysicsTaskScheduler::worker()::$_5::operator()() const /build/openmw/openmw/master2/.build/freebsd/TSAN/../../../apps/openmw/mwphysics/mtphysics.cpp:498:45 (openmw+0xb915e7)
#6 void Misc::Barrier::wait<MWPhysics::PhysicsTaskScheduler::worker()::$_5>(MWPhysics::PhysicsTaskScheduler::worker()::$_5&&) /build/openmw/openmw/master2/.build/freebsd/TSAN/../../.././components/misc/barrier.hpp:30:21 (openmw+0xb915e7)
#7 MWPhysics::PhysicsTaskScheduler::worker() /build/openmw/openmw/master2/.build/freebsd/TSAN/../../../apps/openmw/mwphysics/mtphysics.cpp:498:31 (openmw+0xb915e7)
#8 MWPhysics::PhysicsTaskScheduler::PhysicsTaskScheduler(float, btCollisionWorld*, MWRender::DebugDrawer*)::$_0::operator()() const /build/openmw/openmw/master2/.build/freebsd/TSAN/../../../apps/openmw/mwphysics/mtphysics.cpp:162:45 (openmw+0xb92630)
#9 decltype(std::__1::forward<MWPhysics::PhysicsTaskScheduler::PhysicsTaskScheduler(float, btCollisionWorld*, MWRender::DebugDrawer*)::$_0>(fp)()) std::__1::__invoke<MWPhysics::PhysicsTaskScheduler::PhysicsTaskScheduler(float, btCollisionWorld*, MWRender::DebugDrawer*)::$_0>(MWPhysics::PhysicsTaskScheduler::PhysicsTaskScheduler(float, btCollisionWorld*, MWRender::DebugDrawer*)::$_0&&) /usr/include/c++/v1/type_traits:3899:1 (openmw+0xb92630)
#10 void std::__1::__thread_execute<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, MWPhysics::PhysicsTaskScheduler::PhysicsTaskScheduler(float, btCollisionWorld*, MWRender::DebugDrawer*)::$_0>(std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, MWPhysics::PhysicsTaskScheduler::PhysicsTaskScheduler(float, btCollisionWorld*, MWRender::DebugDrawer*)::$_0>&, std::__1::__tuple_indices<>) /usr/include/c++/v1/thread:280:5 (openmw+0xb92630)
#11 void* std::__1::__thread_proxy<std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, MWPhysics::PhysicsTaskScheduler::PhysicsTaskScheduler(float, btCollisionWorld*, MWRender::DebugDrawer*)::$_0> >(void*) /usr/include/c++/v1/thread:291:5 (openmw+0xb92630)
Previous write of size 8 at 0x7b50005b75b0 by main thread:
#0 memcpy /wrkdirs/usr/ports/devel/llvm-devel/work-default/llvm-project-3f6753efe1990a928ed120bd907940a9fb3e2fc3/compiler-rt/lib/tsan/../sanitizer_common/sanitizer_common_interceptors.inc:827:5 (openmw+0x55a057)
#1 MWWorld::RefData::setPosition(ESM::Position const&) /build/openmw/openmw/master2/.build/freebsd/TSAN/../../../apps/openmw/mwworld/refdata.cpp:216:19 (openmw+0xa3de1c)
#2 MWWorld::World::moveObject(MWWorld::Ptr const&, MWWorld::CellStore*, float, float, float, bool) /build/openmw/openmw/master2/.build/freebsd/TSAN/../../../apps/openmw/mwworld/worldimp.cpp:1130:26 (openmw+0xa57300)
#3 MWWorld::World::moveObject(MWWorld::Ptr const&, float, float, float, bool, bool) /build/openmw/openmw/master2/.build/freebsd/TSAN/../../../apps/openmw/mwworld/worldimp.cpp:1253:16 (openmw+0xa580c8)
#4 MWWorld::World::doPhysics(float, unsigned long long, unsigned int, osg::Stats&) /build/openmw/openmw/master2/.build/freebsd/TSAN/../../../apps/openmw/mwworld/worldimp.cpp:1530:17 (openmw+0xa5af8f)
#5 MWWorld::World::updatePhysics(float, bool, unsigned long long, unsigned int, osg::Stats&) /build/openmw/openmw/master2/.build/freebsd/TSAN/../../../apps/openmw/mwworld/worldimp.cpp:1862:13 (openmw+0xa61a7c)
#6 OMW::Engine::frame(float) /build/openmw/openmw/master2/.build/freebsd/TSAN/../../../apps/openmw/engine.cpp:333:42 (openmw+0xcce9e7)
#7 OMW::Engine::go() /build/openmw/openmw/master2/.build/freebsd/TSAN/../../../apps/openmw/engine.cpp:935:14 (openmw+0xcd86ed)
#8 runApplication(int, char**) /build/openmw/openmw/master2/.build/freebsd/TSAN/../../../apps/openmw/main.cpp:296:17 (openmw+0xcbffac)
#9 wrapApplication(int (*)(int, char**), int, char**, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) /build/openmw/openmw/master2/.build/freebsd/TSAN/../../../components/debug/debugging.cpp:205:15 (openmw+0x1335442)
#10 main /build/openmw/openmw/master2/.build/freebsd/TSAN/../../../apps/openmw/main.cpp:308:12 (openmw+0xcc008a)
:wqa
One of the issue since the introduction of async physics is the quirky
handling of scripted moves. Previous attempt to account for them was
based on detecting changes in actor position while the physics thread is
running. To this end, semantics of Actor::updatePosition() (which is
responsible for set the absolute position of an actor in the world) was
toned down to merely store the desired position, with the physics system
actually responsible for moving the actor. For the cases were complete
override of the physics simulation was needed, I introduced
Actor::resetPosition(), which actually have same semantics as
original updatePosition(). This in turn introduced a loads of new bugs
when the weakened semantics broke key assumptions inside the engine
(spawning, summoning, teleport, etc).
Instead of tracking them down, count on the newly introduced support for
object relative movements in the engine (World::moveObjectBy) to
register relative movements and restore original handling of absolute positionning.
Changes are relatively small:
- move resetPosition() content into updatePosition()
- call updatePosition() everywhere it was called before
- remove all added calls to the now non-existing resetPosition()
tldr; ditch last month worth of bug introduction and eradication and redo
it properly
This gives finer control over reseting positions (switch off tcl is no
longer glitchy) and solve most of the erroneous usage of stale World::Ptr
indicated by:
"Error in frame: moveTo: object is not in this cell"
its current position.
Use it in relevant MWScripts opcode (move and moveworld).
Remove the fragile detection of scripted translation from PhysicsTaskScheduler.
No user visible change, just a more robust mechanism.
be sure the simulation is over. Otherwise, if the simulation is too slow
the position is wrong, and the actors would jump back and forth between
old and new position instead of actually moving.
Before this change, if an actor position was changed while the physics
simulation was running, the simulation result would be discarded. It is
fine in case of one off event such as teleport, but in the case of
scripts making use of this functionality to make lifts or conveyor (such
as Sotha Sil Expanded mod) it broke actor movement.
To alleviate this issue, at the end of the simulation, the position of the Actor
in the world is compared to the position it had at the beginning of the
simulation. A difference indicate a force move occured. In this case,
the Actor mPosition and mPreviousPosition are translated by the difference of position.
Since the Actor position will be really set while the next simulation runs, we
save it in the mNextPosition field.
Before movement calculation, the main thread prepare a
vector of ActorFrameData, which contains all data necessary to perform
the simulation, and feed it to the solver. At the same time it fetches
the result from the previous background simulation, which in turn is
used by the game mechanics.
Other functions of the physics system (weapon hit for instance)
interrupt the background simulation, with some exceptions described
below.
The number of threads is controlled by the numeric setting
[Physics]
async num threads
In case 'async num threads' > 1 and Bullet doesn't support multiple threads,
1 async thread will be used. 0 means synchronous solver.
Additional settings (will be silently switched off if async num threads = 0)
[Physics]
defer aabb update
Update AABBs of actors and objects in the background thread(s). It is not an especially
costly operation, but it needs exclusive access to the collision world, which blocks
other operations. Since AABB needs to be updated for collision detection, one can queue
them to defer update before start of the movement solver. Extensive tests on as much
as one installation (mine) show no drawback having that switched on.
[Physics]
lineofsight keep inactive cache
Control for how long (how many frames) the line of sight (LOS) request will be kept updated.
When a request for LOS is made for the first time, the background threads are stopped to
service it. From now on, the LOS will be refreshed preemptively as part of the background
routine until it is not required for lineofsight keep inactive cache frames. This mean
that subsequent request will not interrupt the background computation.
