BulletSim: clean up actor code so routines use the same coding pattern.

Fix a few enabling problems.

OpenSim/Region/Physics/BulletSPlugin/BSActorAvatarMove.cs

@@ -67,14 +67,6 @@ public class BSActorAvatarMove : BSActor
     {
         m_physicsScene.DetailLog("{0},BSActorAvatarMove,refresh", m_controllingPrim.LocalID);
 
-        // If not active any more, get rid of me (shouldn't ever happen, but just to be safe)
-        if (m_controllingPrim.RawForce == OMV.Vector3.Zero)
-        {
-            m_physicsScene.DetailLog("{0},BSActorAvatarMove,refresh,notAvatarMove,removing={1}", m_controllingPrim.LocalID, ActorName);
-            m_controllingPrim.PhysicalActors.RemoveAndRelease(ActorName);
-            return;
-        }
-
         // If the object is physically active, add the hoverer prestep action
         if (isActive)
         {
@@ -95,14 +87,19 @@ public class BSActorAvatarMove : BSActor
         // Nothing to do for the hoverer since it is all software at pre-step action time.
     }
 
+    // Usually called when target velocity changes to set the current velocity and the target
+    //     into the movement motor.
     public void SetVelocityAndTarget(OMV.Vector3 vel, OMV.Vector3 targ, bool inTaintTime)
     {
         m_physicsScene.TaintedObject(inTaintTime, "BSActorAvatarMove.setVelocityAndTarget", delegate()
         {
-            m_velocityMotor.Reset();
+            if (m_velocityMotor != null)
-            m_velocityMotor.SetTarget(targ);
+            {
-            m_velocityMotor.SetCurrent(vel);
+                m_velocityMotor.Reset();
-            m_velocityMotor.Enabled = true;
+                m_velocityMotor.SetTarget(targ);
+                m_velocityMotor.SetCurrent(vel);
+                m_velocityMotor.Enabled = true;
+            }
         });
     }
 
@@ -119,6 +116,7 @@ public class BSActorAvatarMove : BSActor
                                                 1f                          // efficiency
             );
             // _velocityMotor.PhysicsScene = PhysicsScene; // DEBUG DEBUG so motor will output detail log messages.
+            SetVelocityAndTarget(m_controllingPrim.RawVelocity, m_controllingPrim.TargetVelocity, true /* inTaintTime */);
 
             m_physicsScene.BeforeStep += Mover;
         }

OpenSim/Region/Physics/BulletSPlugin/BSActorHover.cs

@@ -67,12 +67,10 @@ public class BSActorHover : BSActor
     {
         m_physicsScene.DetailLog("{0},BSActorHover,refresh", m_controllingPrim.LocalID);
 
-        // If not active any more, get rid of me (shouldn't ever happen, but just to be safe)
+        // If not active any more, turn me off
         if (!m_controllingPrim.HoverActive)
         {
-            m_physicsScene.DetailLog("{0},BSActorHover,refresh,notHovering,removing={1}", m_controllingPrim.LocalID, ActorName);
+            SetEnabled(false);
-            m_controllingPrim.PhysicalActors.RemoveAndRelease(ActorName);
-            return;
         }
 
         // If the object is physically active, add the hoverer prestep action

OpenSim/Region/Physics/BulletSPlugin/BSActorLockAxis.cs

@@ -36,7 +36,6 @@ namespace OpenSim.Region.Physics.BulletSPlugin
 {
 public class BSActorLockAxis : BSActor
 {
-    bool TryExperimentalLockAxisCode = true;
     BSConstraint LockAxisConstraint = null;
 
     public BSActorLockAxis(BSScene physicsScene, BSPhysObject pObj, string actorName)
@@ -69,18 +68,13 @@ public class BSActorLockAxis : BSActor
         // If all the axis are free, we don't need to exist
         if (m_controllingPrim.LockedAxis == m_controllingPrim.LockedAxisFree)
         {
-            m_physicsScene.DetailLog("{0},BSActorLockAxis,refresh,allAxisFree,removing={1}", m_controllingPrim.LocalID, ActorName);
+            Enabled = false;
-            m_controllingPrim.PhysicalActors.RemoveAndRelease(ActorName);
-            return;
         }
+
         // If the object is physically active, add the axis locking constraint
-        if (Enabled
+        if (isActive)
-                && m_controllingPrim.IsPhysicallyActive
-                && TryExperimentalLockAxisCode
-                && m_controllingPrim.LockedAxis != m_controllingPrim.LockedAxisFree)
         {
-            if (LockAxisConstraint == null)
+            AddAxisLockConstraint();
-                AddAxisLockConstraint();
         }
         else
         {
@@ -108,58 +102,61 @@ public class BSActorLockAxis : BSActor
 
     private void AddAxisLockConstraint()
     {
-        // Lock that axis by creating a 6DOF constraint that has one end in the world and
+        if (LockAxisConstraint == null)
-        //    the other in the object.
-        // http://www.bulletphysics.org/Bullet/phpBB3/viewtopic.php?p=20817
-        // http://www.bulletphysics.org/Bullet/phpBB3/viewtopic.php?p=26380
-
-        // Remove any existing axis constraint (just to be sure)
-        RemoveAxisLockConstraint();
-
-        BSConstraint6Dof axisConstrainer = new BSConstraint6Dof(m_physicsScene.World, m_controllingPrim.PhysBody, 
-                            OMV.Vector3.Zero, OMV.Quaternion.Identity,
-                            false /* useLinearReferenceFrameB */, true /* disableCollisionsBetweenLinkedBodies */);
-        LockAxisConstraint = axisConstrainer;
-        m_physicsScene.Constraints.AddConstraint(LockAxisConstraint);
-
-        // The constraint is tied to the world and oriented to the prim.
-
-        // Free to move linearly in the region
-        OMV.Vector3 linearLow = OMV.Vector3.Zero;
-        OMV.Vector3 linearHigh = m_physicsScene.TerrainManager.DefaultRegionSize;
-        axisConstrainer.SetLinearLimits(linearLow, linearHigh);
-
-        // Angular with some axis locked
-        float fPI = (float)Math.PI;
-        OMV.Vector3 angularLow = new OMV.Vector3(-fPI, -fPI, -fPI);
-        OMV.Vector3 angularHigh = new OMV.Vector3(fPI, fPI, fPI);
-        if (m_controllingPrim.LockedAxis.X != 1f)
         {
-            angularLow.X = 0f;
+            // Lock that axis by creating a 6DOF constraint that has one end in the world and
-            angularHigh.X = 0f;
+            //    the other in the object.
-        }
+            // http://www.bulletphysics.org/Bullet/phpBB3/viewtopic.php?p=20817
-        if (m_controllingPrim.LockedAxis.Y != 1f)
+            // http://www.bulletphysics.org/Bullet/phpBB3/viewtopic.php?p=26380
-        {
-            angularLow.Y = 0f;
-            angularHigh.Y = 0f;
-        }
-        if (m_controllingPrim.LockedAxis.Z != 1f)
-        {
-            angularLow.Z = 0f;
-            angularHigh.Z = 0f;
-        }
-        if (!axisConstrainer.SetAngularLimits(angularLow, angularHigh))
-        {
-            m_physicsScene.DetailLog("{0},BSActorLockAxis.AddAxisLockConstraint,failedSetAngularLimits", m_controllingPrim.LocalID);
-        }
 
-        m_physicsScene.DetailLog("{0},BSActorLockAxis.AddAxisLockConstraint,create,linLow={1},linHi={2},angLow={3},angHi={4}",
+            // Remove any existing axis constraint (just to be sure)
-                                    m_controllingPrim.LocalID, linearLow, linearHigh, angularLow, angularHigh);
+            RemoveAxisLockConstraint();
 
-        // Constants from one of the posts mentioned above and used in Bullet's ConstraintDemo.
+            BSConstraint6Dof axisConstrainer = new BSConstraint6Dof(m_physicsScene.World, m_controllingPrim.PhysBody,
-        axisConstrainer.TranslationalLimitMotor(true /* enable */, 5.0f, 0.1f);
+                                OMV.Vector3.Zero, OMV.Quaternion.Identity,
+                                false /* useLinearReferenceFrameB */, true /* disableCollisionsBetweenLinkedBodies */);
+            LockAxisConstraint = axisConstrainer;
+            m_physicsScene.Constraints.AddConstraint(LockAxisConstraint);
 
-        axisConstrainer.RecomputeConstraintVariables(m_controllingPrim.RawMass);
+            // The constraint is tied to the world and oriented to the prim.
+
+            // Free to move linearly in the region
+            OMV.Vector3 linearLow = OMV.Vector3.Zero;
+            OMV.Vector3 linearHigh = m_physicsScene.TerrainManager.DefaultRegionSize;
+            axisConstrainer.SetLinearLimits(linearLow, linearHigh);
+
+            // Angular with some axis locked
+            float fPI = (float)Math.PI;
+            OMV.Vector3 angularLow = new OMV.Vector3(-fPI, -fPI, -fPI);
+            OMV.Vector3 angularHigh = new OMV.Vector3(fPI, fPI, fPI);
+            if (m_controllingPrim.LockedAxis.X != 1f)
+            {
+                angularLow.X = 0f;
+                angularHigh.X = 0f;
+            }
+            if (m_controllingPrim.LockedAxis.Y != 1f)
+            {
+                angularLow.Y = 0f;
+                angularHigh.Y = 0f;
+            }
+            if (m_controllingPrim.LockedAxis.Z != 1f)
+            {
+                angularLow.Z = 0f;
+                angularHigh.Z = 0f;
+            }
+            if (!axisConstrainer.SetAngularLimits(angularLow, angularHigh))
+            {
+                m_physicsScene.DetailLog("{0},BSActorLockAxis.AddAxisLockConstraint,failedSetAngularLimits", m_controllingPrim.LocalID);
+            }
+
+            m_physicsScene.DetailLog("{0},BSActorLockAxis.AddAxisLockConstraint,create,linLow={1},linHi={2},angLow={3},angHi={4}",
+                                        m_controllingPrim.LocalID, linearLow, linearHigh, angularLow, angularHigh);
+
+            // Constants from one of the posts mentioned above and used in Bullet's ConstraintDemo.
+            axisConstrainer.TranslationalLimitMotor(true /* enable */, 5.0f, 0.1f);
+
+            axisConstrainer.RecomputeConstraintVariables(m_controllingPrim.RawMass);
+        }
     }
 
     private void RemoveAxisLockConstraint()

OpenSim/Region/Physics/BulletSPlugin/BSActorMoveToTarget.cs

@@ -67,15 +67,12 @@ public class BSActorMoveToTarget : BSActor
     {
         m_physicsScene.DetailLog("{0},BSActorMoveToTarget,refresh", m_controllingPrim.LocalID);
 
-        // If not active any more, get rid of me (shouldn't ever happen, but just to be safe)
+        // If not active any more...
-        if (!m_controllingPrim.HoverActive)
+        if (!m_controllingPrim.MoveToTargetActive)
         {
-            m_physicsScene.DetailLog("{0},BSActorMoveToTarget,refresh,notMoveToTarget,removing={1}", m_controllingPrim.LocalID, ActorName);
+            Enabled = false;
-            m_controllingPrim.PhysicalActors.RemoveAndRelease(ActorName);
-            return;
         }
 
-        // If the object is physically active, add the hoverer prestep action
         if (isActive)
         {
             ActivateMoveToTarget();
@@ -92,7 +89,7 @@ public class BSActorMoveToTarget : BSActor
     // BSActor.RemoveBodyDependencies()
     public override void RemoveBodyDependencies()
     {
-        // Nothing to do for the hoverer since it is all software at pre-step action time.
+        // Nothing to do for the moveToTarget since it is all software at pre-step action time.
     }
 
     // If a hover motor has not been created, create one and start the hovering.
@@ -144,7 +141,6 @@ public class BSActorMoveToTarget : BSActor
             m_physicsScene.DetailLog("{0},BSPrim.PIDTarget,zeroMovement,movePos={1},pos={2},mass={3}",
                                     m_controllingPrim.LocalID, movePosition, m_controllingPrim.RawPosition, m_controllingPrim.Mass);
             m_controllingPrim.ForcePosition = m_targetMotor.TargetValue;
-            m_targetMotor.Enabled = false;
         }
         else
         {

OpenSim/Region/Physics/BulletSPlugin/BSActorSetForce.cs

@@ -71,7 +71,7 @@ public class BSActorSetForce : BSActor
         if (m_controllingPrim.RawForce == OMV.Vector3.Zero)
         {
             m_physicsScene.DetailLog("{0},BSActorSetForce,refresh,notSetForce,removing={1}", m_controllingPrim.LocalID, ActorName);
-            m_controllingPrim.PhysicalActors.RemoveAndRelease(ActorName);
+            Enabled = false;
             return;
         }
 

OpenSim/Region/Physics/BulletSPlugin/BSActorSetTorque.cs

@@ -70,8 +70,8 @@ public class BSActorSetTorque : BSActor
         // If not active any more, get rid of me (shouldn't ever happen, but just to be safe)
         if (m_controllingPrim.RawTorque == OMV.Vector3.Zero)
         {
-            m_physicsScene.DetailLog("{0},BSActorSetTorque,refresh,notSetTorque,removing={1}", m_controllingPrim.LocalID, ActorName);
+            m_physicsScene.DetailLog("{0},BSActorSetTorque,refresh,notSetTorque,disabling={1}", m_controllingPrim.LocalID, ActorName);
-            m_controllingPrim.PhysicalActors.RemoveAndRelease(ActorName);
+            Enabled = false;
             return;
         }
 

OpenSim/Region/Physics/BulletSPlugin/BSActors.cs

@@ -42,24 +42,36 @@ public class BSActorCollection
     }
     public void Add(string name, BSActor actor)
     {
-        m_actors[name] = actor;
+        lock (m_actors)
+        {
+            if (!m_actors.ContainsKey(name))
+            {
+                m_actors[name] = actor;
+            }
+        }
     }
     public bool RemoveAndRelease(string name)
     {
         bool ret = false;
-        if (m_actors.ContainsKey(name))
+        lock (m_actors)
         {
-            BSActor beingRemoved = m_actors[name];
+            if (m_actors.ContainsKey(name))
-            beingRemoved.Dispose();
+            {
-            m_actors.Remove(name);
+                BSActor beingRemoved = m_actors[name];
-            ret = true;
+                m_actors.Remove(name);
+                beingRemoved.Dispose();
+                ret = true;
+            }
         }
         return ret;
     }
     public void Clear()
     {
-        Release();
+        lock (m_actors)
-        m_actors.Clear();
+        {
+            Release();
+            m_actors.Clear();
+        }
     }
     public void Dispose()
     {
@@ -69,15 +81,22 @@ public class BSActorCollection
     {
         return m_actors.ContainsKey(name);
     }
+    public bool TryGetActor(string actorName, out BSActor theActor)
+    {
+        return m_actors.TryGetValue(actorName, out theActor);
+    }
     public void ForEachActor(Action<BSActor> act)
     {
-        foreach (KeyValuePair<string, BSActor> kvp in m_actors)
+        lock (m_actors)
-            act(kvp.Value);
+        {
+            foreach (KeyValuePair<string, BSActor> kvp in m_actors)
+                act(kvp.Value);
+        }
     }
 
     public void Enable(bool enabl)
     {
-        ForEachActor(a => a.Enable(enabl));
+        ForEachActor(a => a.SetEnabled(enabl));
     }
     public void Release()
     {
@@ -106,7 +125,7 @@ public abstract class BSActor
 {
     protected BSScene m_physicsScene { get; private set; }
     protected BSPhysObject m_controllingPrim { get; private set; }
-    protected bool Enabled { get; set; }
+    public virtual bool Enabled { get; set; }
     public string ActorName { get; private set; }
 
     public BSActor(BSScene physicsScene, BSPhysObject pObj, string actorName)
@@ -122,8 +141,10 @@ public abstract class BSActor
     {
         get { return Enabled; }
     }
-    // Turn the actor on an off.
+
-    public virtual void Enable(bool setEnabled)
+    // Turn the actor on an off. Only used by ActorCollection to set all enabled/disabled.
+    // Anyone else should assign true/false to 'Enabled'.
+    public void SetEnabled(bool setEnabled)
     {
         Enabled = setEnabled;
     }

OpenSim/Region/Physics/BulletSPlugin/BSCharacter.cs

@@ -160,6 +160,9 @@ public sealed class BSCharacter : BSPhysObject
         // Make so capsule does not fall over
         PhysicsScene.PE.SetAngularFactorV(PhysBody, OMV.Vector3.Zero);
 
+        // The avatar mover sets some parameters.
+        PhysicalActors.Refresh();
+
         PhysicsScene.PE.AddToCollisionFlags(PhysBody, CollisionFlags.CF_CHARACTER_OBJECT);
 
         PhysicsScene.PE.AddObjectToWorld(PhysicsScene.World, PhysBody);

OpenSim/Region/Physics/BulletSPlugin/BSPhysObject.cs

@@ -282,30 +282,31 @@ public abstract class BSPhysObject : PhysicsActor
 
     // 'actors' act on the physical object to change or constrain its motion. These can range from
     //       hovering to complex vehicle motion.
+    // May be called at non-taint time as this just adds the actor to the action list and the real
+    //    work is done during the simulation step.
+    // Note that, if the actor is already in the list and we are disabling same, the actor is just left
+    //    in the list disabled.
     public delegate BSActor CreateActor();
-    public void CreateRemoveActor(bool createRemove, string actorName, bool inTaintTime, CreateActor creator)
+    public void EnableActor(bool enableActor, string actorName, CreateActor creator)
     {
-        if (createRemove)
+        lock (PhysicalActors)
         {
-            PhysicsScene.TaintedObject(inTaintTime, "BSPrim.CreateRemoveActor:" + actorName, delegate()
+            BSActor theActor;
+            if (PhysicalActors.TryGetActor(actorName, out theActor))
             {
-                if (!PhysicalActors.HasActor(actorName))
+                // The actor already exists so just turn it on or off
-                {
+                theActor.Enabled = enableActor;
-                    DetailLog("{0},BSPrim.CreateRemoveActor,taint,registerActor,a={1}", LocalID, actorName);
+            }
-                    PhysicalActors.Add(actorName, creator());
+            else
-                }
-            });
-        }
-        else
-        {
-            PhysicsScene.TaintedObject(inTaintTime, "BSPrim.CreateRemoveActor:" + actorName, delegate()
             {
-                if (PhysicalActors.HasActor(actorName))
+                // The actor does not exist. If it should, create it.
+                if (enableActor)
                 {
-                    DetailLog("{0},BSPrim.CreateRemoveActor,taint,unregisterActor,a={1}", LocalID, actorName);
+                    theActor = creator();
-                    PhysicalActors.RemoveAndRelease(actorName);
+                    PhysicalActors.Add(actorName, theActor);
+                    theActor.Enabled = true;
                 }
-            });
+            }
         }
     }
 

OpenSim/Region/Physics/BulletSPlugin/BSPrim.cs

@@ -95,6 +95,7 @@ public class BSPrim : BSPhysObject
         _isPhysical = pisPhysical;
         _isVolumeDetect = false;
 
+        // We keep a handle to the vehicle actor so we can set vehicle parameters later.
         VehicleActor = new BSDynamics(PhysicsScene, this, VehicleActorName);
         PhysicalActors.Add(VehicleActorName, VehicleActor);
 
@@ -264,7 +265,7 @@ public class BSPrim : BSPhysObject
         if (axis.Z != 1) locking.Z = 0f;
         LockedAxis = locking;
 
-        CreateRemoveActor(LockedAxis != LockedAxisFree /* creatActor */, LockedAxisActorName, false /* inTaintTime */, delegate()
+        EnableActor(LockedAxis != LockedAxisFree, LockedAxisActorName, delegate()
         {
             return new BSActorLockAxis(PhysicsScene, this, LockedAxisActorName);
         });
@@ -501,7 +502,7 @@ public class BSPrim : BSPhysObject
         get { return RawForce; }
         set {
             RawForce = value;
-            CreateRemoveActor(RawForce == OMV.Vector3.Zero, SetForceActorName, false /* inTaintTime */, delegate()
+            EnableActor(RawForce != OMV.Vector3.Zero, SetForceActorName, delegate()
             {
                 return new BSActorSetForce(PhysicsScene, this, SetForceActorName);
             });
@@ -510,14 +511,13 @@ public class BSPrim : BSPhysObject
 
     public override int VehicleType {
         get {
-            return (int)VehicleActor.Type;   // if we are a vehicle, return that type
+            return (int)VehicleActor.Type;
         }
         set {
             Vehicle type = (Vehicle)value;
 
             PhysicsScene.TaintedObject("setVehicleType", delegate()
             {
-                // Done at taint time so we're sure the physics engine is not using the variables
                 // Vehicle code changes the parameters for this vehicle type.
                 VehicleActor.ProcessTypeChange(type);
                 ActivateIfPhysical(false);
@@ -669,11 +669,11 @@ public class BSPrim : BSPhysObject
         get { return RawTorque; }
         set {
             RawTorque = value;
-            CreateRemoveActor(RawTorque == OMV.Vector3.Zero, SetTorqueActorName, false /* inTaintTime */, delegate()
+            EnableActor(RawTorque != OMV.Vector3.Zero, SetTorqueActorName, delegate()
             {
                 return new BSActorSetTorque(PhysicsScene, this, SetTorqueActorName);
             });
-            // DetailLog("{0},BSPrim.SetTorque,call,torque={1}", LocalID, _torque);
+            DetailLog("{0},BSPrim.SetTorque,call,torque={1}", LocalID, RawTorque);
         }
     }
     public override OMV.Vector3 Acceleration {
@@ -786,7 +786,6 @@ public class BSPrim : BSPhysObject
         MakeDynamic(IsStatic);
 
         // Update vehicle specific parameters (after MakeDynamic() so can change physical parameters)
-        VehicleActor.Refresh();
         PhysicalActors.Refresh();
 
         // Arrange for collision events if the simulator wants them
@@ -1037,7 +1036,7 @@ public class BSPrim : BSPhysObject
     public override bool PIDActive {
         set {
             base.MoveToTargetActive = value;
-            CreateRemoveActor(MoveToTargetActive, MoveToTargetActorName, false /* inTaintTime */, delegate()
+            EnableActor(MoveToTargetActive, MoveToTargetActorName, delegate()
             {
                 return new BSActorMoveToTarget(PhysicsScene, this, MoveToTargetActorName);
             });
@@ -1049,7 +1048,7 @@ public class BSPrim : BSPhysObject
     public override bool PIDHoverActive {
         set {
             base.HoverActive = value;
-            CreateRemoveActor(HoverActive /* creatActor */, HoverActorName, false /* inTaintTime */, delegate()
+            EnableActor(HoverActive, HoverActorName, delegate()
             {
                 return new BSActorHover(PhysicsScene, this, HoverActorName);
             });
@@ -1458,7 +1457,7 @@ public class BSPrim : BSPhysObject
     {
         // Create the correct physical representation for this type of object.
         // Updates base.PhysBody and base.PhysShape with the new information.
-        // Ignore 'forceRebuild'. This routine makes the right choices and changes of necessary.
+        // Ignore 'forceRebuild'. 'GetBodyAndShape' makes the right choices and changes of necessary.
         PhysicsScene.Shapes.GetBodyAndShape(false /*forceRebuild */, PhysicsScene.World, this, null, delegate(BulletBody dBody)
         {
             // Called if the current prim body is about to be destroyed.
@@ -1472,9 +1471,9 @@ public class BSPrim : BSPhysObject
         return;
     }
 
+    // Called at taint-time
     protected virtual void RemoveBodyDependencies()
     {
-        VehicleActor.RemoveBodyDependencies();
         PhysicalActors.RemoveBodyDependencies();
     }
 
@@ -1482,6 +1481,7 @@ public class BSPrim : BSPhysObject
     // the world that things have changed.
     public override void UpdateProperties(EntityProperties entprop)
     {
+        // Let anyone (like the actors) modify the updated properties before they are pushed into the object and the simulator.
         TriggerPreUpdatePropertyAction(ref entprop);
 
         // DetailLog("{0},BSPrim.UpdateProperties,entry,entprop={1}", LocalID, entprop);   // DEBUG DEBUG