SceneObjectGroup cleanup. Removes the default constructor and unnecessary null checks on m_rootPart

4 files changed, 52 insertions, 118 deletions

diff --git a/OpenSim/Region/Framework/Scenes/SceneObjectGroup.cs b/OpenSim/Region/Framework/Scenes/SceneObjectGroup.cs
index 6ba7e41..3c17bbe 100644
--- a/OpenSim/Region/Framework/Scenes/SceneObjectGroup.cs
+++ b/OpenSim/Region/Framework/Scenes/SceneObjectGroup.cs
@@ -250,16 +250,7 @@ namespace OpenSim.Region.Framework.Scenes
         /// </summary>
         public override Vector3 AbsolutePosition
         {
-            get
-            {
-                if (m_rootPart == null)
-                {
-                    throw new NullReferenceException(
-                        string.Format("[SCENE OBJECT GROUP]: Object {0} has no root part.", m_uuid));
-                }
-
-                return m_rootPart.GroupPosition;
-            }
+            get { return m_rootPart.GroupPosition; }
             set
             {
                 Vector3 val = value;
@@ -291,41 +282,19 @@ namespace OpenSim.Region.Framework.Scenes
 
         public override uint LocalId
         {
-            get
-            {
-                if (m_rootPart == null)
-                {
-                    m_log.Error("[SCENE OBJECT GROUP]: Unable to find the rootpart for a LocalId Request!");
-                    return 0;
-                }
-
-                return m_rootPart.LocalId;
-            }
+            get { return m_rootPart.LocalId; }
             set { m_rootPart.LocalId = value; }
         }
 
         public override UUID UUID
         {
-            get {
-                if (m_rootPart == null)
-                {
-                    m_log.Error("Got a null rootpart while requesting UUID. Called from: ", new Exception());
-                    return UUID.Zero;
-                }
-                else return m_rootPart.UUID;
-            }
+            get { return m_rootPart.UUID; }
             set { m_rootPart.UUID = value; }
         }
 
         public UUID OwnerID
         {
-            get
-            {
-                if (m_rootPart == null)
-                    return UUID.Zero;
-
-                return m_rootPart.OwnerID;
-            }
+            get { return m_rootPart.OwnerID; }
             set { m_rootPart.OwnerID = value; }
         }
 
@@ -366,7 +335,7 @@ namespace OpenSim.Region.Framework.Scenes
             {
                 m_isSelected = value;
                 // Tell physics engine that group is selected
-                if (m_rootPart != null && m_rootPart.PhysActor != null)
+                if (m_rootPart.PhysActor != null)
                 {
                     m_rootPart.PhysActor.Selected = value;
                     // Pass it on to the children.
@@ -399,13 +368,6 @@ namespace OpenSim.Region.Framework.Scenes
         #region Constructors
 
         /// <summary>
-        /// Constructor
-        /// </summary>
-        public SceneObjectGroup()
-        {
-        }
-
-        /// <summary>
         /// This constructor creates a SceneObjectGroup using a pre-existing SceneObjectPart.
         /// The original SceneObjectPart will be used rather than a copy, preserving
         /// its existing localID and UUID.
@@ -419,9 +381,8 @@ namespace OpenSim.Region.Framework.Scenes
         /// Constructor.  This object is added to the scene later via AttachToScene()
         /// </summary>
         public SceneObjectGroup(UUID ownerID, Vector3 pos, Quaternion rot, PrimitiveBaseShape shape)
-        {
-            Vector3 rootOffset = new Vector3(0, 0, 0);   
-            SetRootPart(new SceneObjectPart(ownerID, shape, pos, rot, rootOffset));
+        { 
+            SetRootPart(new SceneObjectPart(ownerID, shape, pos, rot, Vector3.Zero));
         }
 
         /// <summary>
@@ -462,11 +423,7 @@ namespace OpenSim.Region.Framework.Scenes
 
         public UUID GetFromItemID()
         {
-            if (m_rootPart != null)
-            {
-                return m_rootPart.FromItemID;
-            }
-            return UUID.Zero;
+            return m_rootPart.FromItemID;
         }
 
         /// <summary>
@@ -958,11 +915,7 @@ namespace OpenSim.Region.Framework.Scenes
 
         public byte GetAttachmentPoint()
         {
-            if (m_rootPart != null)
-            {
-                return m_rootPart.Shape.State;
-            }
-            return (byte)0;
+            return m_rootPart.Shape.State;
         }
 
         public void ClearPartAttachmentData()
@@ -1071,7 +1024,10 @@ namespace OpenSim.Region.Framework.Scenes
         /// </summary>
         /// <param name="part"></param>
         public void SetRootPart(SceneObjectPart part)
-        {            
+        {
+            if (part == null)
+                throw new ArgumentNullException("Cannot give SceneObjectGroup a null root SceneObjectPart");
+
             part.SetParent(this);
             m_rootPart = part;
             if (!IsAttachment)
@@ -1224,7 +1180,7 @@ namespace OpenSim.Region.Framework.Scenes
 
                         if (!silent)
                         {
-                            if (m_rootPart != null && part == m_rootPart)
+                            if (part == m_rootPart)
                                 avatars[i].ControllingClient.SendKillObject(m_regionHandle, part.LocalId);
                         }
                     }
@@ -1447,7 +1403,7 @@ namespace OpenSim.Region.Framework.Scenes
         /// <param name="part"></param>
         internal void SendPartFullUpdate(IClientAPI remoteClient, SceneObjectPart part, uint clientFlags)
         {
-            if (m_rootPart != null && m_rootPart.UUID == part.UUID)
+            if (m_rootPart.UUID == part.UUID)
             {
                 if (IsAttachment)
                 {
@@ -1881,12 +1837,6 @@ namespace OpenSim.Region.Framework.Scenes
             if (m_isDeleted)
                 return;
 
-            // This is what happens when an orphanced link set child prim's
-            // group was queued when it was linked
-            //
-            if (m_rootPart == null)
-                return;
-
             // Even temporary objects take part in physics (e.g. temp-on-rez bullets)
             //if ((RootPart.Flags & PrimFlags.TemporaryOnRez) != 0)
             //    return;
@@ -3129,26 +3079,22 @@ namespace OpenSim.Region.Framework.Scenes
             int yaxis = 4;
             int zaxis = 8;
 
-            if (m_rootPart != null)
-            {
-                setX = ((axis & xaxis) != 0) ? true : false;
-                setY = ((axis & yaxis) != 0) ? true : false;
-                setZ = ((axis & zaxis) != 0) ? true : false;
+            setX = ((axis & xaxis) != 0) ? true : false;
+            setY = ((axis & yaxis) != 0) ? true : false;
+            setZ = ((axis & zaxis) != 0) ? true : false;
 
-                float setval = (rotate10 > 0) ? 1f : 0f;
+            float setval = (rotate10 > 0) ? 1f : 0f;
 
-                if (setX)
-                    m_rootPart.RotationAxis.X = setval;
-                if (setY)
-                    m_rootPart.RotationAxis.Y = setval;
-                if (setZ)
-                    m_rootPart.RotationAxis.Z = setval;
-
-                if (setX || setY || setZ)
-                {
-                    m_rootPart.SetPhysicsAxisRotation();
-                }
+            if (setX)
+                m_rootPart.RotationAxis.X = setval;
+            if (setY)
+                m_rootPart.RotationAxis.Y = setval;
+            if (setZ)
+                m_rootPart.RotationAxis.Z = setval;
 
+            if (setX || setY || setZ)
+            {
+                m_rootPart.SetPhysicsAxisRotation();
             }
         }
 
diff --git a/OpenSim/Region/Framework/Scenes/Serialization/SceneObjectSerializer.cs b/OpenSim/Region/Framework/Scenes/Serialization/SceneObjectSerializer.cs
index 5ae81cd..fe74158 100644
--- a/OpenSim/Region/Framework/Scenes/Serialization/SceneObjectSerializer.cs
+++ b/OpenSim/Region/Framework/Scenes/Serialization/SceneObjectSerializer.cs
@@ -65,8 +65,6 @@ namespace OpenSim.Region.Framework.Scenes.Serialization
             //m_log.DebugFormat("[SOG]: Starting deserialization of SOG");
             //int time = System.Environment.TickCount;
 
-            SceneObjectGroup sceneObject = new SceneObjectGroup();            
-
             // libomv.types changes UUID to Guid
             xmlData = xmlData.Replace("<UUID>", "<Guid>");
             xmlData = xmlData.Replace("</UUID>", "</Guid>");
@@ -88,17 +86,13 @@ namespace OpenSim.Region.Framework.Scenes.Serialization
                 parts = doc.GetElementsByTagName("RootPart");
 
                 if (parts.Count == 0)
-                {
                     throw new Exception("Invalid Xml format - no root part");
-                }
-                else
-                {
-                    sr = new StringReader(parts[0].InnerXml);
-                    reader = new XmlTextReader(sr);
-                    sceneObject.SetRootPart(SceneObjectPart.FromXml(fromUserInventoryItemID, reader));
-                    reader.Close();
-                    sr.Close();
-                }
+
+                sr = new StringReader(parts[0].InnerXml);
+                reader = new XmlTextReader(sr);
+                SceneObjectGroup sceneObject = new SceneObjectGroup(SceneObjectPart.FromXml(fromUserInventoryItemID, reader));
+                reader.Close();
+                sr.Close();
 
                 parts = doc.GetElementsByTagName("Part");
 
@@ -119,16 +113,15 @@ namespace OpenSim.Region.Framework.Scenes.Serialization
                 // Script state may, or may not, exist. Not having any, is NOT
                 // ever a problem.
                 sceneObject.LoadScriptState(doc);
+
+                return sceneObject;
             }
             catch (Exception e)
             {
                 m_log.ErrorFormat(
                     "[SERIALIZER]: Deserialization of xml failed with {0}.  xml was {1}", e, xmlData);
+                return null;
             }
-
-            //m_log.DebugFormat("[SERIALIZER]: Finished deserialization of SOG {0}, {1}ms", Name, System.Environment.TickCount - time);
-
-            return sceneObject;
         }      
 
         /// <summary>
@@ -194,8 +187,6 @@ namespace OpenSim.Region.Framework.Scenes.Serialization
         {
             //m_log.DebugFormat("[SOG]: Starting deserialization of SOG");
             //int time = System.Environment.TickCount;
-
-            SceneObjectGroup sceneObject = new SceneObjectGroup();
             
             // libomv.types changes UUID to Guid
             xmlData = xmlData.Replace("<UUID>", "<Guid>");
@@ -212,21 +203,23 @@ namespace OpenSim.Region.Framework.Scenes.Serialization
 
                 XmlNodeList parts = doc.GetElementsByTagName("SceneObjectPart");
 
-                // Process the root part first
-                if (parts.Count > 0)
+                if (parts.Count == 0)
                 {
-                    StringReader      sr = new StringReader(parts[0].OuterXml);
-                    XmlTextReader reader = new XmlTextReader(sr);
-                    sceneObject.SetRootPart(SceneObjectPart.FromXml(reader));
-                    reader.Close();
-                    sr.Close();
+                    m_log.ErrorFormat("[SERIALIZER]: Deserialization of xml failed: No SceneObjectPart nodes. xml was " + xmlData);
+                    return null;
                 }
 
+                StringReader sr = new StringReader(parts[0].OuterXml);
+                XmlTextReader reader = new XmlTextReader(sr);
+                SceneObjectGroup sceneObject = new SceneObjectGroup(SceneObjectPart.FromXml(reader));
+                reader.Close();
+                sr.Close();
+
                 // Then deal with the rest
                 for (int i = 1; i < parts.Count; i++)
                 {
-                    StringReader      sr = new StringReader(parts[i].OuterXml);
-                    XmlTextReader reader = new XmlTextReader(sr);
+                    sr = new StringReader(parts[i].OuterXml);
+                    reader = new XmlTextReader(sr);
                     SceneObjectPart part = SceneObjectPart.FromXml(reader);
                     sceneObject.AddPart(part);
                     part.StoreUndoState();
@@ -238,15 +231,13 @@ namespace OpenSim.Region.Framework.Scenes.Serialization
                 // ever a problem.
 
                 sceneObject.LoadScriptState(doc);
+                return sceneObject;
             }
             catch (Exception e)
             {
                 m_log.ErrorFormat("[SERIALIZER]: Deserialization of xml failed with {0}.  xml was {1}", e, xmlData);
+                return null;
             }
-
-            //m_log.DebugFormat("[SERIALIZER]: Finished deserialization of SOG {0}, {1}ms", Name, System.Environment.TickCount - time);
-
-            return sceneObject;
         }         
 
         /// <summary>
diff --git a/OpenSim/Region/Framework/Scenes/Tests/EntityManagerTests.cs b/OpenSim/Region/Framework/Scenes/Tests/EntityManagerTests.cs
index bb8f27d..3b0e77f 100644
--- a/OpenSim/Region/Framework/Scenes/Tests/EntityManagerTests.cs
+++ b/OpenSim/Region/Framework/Scenes/Tests/EntityManagerTests.cs
@@ -128,7 +128,6 @@ namespace OpenSim.Region.Framework.Scenes.Tests
 
         private SceneObjectGroup NewSOG()
         {
-            SceneObjectGroup sog = new SceneObjectGroup();
             SceneObjectPart sop = new SceneObjectPart(UUID.Random(), PrimitiveBaseShape.Default, Vector3.Zero, Quaternion.Identity, Vector3.Zero);
             sop.Name = RandomName();
             sop.Description = sop.Name;
@@ -136,9 +135,8 @@ namespace OpenSim.Region.Framework.Scenes.Tests
             sop.SitName = RandomName();
             sop.TouchName = RandomName();
             sop.ObjectFlags |= (uint)PrimFlags.Phantom;
-            
-            sog.SetRootPart(sop);
 
+            SceneObjectGroup sog = new SceneObjectGroup(sop);
             scene.AddNewSceneObject(sog, false);
             
             return sog;
diff --git a/OpenSim/Region/Framework/Scenes/Tests/ScenePresenceTests.cs b/OpenSim/Region/Framework/Scenes/Tests/ScenePresenceTests.cs
index 7fb2d25..19c0fea 100644
--- a/OpenSim/Region/Framework/Scenes/Tests/ScenePresenceTests.cs
+++ b/OpenSim/Region/Framework/Scenes/Tests/ScenePresenceTests.cs
@@ -407,9 +407,8 @@ namespace OpenSim.Region.Framework.Scenes.Tests
             sop.Shape.State = 1;
             sop.OwnerID = agent;
 
-            SceneObjectGroup sog = new SceneObjectGroup();
+            SceneObjectGroup sog = new SceneObjectGroup(sop);
             sog.SetScene(scene);
-            sog.SetRootPart(sop);
 
             return sog;
         }