Code cleanup related to ScenePresence.PhysicsActor and OdeScene/OdeCharacter

Stop hiding RemoveAvatar failure, add log messages when characters are removed through defects or re-added unexpectedly.
Add commented out log lines for future use.
Use automatic property for PhysicsActor for better code readability and simplicity

OpenSim/Region/Framework/Scenes/ScenePresence.cs

@@ -285,16 +285,10 @@ namespace OpenSim.Region.Framework.Scenes
 
         #region Properties
 
-        protected PhysicsActor m_physicsActor;
-
         /// <summary>
         /// Physical scene representation of this Avatar.
         /// </summary>
-        public PhysicsActor PhysicsActor
+        public PhysicsActor PhysicsActor { get; private set; }
-        {
-            set { m_physicsActor = value; }
-            get { return m_physicsActor; }
-        }
 
         private byte m_movementflag;
 
@@ -1032,18 +1026,19 @@ namespace OpenSim.Region.Framework.Scenes
         {
             if (PhysicsActor != null)
             {
-                try
+//                PhysicsActor.OnRequestTerseUpdate -= SendTerseUpdateToAllClients;
-                {
+                PhysicsActor.OnOutOfBounds -= OutOfBoundsCall;
-                    PhysicsActor.OnRequestTerseUpdate -= SendTerseUpdateToAllClients;
+                m_scene.PhysicsScene.RemoveAvatar(PhysicsActor);
-                    PhysicsActor.OnOutOfBounds -= OutOfBoundsCall;
+                PhysicsActor.UnSubscribeEvents();
-                    m_scene.PhysicsScene.RemoveAvatar(PhysicsActor);
+                PhysicsActor.OnCollisionUpdate -= PhysicsCollisionUpdate;
-                    PhysicsActor.UnSubscribeEvents();
+                PhysicsActor = null;
-                    PhysicsActor.OnCollisionUpdate -= PhysicsCollisionUpdate;
-                    PhysicsActor = null;
-                }
-                catch
-                { }
             }
+//            else
+//            {
+//                m_log.ErrorFormat(
+//                    "[SCENE PRESENCE]: Attempt to remove physics actor for {0} on {1} but this scene presence has no physics actor",
+//                    Name, Scene.RegionInfo.RegionName);
+//            }
         }
 
         /// <summary>
@@ -1925,7 +1920,7 @@ namespace OpenSim.Region.Framework.Scenes
 //                m_log.DebugFormat("[SCENE PRESENCE]: {0} {1}", SitTargetisSet, SitTargetUnOccupied);
 
             if (PhysicsActor != null)
-                m_sitAvatarHeight = m_physicsActor.Size.Z;
+                m_sitAvatarHeight = PhysicsActor.Size.Z;
 
             bool canSit = false;
             pos = part.AbsolutePosition + offset;
@@ -2570,7 +2565,10 @@ namespace OpenSim.Region.Framework.Scenes
             // only send update from root agents to other clients; children are only "listening posts"
             if (IsChildAgent)
             {
-                m_log.Warn("[SCENE PRESENCE]: Attempt to send avatar data from a child agent");
+                m_log.WarnFormat(
+                    "[SCENE PRESENCE]: Attempt to send avatar data from a child agent for {0} in {1}",
+                    Name, Scene.RegionInfo.RegionName);
+
                 return;
             }
 
@@ -2628,7 +2626,10 @@ namespace OpenSim.Region.Framework.Scenes
             // only send update from root agents to other clients; children are only "listening posts"
             if (IsChildAgent)
             {
-                m_log.Warn("[SCENE PRESENCE]: Attempt to send avatar data from a child agent");
+                m_log.WarnFormat(
+                    "[SCENE PRESENCE]: Attempt to send avatar data from a child agent for {0} in {1}",
+                    Name, Scene.RegionInfo.RegionName);
+
                 return;
             }
             
@@ -3039,8 +3040,6 @@ namespace OpenSim.Region.Framework.Scenes
 
             CameraPosition = cAgentData.Center + offset;
 
-            //SetHeight(cAgentData.AVHeight);
-
             if ((cAgentData.Throttles != null) && cAgentData.Throttles.Length > 0)
                 ControllingClient.SetChildAgentThrottle(cAgentData.Throttles);
 
@@ -3251,6 +3250,13 @@ namespace OpenSim.Region.Framework.Scenes
 //                "[SCENE PRESENCE]: Adding physics actor for {0}, ifFlying = {1} in {2}",
 //                Name, isFlying, Scene.RegionInfo.RegionName);
 
+            if (PhysicsActor != null)
+            {
+                m_log.ErrorFormat(
+                    "[SCENE PRESENCE]: Adding physics actor for {0} to {1} but this scene presence already has a physics actor",
+                    Name, Scene.RegionInfo.RegionName);
+            }
+
             if (Appearance.AvatarHeight == 0)
                 Appearance.SetHeight();
 
@@ -3258,11 +3264,10 @@ namespace OpenSim.Region.Framework.Scenes
 
             Vector3 pVec = AbsolutePosition;
 
-            // Old bug where the height was in centimeters instead of meters
+            PhysicsActor = scene.AddAvatar(
-            PhysicsActor = scene.AddAvatar(LocalId, Firstname + "." + Lastname, pVec,
+                LocalId, Firstname + "." + Lastname, pVec,
-                                                 new Vector3(0f, 0f, Appearance.AvatarHeight), isFlying);
+                new Vector3(0f, 0f, Appearance.AvatarHeight), isFlying);
 
-            scene.AddPhysicsActorTaint(PhysicsActor);
             //PhysicsActor.OnRequestTerseUpdate += SendTerseUpdateToAllClients;
             PhysicsActor.OnCollisionUpdate += PhysicsCollisionUpdate;
             PhysicsActor.OnOutOfBounds += OutOfBoundsCall; // Called for PhysicsActors when there's something wrong

OpenSim/Region/Physics/OdePlugin/ODECharacter.cs

@@ -137,7 +137,7 @@ namespace OpenSim.Region.Physics.OdePlugin
         internal IntPtr Body = IntPtr.Zero;
         private OdeScene _parent_scene;
         internal IntPtr Shell = IntPtr.Zero;
-        internal IntPtr Amotor = IntPtr.Zero;
+        private IntPtr Amotor = IntPtr.Zero;
         private d.Mass ShellMass;
 
         private int m_eventsubscription = 0;
@@ -549,8 +549,8 @@ namespace OpenSim.Region.Physics.OdePlugin
         {
             get
             {
-                float AVvolume = (float) (Math.PI*Math.Pow(CAPSULE_RADIUS, 2)*CAPSULE_LENGTH);
+                float AVvolume = (float)(Math.PI * Math.Pow(CAPSULE_RADIUS, 2) * CAPSULE_LENGTH);
-                return m_density*AVvolume;
+                return m_density * AVvolume;
             }
         }
 

OpenSim/Region/Physics/OdePlugin/OdeScene.cs

@@ -1703,7 +1703,6 @@ namespace OpenSim.Region.Physics.OdePlugin
 //                "[ODE SCENE]: Removing physics character {0} {1} from physics scene {2}",
 //                actor.Name, actor.LocalID, Name);
 
-            //m_log.Debug("[PHYSICS]:ODELOCK");
             ((OdeCharacter) actor).Destroy();
         }
 
@@ -1711,11 +1710,12 @@ namespace OpenSim.Region.Physics.OdePlugin
         {
             if (!_characters.Contains(chr))
             {
-//                m_log.DebugFormat(
-//                    "[ODE SCENE]: Adding physics character {0} {1} to physics scene {2}", chr.Name, chr.LocalID, Name);
-
                 _characters.Add(chr);
 
+//                m_log.DebugFormat(
+//                    "[ODE SCENE]: Adding physics character {0} {1} to physics scene {2}.  Count now {3}",
+//                    chr.Name, chr.LocalID, Name, _characters.Count);
+
                 if (chr.bad)
                     m_log.ErrorFormat("[ODE SCENE]: Added BAD actor {0} to characters list", chr.m_uuid);
             }
@@ -1730,11 +1730,19 @@ namespace OpenSim.Region.Physics.OdePlugin
         internal void RemoveCharacter(OdeCharacter chr)
         {
             if (_characters.Contains(chr))
+            {
                 _characters.Remove(chr);
+
+//                m_log.DebugFormat(
+//                    "[ODE SCENE]: Removing physics character {0} {1} from physics scene {2}.  Count now {3}",
+//                    chr.Name, chr.LocalID, Name, _characters.Count);
+            }
             else
+            {
                 m_log.ErrorFormat(
                     "[ODE SCENE]: Tried to remove character {0} {1} but they are not in the list!",
                     chr.Name, chr.LocalID);
+            }
         }
 
         private PhysicsActor AddPrim(String name, Vector3 position, Vector3 size, Quaternion rotation,
@@ -1772,7 +1780,7 @@ namespace OpenSim.Region.Physics.OdePlugin
         public override PhysicsActor AddPrimShape(string primName, PrimitiveBaseShape pbs, Vector3 position,
                                                   Vector3 size, Quaternion rotation, bool isPhysical, uint localid)
         {
-//            m_log.DebugFormat("[ODE SCENE]: Adding physics prim {0} {1} to physics scene {2}", primName, localid, Name);
+            m_log.DebugFormat("[ODE SCENE]: Adding physics prim {0} {1} to physics scene {2}", primName, localid, Name);
 
             return AddPrim(primName, position, size, rotation, pbs, isPhysical, localid);
         }
@@ -2762,6 +2770,10 @@ namespace OpenSim.Region.Physics.OdePlugin
                         {
                             foreach (OdeCharacter actor in defects)
                             {
+                                m_log.ErrorFormat(
+                                    "[ODE SCENE]: Removing physics character {0} {1} from physics scene {2} due to defect found when moving",
+                                    actor.Name, actor.LocalID, Name);
+
                                 RemoveCharacter(actor);
                                 actor.DestroyOdeStructures();
                             }
@@ -2832,6 +2844,10 @@ namespace OpenSim.Region.Physics.OdePlugin
                 {
                     foreach (OdeCharacter actor in defects)
                     {
+                        m_log.ErrorFormat(
+                            "[ODE SCENE]: Removing physics character {0} {1} from physics scene {2} due to defect found when updating position and velocity",
+                            actor.Name, actor.LocalID, Name);
+
                         RemoveCharacter(actor);
                         actor.DestroyOdeStructures();
                     }