From 364816421985c052521cf7b444e124760c0a1025 Mon Sep 17 00:00:00 2001
From: Justin Clark-Casey (justincc)
Date: Tue, 19 Mar 2013 21:44:18 +0000
Subject: Prevent multiple instances of the same item ID being appended to an
AvatarAppearance
It looks like this was happening when AttachmentsModule.RezAttachments was doing a secondary set of each attachment to update with the asset ID (initially they only have the inventory ID).
However, with multi-attach this was appending a second copy of the same attachment rather than updating the data that was already there.
This commit requires both simulator and service to be updated.
---
OpenSim/Framework/AvatarAppearance.cs | 78 +++++++++++++---------
.../Avatar/Attachments/AttachmentsModule.cs | 4 +-
.../Avatar/AvatarFactory/AvatarFactoryModule.cs | 12 +++-
3 files changed, 60 insertions(+), 34 deletions(-)
(limited to 'OpenSim')
diff --git a/OpenSim/Framework/AvatarAppearance.cs b/OpenSim/Framework/AvatarAppearance.cs
index 95e9667..494ae5e 100644
--- a/OpenSim/Framework/AvatarAppearance.cs
+++ b/OpenSim/Framework/AvatarAppearance.cs
@@ -459,45 +459,59 @@ namespace OpenSim.Framework
if (attachpoint == 0)
return false;
- if (item == UUID.Zero)
+ lock (m_attachments)
{
- lock (m_attachments)
+ if (item == UUID.Zero)
{
if (m_attachments.ContainsKey(attachpoint))
{
m_attachments.Remove(attachpoint);
return true;
}
+
+ return false;
}
-
- return false;
- }
- // 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;
- }
-
- // check if this is an append or a replace, 0x80 marks it as an append
- if ((attachpoint & 0x80) > 0)
- {
- // strip the append bit
- int point = attachpoint & 0x7F;
- AppendAttachment(new AvatarAttachment(point, item, asset));
- }
- else
- {
- ReplaceAttachment(new AvatarAttachment(attachpoint,item, asset));
+ // 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)
+ {
+// m_log.DebugFormat(
+// "[AVATAR APPEARANCE]: Found existing attachment for {0}, asset {1} at point {2}",
+// existingAttachment.ItemID, existingAttachment.AssetID, existingAttachment.AttachPoint);
+
+ if (existingAttachment.AssetID != UUID.Zero && existingAttachment.AttachPoint == (attachpoint & 0x7F))
+ {
+ m_log.DebugFormat(
+ "[AVATAR APPEARANCE]: Ignoring attempt to attach an already attached item {0} at point {1}",
+ item, attachpoint);
+
+ return false;
+ }
+ else
+ {
+ // Remove it here so that the later append does not add a second attachment but we still update
+ // the assetID
+ DetachAttachment(existingAttachment.ItemID);
+ }
+ }
+
+ // check if this is an append or a replace, 0x80 marks it as an append
+ if ((attachpoint & 0x80) > 0)
+ {
+ // strip the append bit
+ int point = attachpoint & 0x7F;
+ AppendAttachment(new AvatarAttachment(point, item, asset));
+ }
+ else
+ {
+ ReplaceAttachment(new AvatarAttachment(attachpoint,item, asset));
+ }
}
return true;
@@ -547,6 +561,10 @@ namespace OpenSim.Framework
int index = kvp.Value.FindIndex(delegate(AvatarAttachment a) { return a.ItemID == itemID; });
if (index >= 0)
{
+// m_log.DebugFormat(
+// "[AVATAR APPEARANCE]: Detaching attachment {0}, index {1}, point {2}",
+// m_attachments[kvp.Key][index].ItemID, index, m_attachments[kvp.Key][index].AttachPoint);
+
// Remove it from the list of attachments at that attach point
m_attachments[kvp.Key].RemoveAt(index);
diff --git a/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs b/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs
index 72ba3cf..ad17aa9 100644
--- a/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs
+++ b/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs
@@ -547,8 +547,8 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
}
// m_log.DebugFormat(
-// "[ATTACHMENTS MODULE]: Detaching object {0} {1} for {2} in {3}",
-// so.Name, so.LocalId, sp.Name, m_scene.Name);
+// "[ATTACHMENTS MODULE]: Detaching object {0} {1} (FromItemID {2}) for {3} in {4}",
+// so.Name, so.LocalId, so.FromItemID, sp.Name, m_scene.Name);
// Scripts MUST be snapshotted before the object is
// removed from the scene because doing otherwise will
diff --git a/OpenSim/Region/CoreModules/Avatar/AvatarFactory/AvatarFactoryModule.cs b/OpenSim/Region/CoreModules/Avatar/AvatarFactory/AvatarFactoryModule.cs
index 00d1fd8..ff5bf9f 100644
--- a/OpenSim/Region/CoreModules/Avatar/AvatarFactory/AvatarFactoryModule.cs
+++ b/OpenSim/Region/CoreModules/Avatar/AvatarFactory/AvatarFactoryModule.cs
@@ -326,7 +326,7 @@ namespace OpenSim.Region.CoreModules.Avatar.AvatarFactory
public void QueueAppearanceSave(UUID agentid)
{
- // m_log.WarnFormat("[AVFACTORY]: Queue appearance save for {0}", agentid);
+// m_log.DebugFormat("[AVFACTORY]: Queueing appearance save for {0}", agentid);
// 10000 ticks per millisecond, 1000 milliseconds per second
long timestamp = DateTime.Now.Ticks + Convert.ToInt64(m_savetime * 1000 * 10000);
@@ -529,7 +529,7 @@ namespace OpenSim.Region.CoreModules.Avatar.AvatarFactory
return;
}
- // m_log.WarnFormat("[AVFACTORY] avatar {0} save appearance",agentid);
+// m_log.DebugFormat("[AVFACTORY]: Saving appearance for avatar {0}", agentid);
// This could take awhile since it needs to pull inventory
// We need to do it at the point of save so that there is a sufficient delay for any upload of new body part/shape
@@ -538,6 +538,14 @@ namespace OpenSim.Region.CoreModules.Avatar.AvatarFactory
// multiple save requests.
SetAppearanceAssets(sp.UUID, sp.Appearance);
+// List<AvatarAttachment> attachments = sp.Appearance.GetAttachments();
+// foreach (AvatarAttachment att in attachments)
+// {
+// m_log.DebugFormat(
+// "[AVFACTORY]: For {0} saving attachment {1} at point {2}",
+// sp.Name, att.ItemID, att.AttachPoint);
+// }
+
m_scene.AvatarService.SetAppearance(agentid, sp.Appearance);
// Trigger this here because it's the final step in the set/queue/save process for appearance setting.
--
cgit v1.1