From 4e1ca890c2d41bf244ed7d4847c0d0a4fc7f6c51 Mon Sep 17 00:00:00 2001 From: Robert Adams Date: Mon, 14 Jan 2013 15:46:46 -0800 Subject: [PATCH] BulletSim: fix not moving physical objects below terrain to over terrain. Add locking on register prestep action list preventing potential race conditions. Little comment and formatting changes. --- .../Physics/BulletSPlugin/BSAPIUnman.cs | 2 +- .../Physics/BulletSPlugin/BSCharacter.cs | 4 +-- .../Physics/BulletSPlugin/BSPhysObject.cs | 29 ++++++++++++------- .../Region/Physics/BulletSPlugin/BSPrim.cs | 2 +- .../Region/Physics/BulletSPlugin/BSScene.cs | 17 ++++++++--- 5 files changed, 36 insertions(+), 18 deletions(-) diff --git a/OpenSim/Region/Physics/BulletSPlugin/BSAPIUnman.cs b/OpenSim/Region/Physics/BulletSPlugin/BSAPIUnman.cs index 0fef67c1f0..9ff7084aed 100755 --- a/OpenSim/Region/Physics/BulletSPlugin/BSAPIUnman.cs +++ b/OpenSim/Region/Physics/BulletSPlugin/BSAPIUnman.cs @@ -202,7 +202,7 @@ private void BulletLoggerPhysLog(string msg) } public override int PhysicsStep(BulletWorld world, float timeStep, int maxSubSteps, float fixedTimeStep, - out int updatedEntityCount, out int collidersCount) + out int updatedEntityCount, out int collidersCount) { BulletWorldUnman worldu = world as BulletWorldUnman; return BSAPICPP.PhysicsStep2(worldu.ptr, timeStep, maxSubSteps, fixedTimeStep, out updatedEntityCount, out collidersCount); diff --git a/OpenSim/Region/Physics/BulletSPlugin/BSCharacter.cs b/OpenSim/Region/Physics/BulletSPlugin/BSCharacter.cs index aadb583a5e..a5fec87eed 100644 --- a/OpenSim/Region/Physics/BulletSPlugin/BSCharacter.cs +++ b/OpenSim/Region/Physics/BulletSPlugin/BSCharacter.cs @@ -126,9 +126,9 @@ public sealed class BSCharacter : BSPhysObject DetailLog("{0},BSCharacter.Destroy", LocalID); PhysicsScene.TaintedObject("BSCharacter.destroy", delegate() { - PhysicsScene.Shapes.DereferenceBody(PhysBody, true, null); + PhysicsScene.Shapes.DereferenceBody(PhysBody, true /* inTaintTime */, null /* bodyCallback */); PhysBody.Clear(); - PhysicsScene.Shapes.DereferenceShape(PhysShape, true, null); + PhysicsScene.Shapes.DereferenceShape(PhysShape, true /* inTaintTime */, null /* bodyCallback */); PhysShape.Clear(); }); } diff --git a/OpenSim/Region/Physics/BulletSPlugin/BSPhysObject.cs b/OpenSim/Region/Physics/BulletSPlugin/BSPhysObject.cs index 185f1113ef..821f470bd2 100755 --- a/OpenSim/Region/Physics/BulletSPlugin/BSPhysObject.cs +++ b/OpenSim/Region/Physics/BulletSPlugin/BSPhysObject.cs @@ -382,10 +382,13 @@ public abstract class BSPhysObject : PhysicsActor { string identifier = op + "-" + id.ToString(); - // Clean out any existing action - UnRegisterPreStepAction(op, id); + lock (RegisteredActions) + { + // Clean out any existing action + UnRegisterPreStepAction(op, id); - RegisteredActions[identifier] = actn; + RegisteredActions[identifier] = actn; + } PhysicsScene.BeforeStep += actn; DetailLog("{0},BSPhysObject.RegisterPreStepAction,id={1}", LocalID, identifier); } @@ -395,22 +398,28 @@ public abstract class BSPhysObject : PhysicsActor { string identifier = op + "-" + id.ToString(); bool removed = false; - if (RegisteredActions.ContainsKey(identifier)) + lock (RegisteredActions) { - PhysicsScene.BeforeStep -= RegisteredActions[identifier]; - RegisteredActions.Remove(identifier); - removed = true; + if (RegisteredActions.ContainsKey(identifier)) + { + PhysicsScene.BeforeStep -= RegisteredActions[identifier]; + RegisteredActions.Remove(identifier); + removed = true; + } } DetailLog("{0},BSPhysObject.UnRegisterPreStepAction,id={1},removed={2}", LocalID, identifier, removed); } protected void UnRegisterAllPreStepActions() { - foreach (KeyValuePair kvp in RegisteredActions) + lock (RegisteredActions) { - PhysicsScene.BeforeStep -= kvp.Value; + foreach (KeyValuePair kvp in RegisteredActions) + { + PhysicsScene.BeforeStep -= kvp.Value; + } + RegisteredActions.Clear(); } - RegisteredActions.Clear(); DetailLog("{0},BSPhysObject.UnRegisterAllPreStepActions,", LocalID); } diff --git a/OpenSim/Region/Physics/BulletSPlugin/BSPrim.cs b/OpenSim/Region/Physics/BulletSPlugin/BSPrim.cs index e5f7ab71a6..79fe632a8e 100644 --- a/OpenSim/Region/Physics/BulletSPlugin/BSPrim.cs +++ b/OpenSim/Region/Physics/BulletSPlugin/BSPrim.cs @@ -379,7 +379,7 @@ public sealed class BSPrim : BSPhysObject // If the object is below ground it just has to be moved up because pushing will // not get it through the terrain _position.Z = targetHeight; - if (!inTaintTime) + if (inTaintTime) ForcePosition = _position; ret = true; } diff --git a/OpenSim/Region/Physics/BulletSPlugin/BSScene.cs b/OpenSim/Region/Physics/BulletSPlugin/BSScene.cs index a5bdc07b01..e0b4992e0d 100644 --- a/OpenSim/Region/Physics/BulletSPlugin/BSScene.cs +++ b/OpenSim/Region/Physics/BulletSPlugin/BSScene.cs @@ -387,12 +387,14 @@ public sealed class BSScene : PhysicsScene, IPhysicsParameters if (!m_initialized) return null; BSCharacter actor = new BSCharacter(localID, avName, this, position, size, isFlying); - lock (PhysObjects) PhysObjects.Add(localID, actor); + lock (PhysObjects) + PhysObjects.Add(localID, actor); // TODO: Remove kludge someday. // We must generate a collision for avatars whether they collide or not. // This is required by OpenSim to update avatar animations, etc. - lock (m_avatars) m_avatars.Add(actor); + lock (m_avatars) + m_avatars.Add(actor); return actor; } @@ -408,9 +410,11 @@ public sealed class BSScene : PhysicsScene, IPhysicsParameters { try { - lock (PhysObjects) PhysObjects.Remove(actor.LocalID); + lock (PhysObjects) + PhysObjects.Remove(bsactor.LocalID); // Remove kludge someday - lock (m_avatars) m_avatars.Remove(bsactor); + lock (m_avatars) + m_avatars.Remove(bsactor); } catch (Exception e) { @@ -419,6 +423,11 @@ public sealed class BSScene : PhysicsScene, IPhysicsParameters bsactor.Destroy(); // bsactor.dispose(); } + else + { + m_log.ErrorFormat("{0}: Requested to remove avatar that is not a BSCharacter. ID={1}, type={2}", + LogHeader, actor.LocalID, actor.GetType().Name); + } } public override void RemovePrim(PhysicsActor prim)