From 663b0cc6810690444c03048529be11aaf9988347 Mon Sep 17 00:00:00 2001 From: "Justin Clark-Casey (justincc)" Date: Thu, 28 Jun 2012 23:31:23 +0100 Subject: [PATCH] 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 --- .../Avatar/Attachments/AttachmentsModule.cs | 78 ++++++++++--------- .../Tests/AttachmentsModuleTests.cs | 7 +- .../Interfaces/IAttachmentsModule.cs | 6 +- .../Framework/Interfaces/IScenePresence.cs | 4 + .../Shared/Api/Implementation/LSL_Api.cs | 15 ++-- 5 files changed, 58 insertions(+), 52 deletions(-) diff --git a/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs b/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs index 5b1235d087..6e9ba23c21 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; + m_scene.EventManager.TriggerOnAttach(so.LocalId, so.FromItemID, UUID.Zero); + sp.RemoveAttachment(so); + m_scene.DeleteSceneObject(so, false); - lock (sp.AttachmentsSyncLock) - { - List attachments = sp.GetAttachments(); + // Prepare sog for storage + so.AttachedAvatar = UUID.Zero; + so.RootPart.SetParentLocalId(0); + so.IsAttachment = false; + so.AbsolutePosition = so.RootPart.AttachedPos; - 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; - - UpdateKnownItem(sp, group, true); - - 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 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 65722fe379..b0c087fb56 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); diff --git a/OpenSim/Region/Framework/Interfaces/IAttachmentsModule.cs b/OpenSim/Region/Framework/Interfaces/IAttachmentsModule.cs index 375d3345a2..ba35a41046 100644 --- a/OpenSim/Region/Framework/Interfaces/IAttachmentsModule.cs +++ b/OpenSim/Region/Framework/Interfaces/IAttachmentsModule.cs @@ -109,11 +109,11 @@ namespace OpenSim.Region.Framework.Interfaces void DetachSingleAttachmentToGround(IScenePresence sp, uint objectLocalID); /// - /// 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. /// /// /param> - /// - void DetachSingleAttachmentToInv(IScenePresence sp, UUID itemID); + /// The attachment to detach. + void DetachSingleAttachmentToInv(IScenePresence sp, SceneObjectGroup grp); /// /// Update the position of an attachment. diff --git a/OpenSim/Region/Framework/Interfaces/IScenePresence.cs b/OpenSim/Region/Framework/Interfaces/IScenePresence.cs index 19a8236427..e6b926ce8c 100644 --- a/OpenSim/Region/Framework/Interfaces/IScenePresence.cs +++ b/OpenSim/Region/Framework/Interfaces/IScenePresence.cs @@ -72,6 +72,10 @@ namespace OpenSim.Region.Framework.Interfaces /// List GetAttachments(uint attachmentPoint); + /// + /// Does this avatar have any attachments? + /// + /// bool HasAttachments(); // Don't use these methods directly. Instead, use the AttachmentsModule diff --git a/OpenSim/Region/ScriptEngine/Shared/Api/Implementation/LSL_Api.cs b/OpenSim/Region/ScriptEngine/Shared/Api/Implementation/LSL_Api.cs index 189cf7618d..ecd4be7e14 100644 --- a/OpenSim/Region/ScriptEngine/Shared/Api/Implementation/LSL_Api.cs +++ b/OpenSim/Region/ScriptEngine/Shared/Api/Implementation/LSL_Api.cs @@ -2950,15 +2950,12 @@ namespace OpenSim.Region.ScriptEngine.Shared.Api private void DetachWrapper(object o) { - SceneObjectPart host = (SceneObjectPart)o; - - SceneObjectGroup grp = host.ParentGroup; - UUID itemID = grp.FromItemID; - ScenePresence presence = World.GetScenePresence(host.OwnerID); - - IAttachmentsModule attachmentsModule = m_ScriptEngine.World.AttachmentsModule; - if (attachmentsModule != null) - attachmentsModule.DetachSingleAttachmentToInv(presence, itemID); + if (World.AttachmentsModule != null) + { + SceneObjectPart host = (SceneObjectPart)o; + ScenePresence presence = World.GetScenePresence(host.OwnerID); + World.AttachmentsModule.DetachSingleAttachmentToInv(presence, host.ParentGroup); + } } public void llAttachToAvatar(int attachmentPoint)