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

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 (attachments[0].FromItemID != UUID.Zero)
-        
+                            DetachSingleAttachmentToInvInternal(sp, attachments[0]);
-                        if (oldAttachmentItemID != UUID.Zero)
-                            DetachSingleAttachmentToInvInternal(sp, oldAttachmentItemID);
                         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.
+        private void DetachSingleAttachmentToInvInternal(IScenePresence sp, SceneObjectGroup so)
-        // To LocalId or UUID, *THAT* is the question. How now Brown UUID??
-        private void DetachSingleAttachmentToInvInternal(IScenePresence sp, UUID itemID)
         {
             //            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....
+            m_scene.EventManager.TriggerOnAttach(so.LocalId, so.FromItemID, UUID.Zero);
-                return;
+            sp.RemoveAttachment(so);
-
+            m_scene.DeleteSceneObject(so, false);
-            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;
+            so.AttachedAvatar = UUID.Zero;
-                        group.RootPart.SetParentLocalId(0);
+            so.RootPart.SetParentLocalId(0);
-                        group.IsAttachment = false;
+            so.IsAttachment = false;
-                        group.AbsolutePosition = group.RootPart.AttachedPos;
+            so.AbsolutePosition = so.RootPart.AttachedPos;
 
-                        UpdateKnownItem(sp, group, true);
+            UpdateKnownItem(sp, so, true);
-
-                        return;
-                    }
-                }
-            }
         }
 
         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)

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(
+            SceneObjectGroup so
+                = (SceneObjectGroup)scene.AttachmentsModule.RezSingleAttachmentFromInventory(
                     sp, attItem.ID, (uint)AttachmentPoint.Chest);
-            scene.AttachmentsModule.DetachSingleAttachmentToInv(sp, attItem.ID);
+            scene.AttachmentsModule.DetachSingleAttachmentToInv(sp, so);
 
             // Check status on scene presence
             Assert.That(sp.HasAttachments(), Is.False);

OpenSim/Region/Framework/Interfaces/IAttachmentsModule.cs

@@ -109,11 +109,11 @@ namespace OpenSim.Region.Framework.Interfaces
         void DetachSingleAttachmentToGround(IScenePresence sp, uint objectLocalID);
 
         /// <summary>
-        /// Detach the given item so that it remains in the user's inventory.
+        /// Detach the given attachment so that it remains in the user's inventory.
         /// </summary>
         /// <param name="sp">/param>
-        /// <param name="itemID"></param>
+        /// <param name="grp">The attachment to detach.</param>
-        void DetachSingleAttachmentToInv(IScenePresence sp, UUID itemID);
+        void DetachSingleAttachmentToInv(IScenePresence sp, SceneObjectGroup grp);
         
         /// <summary>
         /// Update the position of an attachment.

OpenSim/Region/Framework/Interfaces/IScenePresence.cs

@@ -72,6 +72,10 @@ namespace OpenSim.Region.Framework.Interfaces
         /// <returns></returns>
         List<SceneObjectGroup> GetAttachments(uint attachmentPoint);
 
+        /// <summary>
+        /// Does this avatar have any attachments?
+        /// </summary>
+        /// <returns></returns>
         bool HasAttachments();
 
         // Don't use these methods directly.  Instead, use the AttachmentsModule

OpenSim/Region/ScriptEngine/Shared/Api/Implementation/LSL_Api.cs

@@ -2949,16 +2949,13 @@ namespace OpenSim.Region.ScriptEngine.Shared.Api
         }
 
         private void DetachWrapper(object o)
+        {
+            if (World.AttachmentsModule != null)
             {
                 SceneObjectPart host = (SceneObjectPart)o;
-
-            SceneObjectGroup grp = host.ParentGroup;
-            UUID itemID = grp.FromItemID;
                 ScenePresence presence = World.GetScenePresence(host.OwnerID);
-
+                World.AttachmentsModule.DetachSingleAttachmentToInv(presence, host.ParentGroup);
-            IAttachmentsModule attachmentsModule = m_ScriptEngine.World.AttachmentsModule;
+            }
-            if (attachmentsModule != null)
-                attachmentsModule.DetachSingleAttachmentToInv(presence, itemID);
         }
 
         public void llAttachToAvatar(int attachmentPoint)