Clean up collision reporting code so they are properly passed to

the simulator in batches.
More comments.

OpenSim/Region/Physics/BulletSPlugin/BSCharacter.cs

@@ -74,7 +74,7 @@ public class BSCharacter : PhysicsActor
     private float _buoyancy;
 
     private int _subscribedEventsMs = 0;
-    private int _lastCollisionTime = 0;
+    private int _nextCollisionOkTime = 0;
 
     private Vector3 _PIDTarget;
     private bool _usePID;
@@ -360,17 +360,22 @@ public class BSCharacter : PhysicsActor
         }
         //m_lastUpdateSent = false;
     }
+
     public override void AddAngularForce(Vector3 force, bool pushforce) { 
     }
     public override void SetMomentum(Vector3 momentum) { 
     }
+
+    // Turn on collision events at a rate no faster than one every the given milliseconds
     public override void SubscribeEvents(int ms) {
         _subscribedEventsMs = ms;
-        _lastCollisionTime = Util.EnvironmentTickCount() - _subscribedEventsMs; // make first collision happen
+        _nextCollisionOkTime = Util.EnvironmentTickCount() - _subscribedEventsMs; // make first collision happen
     }
+    // Stop collision events
     public override void UnSubscribeEvents() { 
         _subscribedEventsMs = 0;
     }
+    // Return 'true' if someone has subscribed to events
     public override bool SubscribedEvents() {
         return (_subscribedEventsMs > 0);
     }
@@ -386,47 +391,57 @@ public class BSCharacter : PhysicsActor
         _mass = _density * _avatarVolume;
     }
 
+    // Set to 'true' if the individual changed items should be checked
+    //   (someday RequestPhysicsTerseUpdate() will take a bitmap of changed properties)
+    const bool SHOULD_CHECK_FOR_INDIVIDUAL_CHANGES = false;
+
     // The physics engine says that properties have updated. Update same and inform
     // the world that things have changed.
     public void UpdateProperties(EntityProperties entprop)
     {
         bool changed = false;
-        // we assign to the local variables so the normal set action does not happen
+        if (SHOULD_CHECK_FOR_INDIVIDUAL_CHANGES) {
-        if (_position != entprop.Position)
+            // we assign to the local variables so the normal set action does not happen
-        {
+            if (_position != entprop.Position) {
+                _position = entprop.Position;
+                changed = true;
+            }
+            if (_orientation != entprop.Rotation) {
+                _orientation = entprop.Rotation;
+                changed = true;
+            }
+            if (_velocity != entprop.Velocity) {
+                _velocity = entprop.Velocity;
+                changed = true;
+            }
+            if (_acceleration != entprop.Acceleration) {
+                _acceleration = entprop.Acceleration;
+                changed = true;
+            }
+            if (_rotationalVelocity != entprop.RotationalVelocity) {
+                _rotationalVelocity = entprop.RotationalVelocity;
+                changed = true;
+            }
+            if (changed) {
+                // m_log.DebugFormat("{0}: UpdateProperties: id={1}, c={2}, pos={3}, rot={4}", LogHeader, LocalID, changed, _position, _orientation);
+                // Avatar movement is not done by generating this event. There is code in the heartbeat
+                //   loop that updates avatars.
+                // base.RequestPhysicsterseUpdate();
+            }
+        }
+        else {
             _position = entprop.Position;
-            changed = true;
-        }
-        if (_orientation != entprop.Rotation)
-        {
             _orientation = entprop.Rotation;
-            changed = true;
-        }
-        if (_velocity != entprop.Velocity)
-        {
             _velocity = entprop.Velocity;
-            changed = true;
-        }
-        if (_acceleration != entprop.Acceleration)
-        {
             _acceleration = entprop.Acceleration;
-            changed = true;
-        }
-        if (_rotationalVelocity != entprop.RotationalVelocity)
-        {
             _rotationalVelocity = entprop.RotationalVelocity;
-            changed = true;
-        }
-        if (changed)
-        {
-            // m_log.DebugFormat("{0}: UpdateProperties: id={1}, c={2}, pos={3}, rot={4}", LogHeader, LocalID, changed, _position, _orientation);
-            // Avatar movement is not done by generating this event. There is a system that
-            //  checks for avatar updates each heartbeat loop.
             // base.RequestPhysicsterseUpdate();
         }
     }
 
     // Called by the scene when a collision with this object is reported
+    // The collision, if it should be reported to the character, is placed in a collection
+    //   that will later be sent to the simulator when SendCollisions() is called.
     CollisionEventUpdate collisionCollection = null;
     public void Collide(uint collidingWith, ActorTypes type, Vector3 contactPoint, Vector3 contactNormal, float pentrationDepth)
     {
@@ -440,29 +455,34 @@ public class BSCharacter : PhysicsActor
         }
 
         // throttle collisions to the rate specified in the subscription
-        if (_subscribedEventsMs == 0) return;   // don't want collisions
+        if (_subscribedEventsMs != 0) {
-        int nowTime = _scene.SimulationNowTime;
+            int nowTime = _scene.SimulationNowTime;
-        if (nowTime < (_lastCollisionTime + _subscribedEventsMs)) return;
+            if (nowTime >= _nextCollisionOkTime) {
-        _lastCollisionTime = nowTime;
+                _nextCollisionOkTime = nowTime + _subscribedEventsMs;
 
-        if (collisionCollection == null)
+                if (collisionCollection == null)
-            collisionCollection = new CollisionEventUpdate();
+                    collisionCollection = new CollisionEventUpdate();
-        collisionCollection.AddCollider(collidingWith, new ContactPoint(contactPoint, contactNormal, pentrationDepth));
+                collisionCollection.AddCollider(collidingWith, new ContactPoint(contactPoint, contactNormal, pentrationDepth));
+            }
+        }
     }
 
     public void SendCollisions()
     {
-        // if (collisionCollection != null)
+        /*
-        // {
+        if (collisionCollection != null && collisionCollection.Count > 0)
-        //     base.SendCollisionUpdate(collisionCollection);
+        {
-        //     collisionCollection = null;
+            base.SendCollisionUpdate(collisionCollection);
-        // }
+            collisionCollection = null;
+        }
+         */
         // Kludge to make a collision call even if there are no collisions.
         // This causes the avatar animation to get updated.
         if (collisionCollection == null)
             collisionCollection = new CollisionEventUpdate();
         base.SendCollisionUpdate(collisionCollection);
-        collisionCollection = null;
+        collisionCollection.Clear();
+        // End kludge
     }
 
 }

OpenSim/Region/Physics/BulletSPlugin/BSPlugin.cs

@@ -32,6 +32,14 @@ using OpenMetaverse;
 
 namespace OpenSim.Region.Physics.BulletSPlugin
 {
+    /// <summary>
+    /// Entry for a port of Bullet (http://bulletphysics.org/) to OpenSim. 
+    /// This module interfaces to an unmanaged C++ library which makes the
+    /// actual calls into the Bullet physics engine.
+    /// The unmanaged library is found in opensim-libs::trunk/unmanaged/BulletSim/.
+    /// The unmanaged library is compiled and linked statically with Bullet
+    /// to create BulletSim.dll and libBulletSim.so (for both 32 and 64 bit).
+    /// </summary>
 public class BSPlugin : IPhysicsPlugin
 {
     //private static readonly log4net.ILog m_log = log4net.LogManager.GetLogger(System.Reflection.MethodBase.GetCurrentMethod().DeclaringType);
@@ -53,6 +61,9 @@ public class BSPlugin : IPhysicsPlugin
         {
             if (Util.IsWindows())
                 Util.LoadArchSpecificWindowsDll("BulletSim.dll");
+            // If not Windows, loading is performed by the
+            // Mono loader as specified in 
+            // "bin/Physics/OpenSim.Region.Physics.BulletSPlugin.dll.config".
 
             _mScene = new BSScene(sceneIdentifier);
         }

OpenSim/Region/Physics/BulletSPlugin/BSPrim.cs

@@ -90,7 +90,7 @@ public sealed class BSPrim : PhysicsActor
     private BSPrim _parentPrim;
 
     private int _subscribedEventsMs = 0;
-    private int _lastCollisionTime = 0;
+    private int _nextCollisionOkTime = 0;
     long _collidingStep;
     long _collidingGroundStep;
 
@@ -597,7 +597,8 @@ public sealed class BSPrim : PhysicsActor
     }
     public override void SubscribeEvents(int ms) { 
         _subscribedEventsMs = ms;
-        _lastCollisionTime = Util.EnvironmentTickCount() - _subscribedEventsMs; // make first collision happen
+        // make sure first collision happens
+        _nextCollisionOkTime = Util.EnvironmentTickCount() - _subscribedEventsMs;
     }
     public override void UnSubscribeEvents() { 
         _subscribedEventsMs = 0;
@@ -1338,23 +1339,27 @@ public sealed class BSPrim : PhysicsActor
             _collidingGroundStep = _scene.SimulationStep;
         }
 
-        if (_subscribedEventsMs == 0) return;   // nothing in the object is waiting for collision events
+        // if someone is subscribed to collision events....
-        // throttle the collisions to the number of milliseconds specified in the subscription
+        if (_subscribedEventsMs != 0) {
-        int nowTime = _scene.SimulationNowTime;
+            // throttle the collisions to the number of milliseconds specified in the subscription
-        if (nowTime < (_lastCollisionTime + _subscribedEventsMs)) return;
+            int nowTime = _scene.SimulationNowTime;
-        _lastCollisionTime = nowTime;
+            if (nowTime >= _nextCollisionOkTime) {
+                _nextCollisionOkTime = nowTime + _subscribedEventsMs;
 
-        if (collisionCollection == null)
+                if (collisionCollection == null)
-            collisionCollection = new CollisionEventUpdate();
+                    collisionCollection = new CollisionEventUpdate();
-        collisionCollection.AddCollider(collidingWith, new ContactPoint(contactPoint, contactNormal, pentrationDepth));
+                collisionCollection.AddCollider(collidingWith, new ContactPoint(contactPoint, contactNormal, pentrationDepth));
+            }
+        }
     }
 
+    // The scene is telling us it's time to pass our collected collisions into the simulator
     public void SendCollisions()
     {
-        if (collisionCollection != null)
+        if (collisionCollection != null && collisionCollection.Count > 0)
         {
             base.SendCollisionUpdate(collisionCollection);
-            collisionCollection = null;
+            collisionCollection.Clear();
         }
     }
 }

OpenSim/Region/Physics/BulletSPlugin/BSScene.cs

@@ -52,6 +52,7 @@ using OpenSim.Region.Framework;
 // Should prim.link() and prim.delink() membership checking happen at taint time?
 // Mesh sharing. Use meshHash to tell if we already have a hull of that shape and only create once
 // Do attachments need to be handled separately? Need collision events. Do not collide with VolumeDetect
+// Use collision masks for collision with terrain and phantom objects 
 // Implement the genCollisions feature in BulletSim::SetObjectProperties (don't pass up unneeded collisions)
 // Implement LockAngularMotion
 // Decide if clearing forces is the right thing to do when setting position (BulletSim::SetObjectTranslation)
@@ -62,9 +63,6 @@ using OpenSim.Region.Framework;
 // Multiple contact points on collision?
 //    See code in ode::near... calls to collision_accounting_events()
 //    (This might not be a problem. ODE collects all the collisions with one object in one tick.)
-// Use collision masks for collision with terrain and phantom objects 
-// Figure out how to not allocate a new Dictionary and List for every collision
-//    in BSPrim.Collide() and BSCharacter.Collide(). Can the same ones be reused?
 // Raycast
 // 
 namespace OpenSim.Region.Physics.BulletSPlugin
@@ -405,6 +403,8 @@ public class BSScene : PhysicsScene, IPhysicsParameters
         // prevent simulation until we've been initialized
         if (!m_initialized) return 10.0f;
 
+        long simulateStartTime = Util.EnvironmentTickCount();
+
         // update the prim states while we know the physics engine is not busy
         ProcessTaints();
 
@@ -437,13 +437,18 @@ public class BSScene : PhysicsScene, IPhysicsParameters
             }
         }
 
-        // The SendCollision's batch up the collisions on the objects. Now push the collisions into the simulator.
+        // The above SendCollision's batch up the collisions on the objects.
+        //      Now push the collisions into the simulator.
         foreach (BSPrim bsp in m_primsWithCollisions)
             bsp.SendCollisions();
         m_primsWithCollisions.Clear();
+
+        // This is a kludge to get avatar movement updated. 
+        //   Don't send collisions only if there were collisions -- send everytime.
+        //   ODE sends collisions even if there are none and this is used to update
+        //   avatar animations and stuff.
         // foreach (BSCharacter bsc in m_avatarsWithCollisions)
         //     bsc.SendCollisions();
-        // This is a kludge to get avatar movement updated. ODE sends collisions even if there isn't any
         foreach (KeyValuePair<uint, BSCharacter> kvp in m_avatars)
             kvp.Value.SendCollisions();
         m_avatarsWithCollisions.Clear();
@@ -465,10 +470,12 @@ public class BSScene : PhysicsScene, IPhysicsParameters
                 if (m_avatars.TryGetValue(entprop.ID, out actor))
                 {
                     actor.UpdateProperties(entprop);
+                    continue;
                 }
             }
         }
 
+        // If enabled, call into the physics engine to dump statistics
         if (m_detailedStatsStep > 0)
         {
             if ((m_simulationStep % m_detailedStatsStep) == 0)
@@ -477,6 +484,11 @@ public class BSScene : PhysicsScene, IPhysicsParameters
             }
         }
 
+        // this is a waste since the outside routine also calcuates the physics simulation
+        //   period. TODO: There should be a way of computing physics frames from simulator computation.
+        // long simulateTotalTime = Util.EnvironmentTickCountSubtract(simulateStartTime);
+        // return (timeStep * (float)simulateTotalTime);
+
         // TODO: FIX THIS: fps calculation wrong. This calculation always returns about 1 in normal operation.
         return timeStep / (numSubSteps * m_fixedTimeStep) * 1000f;
     }
@@ -528,6 +540,7 @@ public class BSScene : PhysicsScene, IPhysicsParameters
     public override void SetWaterLevel(float baseheight) 
     {
         m_waterLevel = baseheight;
+        // TODO: pass to physics engine so things will float?
     }
     public float GetWaterLevel()
     {