From b903d2ca963d5f68803a1cabbd033f89ab72634a Mon Sep 17 00:00:00 2001 From: "Justin Clark-Casey (justincc)" Date: Tue, 6 Sep 2011 01:59:21 +0100 Subject: [PATCH] In SetAttachment, if the existing attachment has no asset id then carry on rather than abort. When a user logs in, the attachment item ids are pulled from persistence in the Avatars table. However, the asset ids are not saved. When the avatar enters a simulator the attachments are set again. If we simply perform an item check then the asset ids (which are now present) are never set, and NPC attachments later fail unless the attachment is detached and reattached. Hopefully resolves part of http://opensimulator.org/mantis/view.php?id=5653 --- OpenSim/Framework/AvatarAppearance.cs | 36 +++++++++++++++++-- OpenSim/Framework/ChildAgentDataUpdate.cs | 1 + .../Tests/AttachmentsModuleTests.cs | 8 +++++ OpenSim/Services/Interfaces/IAvatarService.cs | 4 +-- 4 files changed, 43 insertions(+), 6 deletions(-) diff --git a/OpenSim/Framework/AvatarAppearance.cs b/OpenSim/Framework/AvatarAppearance.cs index 0b0afeb9bf..cce44b02b8 100644 --- a/OpenSim/Framework/AvatarAppearance.cs +++ b/OpenSim/Framework/AvatarAppearance.cs @@ -414,12 +414,16 @@ namespace OpenSim.Framework internal void ReplaceAttachment(AvatarAttachment attach) { +// m_log.DebugFormat( +// "[AVATAR APPEARANCE]: Replacing itemID={0}, assetID={1} at {2}", +// attach.ItemID, attach.AssetID, attach.AttachPoint); + m_attachments[attach.AttachPoint] = new List(); m_attachments[attach.AttachPoint].Add(attach); } /// - /// Add an attachment + /// Set an attachment /// /// /// If the attachpoint has the @@ -449,11 +453,20 @@ namespace OpenSim.Framework m_attachments.Remove(attachpoint); return true; } + return false; } - // check if the item is already attached at this point - if (GetAttachpoint(item) == (attachpoint & 0x7F)) + // When a user logs in, the attachment item ids are pulled from persistence in the Avatars table. However, + // the asset ids are not saved. When the avatar enters a simulator the attachments are set again. If + // we simply perform an item check here then the asset ids (which are now present) are never set, and NPC attachments + // later fail unless the attachment is detached and reattached. + // + // Therefore, we will carry on with the set if the existing attachment has no asset id. + AvatarAttachment existingAttachment = GetAttachmentForItem(item); + if (existingAttachment != null + && existingAttachment.AssetID != UUID.Zero + && existingAttachment.AttachPoint == (attachpoint & 0x7F)) { // m_log.DebugFormat("[AVATAR APPEARANCE] attempt to attach an already attached item {0}",item); return false; @@ -474,6 +487,23 @@ namespace OpenSim.Framework return true; } + /// + /// If the item is already attached, return it. + /// + /// + /// Returns null if this item is not attached. + public AvatarAttachment GetAttachmentForItem(UUID itemID) + { + foreach (KeyValuePair> kvp in m_attachments) + { + int index = kvp.Value.FindIndex(delegate(AvatarAttachment a) { return a.ItemID == itemID; }); + if (index >= 0) + return kvp.Value[index]; + } + + return null; + } + public int GetAttachpoint(UUID itemID) { foreach (KeyValuePair> kvp in m_attachments) diff --git a/OpenSim/Framework/ChildAgentDataUpdate.cs b/OpenSim/Framework/ChildAgentDataUpdate.cs index 613db1c55a..53ec166b4e 100644 --- a/OpenSim/Framework/ChildAgentDataUpdate.cs +++ b/OpenSim/Framework/ChildAgentDataUpdate.cs @@ -628,6 +628,7 @@ namespace OpenSim.Framework // We know all of these must end up as attachments so we // append rather than replace to ensure multiple attachments // per point continues to work +// m_log.DebugFormat("[CHILDAGENTDATAUPDATE]: Appending attachments for {0}", AgentID); Appearance.AppendAttachment(new AvatarAttachment((OSDMap)o)); } } diff --git a/OpenSim/Region/CoreModules/Avatar/Attachments/Tests/AttachmentsModuleTests.cs b/OpenSim/Region/CoreModules/Avatar/Attachments/Tests/AttachmentsModuleTests.cs index 3f8c01fc1c..0fa2e00e2b 100644 --- a/OpenSim/Region/CoreModules/Avatar/Attachments/Tests/AttachmentsModuleTests.cs +++ b/OpenSim/Region/CoreModules/Avatar/Attachments/Tests/AttachmentsModuleTests.cs @@ -247,6 +247,14 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments.Tests Assert.That(attSo.IsAttachment); Assert.That(attSo.UsesPhysics, Is.False); Assert.That(attSo.IsTemporary, Is.False); + + // Check appearance status + List retreivedAttachments = presence.Appearance.GetAttachments(); + Assert.That(retreivedAttachments.Count, Is.EqualTo(1)); + Assert.That(retreivedAttachments[0].AttachPoint, Is.EqualTo((int)AttachmentPoint.Chest)); + Assert.That(retreivedAttachments[0].ItemID, Is.EqualTo(attItemId)); + Assert.That(retreivedAttachments[0].AssetID, Is.EqualTo(attAssetId)); + Assert.That(presence.Appearance.GetAttachpoint(attItemId), Is.EqualTo((int)AttachmentPoint.Chest)); } // I'm commenting this test because scene setup NEEDS InventoryService to diff --git a/OpenSim/Services/Interfaces/IAvatarService.cs b/OpenSim/Services/Interfaces/IAvatarService.cs index 0d5ab7db2e..cda7113a4f 100644 --- a/OpenSim/Services/Interfaces/IAvatarService.cs +++ b/OpenSim/Services/Interfaces/IAvatarService.cs @@ -262,7 +262,6 @@ namespace OpenSim.Services.Interfaces UUID.Parse(Data["SkirtItem"]), UUID.Parse(Data["SkirtAsset"])); - if (Data.ContainsKey("VisualParams")) { string[] vps = Data["VisualParams"].Split(new char[] {','}); @@ -291,7 +290,6 @@ namespace OpenSim.Services.Interfaces } } - // Attachments Dictionary attchs = new Dictionary(); foreach (KeyValuePair _kvp in Data) @@ -308,7 +306,7 @@ namespace OpenSim.Services.Interfaces UUID uuid = UUID.Zero; UUID.TryParse(_kvp.Value, out uuid); - appearance.SetAttachment(point,uuid,UUID.Zero); + appearance.SetAttachment(point, uuid, UUID.Zero); } if (appearance.Wearables[AvatarWearable.BODY].Count == 0)