From f263d6a910f5c382756da04dd423db4a46b1eeb7 Mon Sep 17 00:00:00 2001 From: Justin Clark-Casey (justincc) Date: Thu, 28 Jun 2012 22:48:49 +0100 Subject: Remove code that tried to delete an attachment back to inventory if RezSingleAttachmentFromInventoryInternal() returned null. null would only ever be returned if the item couldn't be located within inventory and this would happen immediately. In this case, derezzing wouldn't work anyway since there is no item to derez. --- OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) (limited to 'OpenSim/Region/CoreModules') diff --git a/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs b/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs index 4a7fbce..e3ee2fc 100644 --- a/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs +++ b/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs @@ -365,12 +365,7 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments return null; } - SceneObjectGroup att = RezSingleAttachmentFromInventoryInternal(sp, itemID, UUID.Zero, AttachmentPt); - - if (att == null) - DetachSingleAttachmentToInv(sp, itemID); - - return att; + return RezSingleAttachmentFromInventoryInternal(sp, itemID, UUID.Zero, AttachmentPt); } public void RezMultipleAttachmentsFromInventory(IScenePresence sp, List> rezlist) -- cgit v1.1 From 571fd966cbcb4e718703bc194384f6bf84c211df Mon Sep 17 00:00:00 2001 From: Justin Clark-Casey (justincc) Date: Thu, 28 Jun 2012 23:01:12 +0100 Subject: Rather than iterating through all SOGs in the scene looking for the one that matches out fromItemID on detach, go through the agent's attachment sog list instead. --- .../Avatar/Attachments/AttachmentsModule.cs | 41 +++++++++------------- 1 file changed, 17 insertions(+), 24 deletions(-) (limited to 'OpenSim/Region/CoreModules') diff --git a/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs b/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs index e3ee2fc..e9f0488 100644 --- a/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs +++ b/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs @@ -666,34 +666,27 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments if (itemID == UUID.Zero) // If this happened, someone made a mistake.... return; - // We can NOT use the dictionries here, as we are looking - // for an entity by the fromAssetID, which is NOT the prim UUID - EntityBase[] detachEntities = m_scene.GetEntities(); - SceneObjectGroup group; - lock (sp.AttachmentsSyncLock) { - foreach (EntityBase entity in detachEntities) + List attachments = sp.GetAttachments(); + + foreach (SceneObjectGroup group in attachments) { - if (entity is SceneObjectGroup) + if (group.FromItemID == itemID) { - group = (SceneObjectGroup)entity; - 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; - } + 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; } } } -- cgit v1.1 From bfa6896678872a4e796ec4de22e83b6cead3ba17 Mon Sep 17 00:00:00 2001 From: Justin Clark-Casey (justincc) Date: Thu, 28 Jun 2012 23:31:23 +0100 Subject: 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 ++++++++++++---------- .../Attachments/Tests/AttachmentsModuleTests.cs | 7 +- 2 files changed, 45 insertions(+), 40 deletions(-) (limited to 'OpenSim/Region/CoreModules') 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 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 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); -- cgit v1.1