7 files changed, 98 insertions, 72 deletions

diff --git a/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs b/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs
index f4bc495..9e5ce8f 100644
--- a/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs
+++ b/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs
@@ -136,10 +136,7 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
 
         public void SaveChangedAttachments(IScenePresence sp)
         {
-            // Need to copy this list because DetachToInventoryPrep mods it
-            List<SceneObjectGroup> attachments = new List<SceneObjectGroup>(sp.Attachments);
-
-            foreach (SceneObjectGroup grp in attachments)
+            foreach (SceneObjectGroup grp in sp.GetAttachments())
             {
                 if (grp.HasGroupChanged) // Resizer scripts?
                 {
@@ -273,14 +270,12 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
 
             // Remove any previous attachments
             UUID itemID = UUID.Zero;
-            foreach (SceneObjectGroup grp in sp.Attachments)
-            {
-                if (grp.AttachmentPoint == attachmentPt)
-                {
-                    itemID = grp.GetFromItemID();
-                    break;
-                }
-            }
+
+            List<SceneObjectGroup> attachments = sp.GetAttachments(attachmentPt);
+
+            // At the moment we can only deal with a single attachment
+            if (attachments.Count != 0)
+                itemID = attachments[0].GetFromItemID();
 
             if (itemID != UUID.Zero)
                 DetachSingleAttachmentToInv(itemID, sp);
diff --git a/OpenSim/Region/CoreModules/Avatar/Attachments/Tests/AttachmentsModuleTests.cs b/OpenSim/Region/CoreModules/Avatar/Attachments/Tests/AttachmentsModuleTests.cs
index b0146a1..363e258 100644
--- a/OpenSim/Region/CoreModules/Avatar/Attachments/Tests/AttachmentsModuleTests.cs
+++ b/OpenSim/Region/CoreModules/Avatar/Attachments/Tests/AttachmentsModuleTests.cs
@@ -106,7 +106,7 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments.Tests
 
             // Check status on scene presence
             Assert.That(m_presence.HasAttachments(), Is.True);
-            List<SceneObjectGroup> attachments = m_presence.Attachments;
+            List<SceneObjectGroup> attachments = m_presence.GetAttachments();
             Assert.That(attachments.Count, Is.EqualTo(1));
             SceneObjectGroup attSo = attachments[0];
             Assert.That(attSo.Name, Is.EqualTo(attName));
@@ -140,7 +140,7 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments.Tests
 
             // Check scene presence status
             Assert.That(m_presence.HasAttachments(), Is.True);
-            List<SceneObjectGroup> attachments = m_presence.Attachments;
+            List<SceneObjectGroup> attachments = m_presence.GetAttachments();
             Assert.That(attachments.Count, Is.EqualTo(1));
             SceneObjectGroup attSo = attachments[0];
             Assert.That(attSo.Name, Is.EqualTo(attName));
@@ -174,7 +174,7 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments.Tests
 
             // Check scene presence status
             Assert.That(m_presence.HasAttachments(), Is.False);
-            List<SceneObjectGroup> attachments = m_presence.Attachments;
+            List<SceneObjectGroup> attachments = m_presence.GetAttachments();
             Assert.That(attachments.Count, Is.EqualTo(0));
 
             // Check appearance status
@@ -208,7 +208,7 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments.Tests
 
             // Check status on scene presence
             Assert.That(m_presence.HasAttachments(), Is.False);
-            List<SceneObjectGroup> attachments = m_presence.Attachments;
+            List<SceneObjectGroup> attachments = m_presence.GetAttachments();
             Assert.That(attachments.Count, Is.EqualTo(0));
 
             // Check item status
@@ -237,7 +237,7 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments.Tests
             ScenePresence presence = SceneHelpers.AddScenePresence(scene, acd);
 
             Assert.That(presence.HasAttachments(), Is.True);
-            List<SceneObjectGroup> attachments = presence.Attachments;
+            List<SceneObjectGroup> attachments = presence.GetAttachments();
 
             Assert.That(attachments.Count, Is.EqualTo(1));
             SceneObjectGroup attSo = attachments[0];
diff --git a/OpenSim/Region/CoreModules/Framework/EntityTransfer/EntityTransferModule.cs b/OpenSim/Region/CoreModules/Framework/EntityTransfer/EntityTransferModule.cs
index 7963e53..c24cc17 100644
--- a/OpenSim/Region/CoreModules/Framework/EntityTransfer/EntityTransferModule.cs
+++ b/OpenSim/Region/CoreModules/Framework/EntityTransfer/EntityTransferModule.cs
@@ -208,7 +208,7 @@ namespace OpenSim.Region.CoreModules.Framework.EntityTransfer
                     sp.ControllingClient.SendLocalTeleport(position, lookAt, teleportFlags);
                     sp.Teleport(position);
 
-                    foreach (SceneObjectGroup grp in sp.Attachments)
+                    foreach (SceneObjectGroup grp in sp.GetAttachments())
                         sp.Scene.EventManager.TriggerOnScriptChangedEvent(grp.LocalId, (uint)Changed.TELEPORT);
                 }
                 else // Another region possibly in another simulator
@@ -559,11 +559,11 @@ namespace OpenSim.Region.CoreModules.Framework.EntityTransfer
 
         protected virtual void AgentHasMovedAway(ScenePresence sp, bool logout)
         {
-            foreach (SceneObjectGroup sop in sp.Attachments)
+            foreach (SceneObjectGroup sop in sp.GetAttachments())
             {
                 sop.Scene.DeleteSceneObject(sop, true);
             }
-            sp.Attachments.Clear();
+            sp.ClearAttachments();
         }
 
         protected void KillEntity(Scene scene, uint localID)
@@ -1764,34 +1764,33 @@ namespace OpenSim.Region.CoreModules.Framework.EntityTransfer
 
         protected bool CrossAttachmentsIntoNewRegion(GridRegion destination, ScenePresence sp, bool silent)
         {
-            List<SceneObjectGroup> m_attachments = sp.Attachments;
-            lock (m_attachments)
+            List<SceneObjectGroup> m_attachments = sp.GetAttachments();
+
+            // Validate
+            foreach (SceneObjectGroup gobj in m_attachments)
             {
-                // Validate
-                foreach (SceneObjectGroup gobj in m_attachments)
-                {
-                    if (gobj == null || gobj.IsDeleted)
-                        return false;
-                }
+                if (gobj == null || gobj.IsDeleted)
+                    return false;
+            }
 
-                foreach (SceneObjectGroup gobj in m_attachments)
+            foreach (SceneObjectGroup gobj in m_attachments)
+            {
+                // If the prim group is null then something must have happened to it!
+                if (gobj != null && gobj.RootPart != null)
                 {
-                    // If the prim group is null then something must have happened to it!
-                    if (gobj != null && gobj.RootPart != null)
-                    {
-                        // Set the parent localID to 0 so it transfers over properly.
-                        gobj.RootPart.SetParentLocalId(0);
-                        gobj.AbsolutePosition = gobj.RootPart.AttachedPos;
-                        gobj.IsAttachment = false;
-                        //gobj.RootPart.LastOwnerID = gobj.GetFromAssetID();
-                        m_log.DebugFormat("[ENTITY TRANSFER MODULE]: Sending attachment {0} to region {1}", gobj.UUID, destination.RegionName);
-                        CrossPrimGroupIntoNewRegion(destination, gobj, silent);
-                    }
+                    // Set the parent localID to 0 so it transfers over properly.
+                    gobj.RootPart.SetParentLocalId(0);
+                    gobj.AbsolutePosition = gobj.RootPart.AttachedPos;
+                    gobj.IsAttachment = false;
+                    //gobj.RootPart.LastOwnerID = gobj.GetFromAssetID();
+                    m_log.DebugFormat("[ENTITY TRANSFER MODULE]: Sending attachment {0} to region {1}", gobj.UUID, destination.RegionName);
+                    CrossPrimGroupIntoNewRegion(destination, gobj, silent);
                 }
-                m_attachments.Clear();
-
-                return true;
             }
+
+            sp.ClearAttachments();
+
+            return true;
         }
 
         #endregion
@@ -1840,7 +1839,9 @@ namespace OpenSim.Region.CoreModules.Framework.EntityTransfer
             int i = 0;
             if (sp.InTransitScriptStates.Count > 0)
             {
-                sp.Attachments.ForEach(delegate(SceneObjectGroup sog)
+                List<SceneObjectGroup> attachments = sp.GetAttachments();
+
+                foreach (SceneObjectGroup sog in attachments)
                 {
                     if (i < sp.InTransitScriptStates.Count)
                     {
@@ -1849,8 +1850,10 @@ namespace OpenSim.Region.CoreModules.Framework.EntityTransfer
                         sog.ResumeScripts();
                     }
                     else
-                        m_log.ErrorFormat("[ENTITY TRANSFER MODULE]: InTransitScriptStates.Count={0} smaller than Attachments.Count={1}", sp.InTransitScriptStates.Count, sp.Attachments.Count);
-                });
+                        m_log.ErrorFormat(
+                            "[ENTITY TRANSFER MODULE]: InTransitScriptStates.Count={0} smaller than Attachments.Count={1}",
+                            sp.InTransitScriptStates.Count, attachments.Count);
+                }
 
                 sp.InTransitScriptStates.Clear();
             }
diff --git a/OpenSim/Region/CoreModules/Scripting/WorldComm/WorldCommModule.cs b/OpenSim/Region/CoreModules/Scripting/WorldComm/WorldCommModule.cs
index 22352f5..057500c 100644
--- a/OpenSim/Region/CoreModules/Scripting/WorldComm/WorldCommModule.cs
+++ b/OpenSim/Region/CoreModules/Scripting/WorldComm/WorldCommModule.cs
@@ -283,7 +283,7 @@ namespace OpenSim.Region.CoreModules.Scripting.WorldComm
         }
 
         /// <summary>
-        /// Delivers the message to.
+        /// Delivers the message to a scene entity.
         /// </summary>
         /// <param name='target'>
         /// Target.
@@ -314,17 +314,19 @@ namespace OpenSim.Region.CoreModules.Scripting.WorldComm
                     m_scene.SimChatBroadcast(Utils.StringToBytes(msg), ChatTypeEnum.Owner, 0, pos, name, id, false);
                 }
 
-                List<SceneObjectGroup> attachments = sp.Attachments;
+                List<SceneObjectGroup> attachments = sp.GetAttachments();
+
                 // Nothing left to do
                 if (attachments == null)
                     return true;
 
                 // Get uuid of attachments
                 List<UUID> targets = new List<UUID>();
-                foreach ( SceneObjectGroup sog in attachments )
+                foreach (SceneObjectGroup sog in attachments)
                 {
                     targets.Add(sog.UUID);
                 }
+
                 // Need to check each attachment
                 foreach (ListenerInfo li in m_listenerManager.GetListeners(UUID.Zero, channel, name, id, msg))
                 {
@@ -334,9 +336,10 @@ namespace OpenSim.Region.CoreModules.Scripting.WorldComm
                     if (m_scene.GetSceneObjectPart(li.GetHostID()) == null)
                         continue;
 
-                    if ( targets.Contains(li.GetHostID()))
+                    if (targets.Contains(li.GetHostID()))
                         QueueMessage(new ListenerInfo(li, name, id, msg));
                 }
+
                 return true;
             }
 
@@ -363,6 +366,7 @@ namespace OpenSim.Region.CoreModules.Scripting.WorldComm
                     break;
                 }
             }
+            
             return true;
         }
 
diff --git a/OpenSim/Region/Framework/Interfaces/IScenePresence.cs b/OpenSim/Region/Framework/Interfaces/IScenePresence.cs
index b07c821..788b36f 100644
--- a/OpenSim/Region/Framework/Interfaces/IScenePresence.cs
+++ b/OpenSim/Region/Framework/Interfaces/IScenePresence.cs
@@ -58,10 +58,13 @@ namespace OpenSim.Region.Framework.Interfaces
         /// <summary>
         /// The scene objects attached to this avatar.
         /// </summary>
+        /// <returns>
+        /// A copy of the list.
+        /// </returns>
         /// <remarks>
         ///  Do not change this list directly - use methods such as
-        /// AddAttachment() and RemoveAttachment().  Lock this list when performing any read operations upon it.
+        /// AddAttachment() and RemoveAttachment().
         /// </remarks>
-        List<SceneObjectGroup> Attachments { get; }
+        List<SceneObjectGroup> GetAttachments();
     }
 }
\ No newline at end of file
diff --git a/OpenSim/Region/Framework/Scenes/ScenePresence.cs b/OpenSim/Region/Framework/Scenes/ScenePresence.cs
index 0a91989..f5c72e1 100644
--- a/OpenSim/Region/Framework/Scenes/ScenePresence.cs
+++ b/OpenSim/Region/Framework/Scenes/ScenePresence.cs
@@ -114,10 +114,13 @@ namespace OpenSim.Region.Framework.Scenes
         }
         protected ScenePresenceAnimator m_animator;
 
-        public List<SceneObjectGroup> Attachments
-        {
-            get { return m_attachments; }
-        }
+        /// <summary>
+        /// Attachments recorded on this avatar.
+        /// </summary>
+        /// <remarks>
+        /// TODO: For some reason, we effectively have a list both here and in Appearance.  Need to work out if this is
+        /// necessary.
+        /// </remarks>
         protected List<SceneObjectGroup> m_attachments = new List<SceneObjectGroup>();
 
         private Dictionary<UUID, ScriptControllers> scriptedcontrols = new Dictionary<UUID, ScriptControllers>();
@@ -940,15 +943,18 @@ namespace OpenSim.Region.Framework.Scenes
             // and it has already rezzed the attachments and started their scripts.
             // We do the following only for non-login agents, because their scripts
             // haven't started yet.
-            if (wasChild && Attachments != null && Attachments.Count > 0)
+            lock (m_attachments)
             {
-                m_log.DebugFormat("[SCENE PRESENCE]: Restarting scripts in attachments...");
-                // Resume scripts
-                Attachments.ForEach(delegate(SceneObjectGroup sog)
+                if (wasChild && m_attachments != null && m_attachments.Count > 0)
                 {
-                    sog.RootPart.ParentGroup.CreateScriptInstances(0, false, m_scene.DefaultScriptEngine, GetStateSource());
-                    sog.ResumeScripts();
-                });
+                    m_log.DebugFormat("[SCENE PRESENCE]: Restarting scripts in attachments...");
+                    // Resume scripts
+                    foreach (SceneObjectGroup sog in m_attachments)
+                    {
+                        sog.RootPart.ParentGroup.CreateScriptInstances(0, false, m_scene.DefaultScriptEngine, GetStateSource());
+                        sog.ResumeScripts();
+                    }
+                }
             }
 
             // send the animations of the other presences to me
@@ -3472,9 +3478,19 @@ namespace OpenSim.Region.Framework.Scenes
                 m_attachments.Add(gobj);
             }
         }
-        
+
         /// <summary>
-        /// Get the scene object attached to the given point.
+        /// Get all the presence's attachments.
+        /// </summary>
+        /// <returns>A copy of the list which contains the attachments.</returns>
+        public List<SceneObjectGroup> GetAttachments()
+        {
+            lock (m_attachments)
+                return new List<SceneObjectGroup>(m_attachments);
+        }
+
+        /// <summary>
+        /// Get the scene objects attached to the given point.
         /// </summary>
         /// <param name="attachmentPoint"></param>
         /// <returns>Returns an empty list if there were no attachments at the point.</returns>
@@ -3521,6 +3537,15 @@ namespace OpenSim.Region.Framework.Scenes
                 m_attachments.Remove(gobj);
         }
 
+        /// <summary>
+        /// Clear all attachments
+        /// </summary>
+        public void ClearAttachments()
+        {
+            lock (m_attachments)
+                m_attachments.Clear();
+        }
+
         public bool ValidateAttachments()
         {
             lock (m_attachments)
diff --git a/OpenSim/Region/OptionalModules/World/NPC/NPCModule.cs b/OpenSim/Region/OptionalModules/World/NPC/NPCModule.cs
index 79c79e4..e58dca2 100644
--- a/OpenSim/Region/OptionalModules/World/NPC/NPCModule.cs
+++ b/OpenSim/Region/OptionalModules/World/NPC/NPCModule.cs
@@ -144,14 +144,10 @@ namespace OpenSim.Region.OptionalModules.World.NPC
                     return false;
 
             // FIXME: An extremely bad bit of code that reaches directly into the attachments list and manipulates it
-            List<SceneObjectGroup> attachments = sp.Attachments;
-            lock (attachments)
-            {
-                foreach (SceneObjectGroup att in attachments)
-                    scene.DeleteSceneObject(att, false);
+            foreach (SceneObjectGroup att in sp.GetAttachments())
+                scene.DeleteSceneObject(att, false);
 
-                attachments.Clear();
-            }
+            sp.ClearAttachments();
 
             AvatarAppearance npcAppearance = new AvatarAppearance(appearance, true);
             sp.Appearance = npcAppearance;