Change AttachmentsModule.DetachSingleAttachmentToInv() to accept a SOG directly instead of an item ID to then shuffle through attachments, saving CPU busywork.

Almost all callers already had the sog to hand.
Still checking that it's really an attachment, but now by inspecting SOG.AttachedAvatar

2 files changed, 45 insertions, 40 deletions

diff --git a/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs b/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs
index e9f0488..af30a8e 100644
--- a/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs
+++ b/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs
@@ -302,10 +302,8 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
                     // At the moment we can only deal with a single attachment
                     if (attachments.Count != 0)
                     {
-                        UUID oldAttachmentItemID = attachments[0].FromItemID;
-        
-                        if (oldAttachmentItemID != UUID.Zero)
-                            DetachSingleAttachmentToInvInternal(sp, oldAttachmentItemID);
+                        if (attachments[0].FromItemID != UUID.Zero)
+                            DetachSingleAttachmentToInvInternal(sp, attachments[0]);
                         else
                             m_log.WarnFormat(
                                 "[ATTACHMENTS MODULE]: When detaching existing attachment {0} {1} at point {2} to make way for {3} {4} for {5}, couldn't find the associated item ID to adjust inventory attachment record!",
@@ -442,18 +440,27 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
             m_scene.EventManager.TriggerOnAttach(so.LocalId, so.UUID, UUID.Zero);
         }
 
-        public void DetachSingleAttachmentToInv(IScenePresence sp, UUID itemID)
+        public void DetachSingleAttachmentToInv(IScenePresence sp, SceneObjectGroup so)
         {
             lock (sp.AttachmentsSyncLock)
             {
                 // Save avatar attachment information
 //                m_log.Debug("[ATTACHMENTS MODULE]: Detaching from UserID: " + sp.UUID + ", ItemID: " + itemID);
 
-                bool changed = sp.Appearance.DetachAttachment(itemID);
+                if (so.AttachedAvatar != sp.UUID)
+                {
+                    m_log.WarnFormat(
+                        "[ATTACHMENTS MODULE]: Tried to detach object {0} from {1} {2} but attached avatar id was {3} in {4}",
+                        so.Name, sp.Name, sp.UUID, so.AttachedAvatar, m_scene.RegionInfo.RegionName);
+
+                    return;
+                }
+
+                bool changed = sp.Appearance.DetachAttachment(so.FromItemID);
                 if (changed && m_scene.AvatarFactory != null)
                     m_scene.AvatarFactory.QueueAppearanceSave(sp.UUID);
 
-                DetachSingleAttachmentToInvInternal(sp, itemID);
+                DetachSingleAttachmentToInvInternal(sp, so);
             }
         }
         
@@ -657,39 +664,21 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
             return newItem;
         }
 
-        // What makes this method odd and unique is it tries to detach using an UUID....     Yay for standards.
-        // To LocalId or UUID, *THAT* is the question. How now Brown UUID??
-        private void DetachSingleAttachmentToInvInternal(IScenePresence sp, UUID itemID)
+        private void DetachSingleAttachmentToInvInternal(IScenePresence sp, SceneObjectGroup so)
         {
             //            m_log.DebugFormat("[ATTACHMENTS MODULE]: Detaching item {0} to inventory for {1}", itemID, sp.Name);
 
-            if (itemID == UUID.Zero) // If this happened, someone made a mistake....
-                return;
-
-            lock (sp.AttachmentsSyncLock)
-            {
-                List<SceneObjectGroup> attachments = sp.GetAttachments();
-
-                foreach (SceneObjectGroup group in attachments)
-                {
-                    if (group.FromItemID == itemID)
-                    {
-                        m_scene.EventManager.TriggerOnAttach(group.LocalId, itemID, UUID.Zero);
-                        sp.RemoveAttachment(group);
-                        m_scene.DeleteSceneObject(group, false);
-
-                        // Prepare sog for storage
-                        group.AttachedAvatar = UUID.Zero;
-                        group.RootPart.SetParentLocalId(0);
-                        group.IsAttachment = false;
-                        group.AbsolutePosition = group.RootPart.AttachedPos;
+            m_scene.EventManager.TriggerOnAttach(so.LocalId, so.FromItemID, UUID.Zero);
+            sp.RemoveAttachment(so);
+            m_scene.DeleteSceneObject(so, false);
 
-                        UpdateKnownItem(sp, group, true);
+            // Prepare sog for storage
+            so.AttachedAvatar = UUID.Zero;
+            so.RootPart.SetParentLocalId(0);
+            so.IsAttachment = false;
+            so.AbsolutePosition = so.RootPart.AttachedPos;
 
-                        return;
-                    }
-                }
-            }
+            UpdateKnownItem(sp, so, true);
         }
 
         private SceneObjectGroup RezSingleAttachmentFromInventoryInternal(
@@ -897,8 +886,9 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
 
             ScenePresence sp = m_scene.GetScenePresence(remoteClient.AgentId);
             SceneObjectGroup group = m_scene.GetGroupByPrim(objectLocalID);
+
             if (sp != null && group != null)
-                DetachSingleAttachmentToInv(sp, group.FromItemID);
+                DetachSingleAttachmentToInv(sp, group);
         }
 
         private void Client_OnDetachAttachmentIntoInv(UUID itemID, IClientAPI remoteClient)
@@ -908,7 +898,21 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
 
             ScenePresence sp = m_scene.GetScenePresence(remoteClient.AgentId);
             if (sp != null)
-                DetachSingleAttachmentToInv(sp, itemID);
+            {
+                lock (sp.AttachmentsSyncLock)
+                {
+                    List<SceneObjectGroup> attachments = sp.GetAttachments();
+    
+                    foreach (SceneObjectGroup group in attachments)
+                    {
+                        if (group.FromItemID == itemID)
+                        {
+                            DetachSingleAttachmentToInv(sp, group);
+                            return;
+                        }
+                    }
+                }
+            }
         }
 
         private void Client_OnObjectDrop(uint soLocalId, IClientAPI remoteClient)
diff --git a/OpenSim/Region/CoreModules/Avatar/Attachments/Tests/AttachmentsModuleTests.cs b/OpenSim/Region/CoreModules/Avatar/Attachments/Tests/AttachmentsModuleTests.cs
index 65722fe..b0c087f 100644
--- a/OpenSim/Region/CoreModules/Avatar/Attachments/Tests/AttachmentsModuleTests.cs
+++ b/OpenSim/Region/CoreModules/Avatar/Attachments/Tests/AttachmentsModuleTests.cs
@@ -227,9 +227,10 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments.Tests
 
             InventoryItemBase attItem = CreateAttachmentItem(scene, ua1.PrincipalID, "att", 0x10, 0x20);
 
-            scene.AttachmentsModule.RezSingleAttachmentFromInventory(
-                sp, attItem.ID, (uint)AttachmentPoint.Chest);
-            scene.AttachmentsModule.DetachSingleAttachmentToInv(sp, attItem.ID);
+            SceneObjectGroup so
+                = (SceneObjectGroup)scene.AttachmentsModule.RezSingleAttachmentFromInventory(
+                    sp, attItem.ID, (uint)AttachmentPoint.Chest);
+            scene.AttachmentsModule.DetachSingleAttachmentToInv(sp, so);
 
             // Check status on scene presence
             Assert.That(sp.HasAttachments(), Is.False);