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

9 files changed, 78 insertions, 140 deletions

diff --git a/OpenSim/Data/NHibernate/NHibernateRegionData.cs b/OpenSim/Data/NHibernate/NHibernateRegionData.cs
index 26ec500..673ca6f 100644
--- a/OpenSim/Data/NHibernate/NHibernateRegionData.cs
+++ b/OpenSim/Data/NHibernate/NHibernateRegionData.cs
@@ -178,25 +178,38 @@ namespace OpenSim.Data.NHibernate
 
         private SceneObjectGroup LoadObject(UUID uuid, UUID region)
         {
-            SceneObjectGroup group = new SceneObjectGroup();
-
             ICriteria criteria = manager.GetSession().CreateCriteria(typeof(SceneObjectPart));
             criteria.Add(Expression.Eq("RegionID", region));
             criteria.Add(Expression.Eq("ParentUUID", uuid));
             criteria.AddOrder(Order.Asc("ParentID"));
 
-            foreach (SceneObjectPart p in criteria.List())
+            IList<SceneObjectPart> parts = criteria.List<SceneObjectPart>();
+
+            SceneObjectGroup group = null;
+
+            // Find the root part
+            for (int i = 0; i < parts.Count; i++)
             {
-                // root part
-                if (p.UUID == uuid)
+                if (parts[i].UUID == uuid)
                 {
-                    group.SetRootPart(p);
+                    group = new SceneObjectGroup(parts[i]);
+                    break;
                 }
-                else
+            }
+
+            // Add the children parts
+            if (group != null)
+            {
+                for (int i = 0; i < parts.Count; i++)
                 {
-                    group.AddPart(p);
+                    if (parts[i].UUID != uuid)
+                        group.AddPart(parts[i]);
                 }
             }
+            else
+            {
+                m_log.Error("[NHIBERNATE]: LoadObject() Attempted to load a SceneObjectGroup with no root SceneObjectPart ");
+            }
 
             return group;
         }
@@ -237,8 +250,7 @@ namespace OpenSim.Data.NHibernate
                 // root part
                 if (p.UUID == p.ParentUUID)
                 {
-                    SceneObjectGroup group = new SceneObjectGroup();
-                    group.SetRootPart(p);
+                    SceneObjectGroup group = new SceneObjectGroup(p);
                     SOG.Add(p.ParentUUID, group);
                 }
                 else
diff --git a/OpenSim/Data/SQLite/SQLiteRegionData.cs b/OpenSim/Data/SQLite/SQLiteRegionData.cs
index 0259ac5..ea076fe 100644
--- a/OpenSim/Data/SQLite/SQLiteRegionData.cs
+++ b/OpenSim/Data/SQLite/SQLiteRegionData.cs
@@ -416,7 +416,6 @@ namespace OpenSim.Data.SQLite
                         
                         if (uuid == objID) //is new SceneObjectGroup ?
                         {
-                            SceneObjectGroup group = new SceneObjectGroup();
                             prim = buildPrim(primRow);
                             DataRow shapeRow = shapes.Rows.Find(prim.UUID.ToString());
                             if (shapeRow != null)
@@ -430,7 +429,7 @@ namespace OpenSim.Data.SQLite
                                 prim.Shape = PrimitiveBaseShape.Default;
                             }
                             
-                            group.SetRootPart(prim);
+                            SceneObjectGroup group = new SceneObjectGroup(prim);
                             createdObjects.Add(group.UUID, group);
                             retvals.Add(group);
                             LoadItems(prim);
diff --git a/OpenSim/Data/Tests/BasicRegionTest.cs b/OpenSim/Data/Tests/BasicRegionTest.cs
index 8474921..c66ab7c 100644
--- a/OpenSim/Data/Tests/BasicRegionTest.cs
+++ b/OpenSim/Data/Tests/BasicRegionTest.cs
@@ -322,9 +322,8 @@ namespace OpenSim.Data.Tests
             // This is necessary or object will not be inserted in DB            
             sop.ObjectFlags = 0;
 
-            SceneObjectGroup sog = new SceneObjectGroup();
+            SceneObjectGroup sog = new SceneObjectGroup(sop);
             sog.SetScene(scene); // Reguired by nhibernate database module.
-            sog.SetRootPart(sop);
             
             // Inserts group in DB
             db.StoreObject(sog,region3);
@@ -1003,9 +1002,8 @@ namespace OpenSim.Data.Tests
             sop.UUID = uuid;
             sop.Shape = PrimitiveBaseShape.Default;
 
-            SceneObjectGroup sog = new SceneObjectGroup();
+            SceneObjectGroup sog = new SceneObjectGroup(sop);
             sog.SetScene(scene);
-            sog.SetRootPart(sop); 
 
             return sog;
         }
diff --git a/OpenSim/Region/Examples/SimpleModule/ComplexObject.cs b/OpenSim/Region/Examples/SimpleModule/ComplexObject.cs
index f9c3fa6..3809749 100644
--- a/OpenSim/Region/Examples/SimpleModule/ComplexObject.cs
+++ b/OpenSim/Region/Examples/SimpleModule/ComplexObject.cs
@@ -73,10 +73,6 @@ namespace OpenSim.Region.Examples.SimpleModule
             base.UpdateMovement();
         }
 
-        public ComplexObject()
-        {
-        }
-
         public ComplexObject(Scene scene, ulong regionHandle, UUID ownerID, uint localID, Vector3 pos)
             : base(ownerID, pos, PrimitiveBaseShape.Default)
         {
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;
         }
diff --git a/OpenSim/Region/OptionalModules/ContentManagementSystem/PointMetaEntity.cs b/OpenSim/Region/OptionalModules/ContentManagementSystem/PointMetaEntity.cs
index 59b7289..fbe43d6 100644
--- a/OpenSim/Region/OptionalModules/ContentManagementSystem/PointMetaEntity.cs
+++ b/OpenSim/Region/OptionalModules/ContentManagementSystem/PointMetaEntity.cs
@@ -66,7 +66,6 @@ namespace OpenSim.Region.OptionalModules.ContentManagement
 
         private void CreatePointEntity(Scene scene, UUID uuid, Vector3 groupPos)
         {
-            SceneObjectGroup x = new SceneObjectGroup();
             SceneObjectPart y = new SceneObjectPart();
 
             //Initialize part
@@ -93,8 +92,8 @@ namespace OpenSim.Region.OptionalModules.ContentManagement
             y.TrimPermissions();
 
             //Initialize group and add part as root part
+            SceneObjectGroup x = new SceneObjectGroup(y);
             x.SetScene(scene);
-            x.SetRootPart(y);
             x.RegionHandle = scene.RegionInfo.RegionHandle;
             x.SetScene(scene);