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

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 (parts[i].UUID == uuid)
-                if (p.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();
+                    SceneObjectGroup group = new SceneObjectGroup(p);
-                    group.SetRootPart(p);
                     SOG.Add(p.ParentUUID, group);
                 }
                 else

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);

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;
         }

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)
         {

OpenSim/Region/Framework/Scenes/SceneObjectGroup.cs

@@ -250,16 +250,7 @@ namespace OpenSim.Region.Framework.Scenes
         /// </summary>
         public override Vector3 AbsolutePosition
         {
-            get
+            get { return m_rootPart.GroupPosition; }
-            {
-                if (m_rootPart == null)
-                {
-                    throw new NullReferenceException(
-                        string.Format("[SCENE OBJECT GROUP]: Object {0} has no root part.", m_uuid));
-                }
-
-                return m_rootPart.GroupPosition;
-            }
             set
             {
                 Vector3 val = value;
@@ -291,41 +282,19 @@ namespace OpenSim.Region.Framework.Scenes
 
         public override uint LocalId
         {
-            get
+            get { return m_rootPart.LocalId; }
-            {
-                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;
-            }
             set { m_rootPart.LocalId = value; }
         }
 
         public override UUID UUID
         {
-            get {
+            get { return m_rootPart.UUID; }
-                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;
-            }
             set { m_rootPart.UUID = value; }
         }
 
         public UUID OwnerID
         {
-            get
+            get { return m_rootPart.OwnerID; }
-            {
-                if (m_rootPart == null)
-                    return UUID.Zero;
-
-                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.
@@ -398,13 +367,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
@@ -420,8 +382,7 @@ namespace OpenSim.Region.Framework.Scenes
         /// </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, Vector3.Zero));
-            SetRootPart(new SceneObjectPart(ownerID, shape, pos, rot, rootOffset));
         }
 
         /// <summary>
@@ -462,11 +423,7 @@ namespace OpenSim.Region.Framework.Scenes
 
         public UUID GetFromItemID()
         {
-            if (m_rootPart != null)
+            return m_rootPart.FromItemID;
-            {
-                return m_rootPart.FromItemID;
-            }
-            return UUID.Zero;
         }
 
         /// <summary>
@@ -958,11 +915,7 @@ namespace OpenSim.Region.Framework.Scenes
 
         public byte GetAttachmentPoint()
         {
-            if (m_rootPart != null)
+            return m_rootPart.Shape.State;
-            {
-                return m_rootPart.Shape.State;
-            }
-            return (byte)0;
         }
 
         public void ClearPartAttachmentData()
@@ -1072,6 +1025,9 @@ namespace OpenSim.Region.Framework.Scenes
         /// <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;
+
+            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)
             {
-                setX = ((axis & xaxis) != 0) ? true : false;
+                m_rootPart.SetPhysicsAxisRotation();
-                setY = ((axis & yaxis) != 0) ? true : false;
-                setZ = ((axis & zaxis) != 0) ? true : false;
-
-                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();
-                }
-
             }
         }
 

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);
-                    sr = new StringReader(parts[0].InnerXml);
+                SceneObjectGroup sceneObject = new SceneObjectGroup(SceneObjectPart.FromXml(fromUserInventoryItemID, reader));
-                    reader = new XmlTextReader(sr);
+                reader.Close();
-                    sceneObject.SetRootPart(SceneObjectPart.FromXml(fromUserInventoryItemID, reader));
+                sr.Close();
-                    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>
@@ -195,8 +188,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>");
@@ -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);
+                    m_log.ErrorFormat("[SERIALIZER]: Deserialization of xml failed: No SceneObjectPart nodes. xml was " + xmlData);
-                    XmlTextReader reader = new XmlTextReader(sr);
+                    return null;
-                    sceneObject.SetRootPart(SceneObjectPart.FromXml(reader));
-                    reader.Close();
-                    sr.Close();
                 }
 
+                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);
+                    sr = new StringReader(parts[i].OuterXml);
-                    XmlTextReader reader = new XmlTextReader(sr);
+                    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>

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;
@@ -137,8 +136,7 @@ namespace OpenSim.Region.Framework.Scenes.Tests
             sop.TouchName = RandomName();
             sop.ObjectFlags |= (uint)PrimFlags.Phantom;
 
-            sog.SetRootPart(sop);
+            SceneObjectGroup sog = new SceneObjectGroup(sop);
-
             scene.AddNewSceneObject(sog, false);
             
             return sog;

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;
         }

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);