Merge branch 'master' into 0.7.2-post-fixes

5 files changed, 331 insertions, 215 deletions

diff --git a/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs b/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs
index 996e2ab..b965d75 100644
--- a/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs
+++ b/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs
@@ -143,20 +143,26 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
 
         public void SaveChangedAttachments(IScenePresence sp)
         {
+//            m_log.DebugFormat("[ATTACHMENTS MODULE]: Saving changed attachments for {0}", sp.Name);
+
             foreach (SceneObjectGroup grp in sp.GetAttachments())
             {
-                if (grp.HasGroupChanged) // Resizer scripts?
-                {
+//                if (grp.HasGroupChanged) // Resizer scripts?
+//                {
                     grp.IsAttachment = false;
                     grp.AbsolutePosition = grp.RootPart.AttachedPos;
-                    UpdateKnownItem(sp.ControllingClient, grp, grp.GetFromItemID(), grp.OwnerID);
+                    UpdateKnownItem(sp.ControllingClient, grp);
                     grp.IsAttachment = true;
-                }
+//                }
             }
         }
 
         public void DeleteAttachmentsFromScene(IScenePresence sp, bool silent)
         {
+//            m_log.DebugFormat(
+//                "[ATTACHMENTS MODULE]: Deleting attachments from scene {0} for {1}, silent = {2}",
+//                m_scene.RegionInfo.RegionName, sp.Name, silent);
+
             foreach (SceneObjectGroup sop in sp.GetAttachments())
             {
                 sop.Scene.DeleteSceneObject(sop, silent);
@@ -212,7 +218,7 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
                     m_scene.EventManager.TriggerOnAttach(objectLocalID, part.ParentGroup.GetFromItemID(), remoteClient.AgentId);
 
                     // Save avatar attachment information
-                    m_log.Info(
+                    m_log.Debug(
                         "[ATTACHMENTS MODULE]: Saving avatar attachment. AgentID: " + remoteClient.AgentId
                         + ", AttachmentPoint: " + AttachmentPt);
 
@@ -240,80 +246,83 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
         
         private bool AttachObject(IScenePresence sp, SceneObjectGroup group, uint attachmentPt, bool silent)
         {
-//            m_log.DebugFormat(
-//                "[ATTACHMENTS MODULE]: Attaching object {0} {1} to {2} point {3} from ground (silent = {4})",
-//                group.Name, group.LocalId, sp.Name, attachmentPt, silent);
-
-            if (sp.GetAttachments(attachmentPt).Contains(group))
-            {
-//                m_log.WarnFormat(
-//                    "[ATTACHMENTS MODULE]: Ignoring request to attach {0} {1} to {2} on {3} since it's already attached",
-//                    group.Name, group.LocalId, sp.Name, AttachmentPt);
-
-                return false;
-            }
-
-            Vector3 attachPos = group.AbsolutePosition;
-
-            // TODO: this short circuits multiple attachments functionality  in  LL viewer 2.1+ and should
-            // be removed when that functionality is implemented in opensim
-            attachmentPt &= 0x7f;
-            
-            // If the attachment point isn't the same as the one previously used
-            // set it's offset position = 0 so that it appears on the attachment point
-            // and not in a weird location somewhere unknown.
-            if (attachmentPt != 0 && attachmentPt != group.AttachmentPoint)
-            {
-                attachPos = Vector3.Zero;
-            }
-
-            // AttachmentPt 0 means the client chose to 'wear' the attachment.
-            if (attachmentPt == 0)
-            {
-                // Check object for stored attachment point
-                attachmentPt = group.AttachmentPoint;
-            }
-
-            // if we still didn't find a suitable attachment point.......
-            if (attachmentPt == 0)
-            {
-                // Stick it on left hand with Zero Offset from the attachment point.
-                attachmentPt = (uint)AttachmentPoint.LeftHand;
-                attachPos = Vector3.Zero;
-            }
-
-            group.AttachmentPoint = attachmentPt;
-            group.AbsolutePosition = attachPos;
-
-            // We also don't want to do any of the inventory operations for an NPC.
-            if (sp.PresenceType != PresenceType.Npc)
+            lock (sp.AttachmentsSyncLock)
             {
-                // Remove any previous attachments
-                List<SceneObjectGroup> attachments = sp.GetAttachments(attachmentPt);
-
-                // At the moment we can only deal with a single attachment
-                if (attachments.Count != 0)
+//                m_log.DebugFormat(
+//                    "[ATTACHMENTS MODULE]: Attaching object {0} {1} to {2} point {3} from ground (silent = {4})",
+//                    group.Name, group.LocalId, sp.Name, attachmentPt, silent);
+    
+                if (sp.GetAttachments(attachmentPt).Contains(group))
                 {
-                    UUID oldAttachmentItemID = attachments[0].GetFromItemID();
+    //                m_log.WarnFormat(
+    //                    "[ATTACHMENTS MODULE]: Ignoring request to attach {0} {1} to {2} on {3} since it's already attached",
+    //                    group.Name, group.LocalId, sp.Name, AttachmentPt);
     
-                    if (oldAttachmentItemID != UUID.Zero)
-                        DetachSingleAttachmentToInv(oldAttachmentItemID, sp);
-                    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!",
-                            attachments[0].Name, attachments[0].LocalId, attachmentPt, group.Name, group.LocalId, sp.Name);
+                    return false;
                 }
-
-                // Add the new attachment to inventory if we don't already have it.
-                UUID newAttachmentItemID = group.GetFromItemID();
-                if (newAttachmentItemID == UUID.Zero)
-                    newAttachmentItemID = AddSceneObjectAsNewAttachmentInInv(sp.ControllingClient, group).ID;
     
-                ShowAttachInUserInventory(sp, attachmentPt, newAttachmentItemID, group);
+                Vector3 attachPos = group.AbsolutePosition;
+    
+                // TODO: this short circuits multiple attachments functionality  in  LL viewer 2.1+ and should
+                // be removed when that functionality is implemented in opensim
+                attachmentPt &= 0x7f;
+                
+                // If the attachment point isn't the same as the one previously used
+                // set it's offset position = 0 so that it appears on the attachment point
+                // and not in a weird location somewhere unknown.
+                if (attachmentPt != 0 && attachmentPt != group.AttachmentPoint)
+                {
+                    attachPos = Vector3.Zero;
+                }
+    
+                // AttachmentPt 0 means the client chose to 'wear' the attachment.
+                if (attachmentPt == 0)
+                {
+                    // Check object for stored attachment point
+                    attachmentPt = group.AttachmentPoint;
+                }
+    
+                // if we still didn't find a suitable attachment point.......
+                if (attachmentPt == 0)
+                {
+                    // Stick it on left hand with Zero Offset from the attachment point.
+                    attachmentPt = (uint)AttachmentPoint.LeftHand;
+                    attachPos = Vector3.Zero;
+                }
+    
+                group.AttachmentPoint = attachmentPt;
+                group.AbsolutePosition = attachPos;
+    
+                // We also don't want to do any of the inventory operations for an NPC.
+                if (sp.PresenceType != PresenceType.Npc)
+                {
+                    // Remove any previous attachments
+                    List<SceneObjectGroup> attachments = sp.GetAttachments(attachmentPt);
+    
+                    // At the moment we can only deal with a single attachment
+                    if (attachments.Count != 0)
+                    {
+                        UUID oldAttachmentItemID = attachments[0].GetFromItemID();
+        
+                        if (oldAttachmentItemID != UUID.Zero)
+                            DetachSingleAttachmentToInv(oldAttachmentItemID, sp);
+                        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!",
+                                attachments[0].Name, attachments[0].LocalId, attachmentPt, group.Name, group.LocalId, sp.Name);
+                    }
+    
+                    // Add the new attachment to inventory if we don't already have it.
+                    UUID newAttachmentItemID = group.GetFromItemID();
+                    if (newAttachmentItemID == UUID.Zero)
+                        newAttachmentItemID = AddSceneObjectAsNewAttachmentInInv(sp.ControllingClient, group).ID;
+        
+                    ShowAttachInUserInventory(sp, attachmentPt, newAttachmentItemID, group);
+                }
+    
+                AttachToAgent(sp, group, attachmentPt, attachPos, silent);
             }
 
-            AttachToAgent(sp, group, attachmentPt, attachPos, silent);
-
             return true;
         }
 
@@ -322,17 +331,31 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
             RezMultipleAttachmentsFromInvPacket.HeaderDataBlock header,
             RezMultipleAttachmentsFromInvPacket.ObjectDataBlock[] objects)
         {
-            foreach (RezMultipleAttachmentsFromInvPacket.ObjectDataBlock obj in objects)
+            ScenePresence sp = m_scene.GetScenePresence(remoteClient.AgentId);
+
+            if (sp == null)
             {
-                RezSingleAttachmentFromInventory(remoteClient, obj.ItemID, obj.AttachmentPt);
+                m_log.ErrorFormat(
+                    "[ATTACHMENTS MODULE]: Could not find presence for client {0} {1} in RezMultipleAttachmentsFromInventory()",
+                    remoteClient.Name, remoteClient.AgentId);
+                return;
+            }
+
+            lock (sp.AttachmentsSyncLock)
+            {
+//                m_log.DebugFormat("[ATTACHMENTS MODULE]: Rezzing multiple attachments from inventory for {0}", sp.Name);
+
+                foreach (RezMultipleAttachmentsFromInvPacket.ObjectDataBlock obj in objects)
+                {
+                    RezSingleAttachmentFromInventory(sp, obj.ItemID, obj.AttachmentPt);
+                }
             }
         }
         
-        public ISceneEntity RezSingleAttachmentFromInventory(
-            IClientAPI remoteClient, UUID itemID, uint AttachmentPt)
+        public ISceneEntity RezSingleAttachmentFromInventory(IClientAPI remoteClient, UUID itemID, uint AttachmentPt)
         {
 //            m_log.DebugFormat(
-//                "[ATTACHMENTS MODULE]: Rezzing attachment to point {0} from item {1} for {2}", 
+//                "[ATTACHMENTS MODULE]: Rezzing attachment to point {0} from item {1} for {2}",
 //                (AttachmentPoint)AttachmentPt, itemID, remoteClient.Name);
 
             ScenePresence sp = m_scene.GetScenePresence(remoteClient.AgentId);
@@ -344,11 +367,46 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
                     remoteClient.Name, remoteClient.AgentId);
                 return null;
             }
-            
+
+            return RezSingleAttachmentFromInventory(sp, itemID, AttachmentPt);
+        }
+
+        public ISceneEntity RezSingleAttachmentFromInventory(ScenePresence sp, UUID itemID, uint AttachmentPt)
+        {
+//            m_log.DebugFormat(
+//                "[ATTACHMENTS MODULE]: RezSingleAttachmentFromInventory to point {0} from item {1} for {2}",
+//                (AttachmentPoint)AttachmentPt, itemID, sp.Name);
+
             // TODO: this short circuits multiple attachments functionality  in  LL viewer 2.1+ and should
             // be removed when that functionality is implemented in opensim
             AttachmentPt &= 0x7f;
 
+            // Viewer 2/3 sometimes asks to re-wear items that are already worn (and show up in it's inventory as such).
+            // This often happens during login - not sure the exact reason.
+            // For now, we will ignore the request.  Unfortunately, this means that we need to dig through all the
+            // ScenePresence attachments.  We can't use the data in AvatarAppearance because that's present at login
+            // before anything has actually been attached.
+            bool alreadyOn = false;
+            List<SceneObjectGroup> existingAttachments = sp.GetAttachments();
+            foreach (SceneObjectGroup so in existingAttachments)
+            {
+                if (so.GetFromItemID() == itemID)
+                {
+                    alreadyOn = true;
+                    break;
+                }
+            }
+
+//            if (sp.Appearance.GetAttachmentForItem(itemID) != null)
+            if (alreadyOn)
+            {
+//                m_log.WarnFormat(
+//                    "[ATTACHMENTS MODULE]: Ignoring request by {0} to wear item {1} at {2} since it is already worn",
+//                    sp.Name, itemID, AttachmentPt);
+
+                return null;
+            }
+
             SceneObjectGroup att = RezSingleAttachmentFromInventoryInternal(sp, itemID, UUID.Zero, AttachmentPt);
 
             if (att == null)
@@ -363,65 +421,68 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
             IInventoryAccessModule invAccess = m_scene.RequestModuleInterface<IInventoryAccessModule>();
             if (invAccess != null)
             {
-                SceneObjectGroup objatt;
-
-                if (itemID != UUID.Zero)
-                    objatt = invAccess.RezObject(sp.ControllingClient,
-                        itemID, Vector3.Zero, Vector3.Zero, UUID.Zero, (byte)1, true,
-                        false, false, sp.UUID, true);
-                else
-                    objatt = invAccess.RezObject(sp.ControllingClient,
-                        null, assetID, Vector3.Zero, Vector3.Zero, UUID.Zero, (byte)1, true,
-                        false, false, sp.UUID, true);
-
-//                m_log.DebugFormat(
-//                    "[ATTACHMENTS MODULE]: Retrieved single object {0} for attachment to {1} on point {2}",
-//                    objatt.Name, remoteClient.Name, AttachmentPt);
-                
-                if (objatt != null)
+                lock (sp.AttachmentsSyncLock)
                 {
-                    // HasGroupChanged is being set from within RezObject.  Ideally it would be set by the caller.
-                    objatt.HasGroupChanged = false;
-                    bool tainted = false;
-                    if (attachmentPt != 0 && attachmentPt != objatt.AttachmentPoint)
-                        tainted = true;
-
-                    // This will throw if the attachment fails
-                    try
+                    SceneObjectGroup objatt;
+    
+                    if (itemID != UUID.Zero)
+                        objatt = invAccess.RezObject(sp.ControllingClient,
+                            itemID, Vector3.Zero, Vector3.Zero, UUID.Zero, (byte)1, true,
+                            false, false, sp.UUID, true);
+                    else
+                        objatt = invAccess.RezObject(sp.ControllingClient,
+                            null, assetID, Vector3.Zero, Vector3.Zero, UUID.Zero, (byte)1, true,
+                            false, false, sp.UUID, true);
+    
+    //                m_log.DebugFormat(
+    //                    "[ATTACHMENTS MODULE]: Retrieved single object {0} for attachment to {1} on point {2}",
+    //                    objatt.Name, remoteClient.Name, AttachmentPt);
+                    
+                    if (objatt != null)
                     {
-                        AttachObject(sp, objatt, attachmentPt, false);
+                        // HasGroupChanged is being set from within RezObject.  Ideally it would be set by the caller.
+                        objatt.HasGroupChanged = false;
+                        bool tainted = false;
+                        if (attachmentPt != 0 && attachmentPt != objatt.AttachmentPoint)
+                            tainted = true;
+    
+                        // This will throw if the attachment fails
+                        try
+                        {
+                            AttachObject(sp, objatt, attachmentPt, false);
+                        }
+                        catch (Exception e)
+                        {
+                            m_log.ErrorFormat(
+                                "[ATTACHMENTS MODULE]: Failed to attach {0} {1} for {2}, exception {3}{4}",
+                                objatt.Name, objatt.UUID, sp.Name, e.Message, e.StackTrace);
+    
+                            // Make sure the object doesn't stick around and bail
+                            sp.RemoveAttachment(objatt);
+                            m_scene.DeleteSceneObject(objatt, false);
+                            return null;
+                        }
+                        
+                        if (tainted)
+                            objatt.HasGroupChanged = true;
+    
+                        // Fire after attach, so we don't get messy perms dialogs
+                        // 4 == AttachedRez
+                        objatt.CreateScriptInstances(0, true, m_scene.DefaultScriptEngine, 4);
+                        objatt.ResumeScripts();
+    
+                        // Do this last so that event listeners have access to all the effects of the attachment
+                        m_scene.EventManager.TriggerOnAttach(objatt.LocalId, itemID, sp.UUID);
+
+                        return objatt;
                     }
-                    catch (Exception e)
+                    else
                     {
-                        m_log.ErrorFormat(
-                            "[ATTACHMENTS MODULE]: Failed to attach {0} {1} for {2}, exception {3}{4}",
-                            objatt.Name, objatt.UUID, sp.Name, e.Message, e.StackTrace);
-
-                        // Make sure the object doesn't stick around and bail
-                        sp.RemoveAttachment(objatt);
-                        m_scene.DeleteSceneObject(objatt, false);
-                        return null;
+                        m_log.WarnFormat(
+                            "[ATTACHMENTS MODULE]: Could not retrieve item {0} for attaching to avatar {1} at point {2}",
+                            itemID, sp.Name, attachmentPt);
                     }
-                    
-                    if (tainted)
-                        objatt.HasGroupChanged = true;
-
-                    // Fire after attach, so we don't get messy perms dialogs
-                    // 4 == AttachedRez
-                    objatt.CreateScriptInstances(0, true, m_scene.DefaultScriptEngine, 4);
-                    objatt.ResumeScripts();
-
-                    // Do this last so that event listeners have access to all the effects of the attachment
-                    m_scene.EventManager.TriggerOnAttach(objatt.LocalId, itemID, sp.UUID);
-                }
-                else
-                {
-                    m_log.WarnFormat(
-                        "[ATTACHMENTS MODULE]: Could not retrieve item {0} for attaching to avatar {1} at point {2}", 
-                        itemID, sp.Name, attachmentPt);
                 }
-                
-                return objatt;
             }
             
             return null;
@@ -438,8 +499,8 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
             IScenePresence sp, uint AttachmentPt, UUID itemID, SceneObjectGroup att)
         {
 //            m_log.DebugFormat(
-//                "[USER INVENTORY]: Updating attachment {0} for {1} at {2} using item ID {3}", 
-//                att.Name, remoteClient.Name, AttachmentPt, itemID);
+//                "[USER INVENTORY]: Updating attachment {0} for {1} at {2} using item ID {3}",
+//                att.Name, sp.Name, AttachmentPt, itemID);
             
             if (UUID.Zero == itemID)
             {
@@ -462,6 +523,9 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
 
         public void DetachObject(uint objectLocalID, IClientAPI remoteClient)
         {
+//            m_log.DebugFormat(
+//                "[ATTACHMENTS MODULE]: DetachObject() for object {0} on {1}", objectLocalID, remoteClient.Name);
+
             SceneObjectGroup group = m_scene.GetGroupByPrim(objectLocalID);
             if (group != null)
             {
@@ -474,14 +538,17 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
             ScenePresence presence;
             if (m_scene.TryGetScenePresence(remoteClient.AgentId, out presence))
             {
-                // Save avatar attachment information
-                m_log.Debug("[ATTACHMENTS MODULE]: Detaching from UserID: " + remoteClient.AgentId + ", ItemID: " + itemID);
-
-                bool changed = presence.Appearance.DetachAttachment(itemID);
-                if (changed && m_scene.AvatarFactory != null)
-                    m_scene.AvatarFactory.QueueAppearanceSave(remoteClient.AgentId);
+                lock (presence.AttachmentsSyncLock)
+                {
+                    // Save avatar attachment information
+                    m_log.Debug("[ATTACHMENTS MODULE]: Detaching from UserID: " + remoteClient.AgentId + ", ItemID: " + itemID);
 
-                DetachSingleAttachmentToInv(itemID, presence);
+                    bool changed = presence.Appearance.DetachAttachment(itemID);
+                    if (changed && m_scene.AvatarFactory != null)
+                        m_scene.AvatarFactory.QueueAppearanceSave(remoteClient.AgentId);
+    
+                    DetachSingleAttachmentToInv(itemID, presence);
+                }
             }
         }
 
@@ -489,7 +556,7 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
         {
 //            m_log.DebugFormat(
 //                "[ATTACHMENTS MODULE]: DetachSingleAttachmentToGround() for {0}, object {1}",
-//                remoteClient.Name, sceneObjectID);
+//                remoteClient.Name, soLocalId);
 
             SceneObjectGroup so = m_scene.GetGroupByPrim(soLocalId);
 
@@ -508,24 +575,27 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
             ScenePresence presence;
             if (m_scene.TryGetScenePresence(remoteClient.AgentId, out presence))
             {
-                if (!m_scene.Permissions.CanRezObject(
-                    so.PrimCount, remoteClient.AgentId, presence.AbsolutePosition))
-                    return;
+                lock (presence.AttachmentsSyncLock)
+                {
+                    if (!m_scene.Permissions.CanRezObject(
+                        so.PrimCount, remoteClient.AgentId, presence.AbsolutePosition))
+                        return;
 
-                bool changed = presence.Appearance.DetachAttachment(inventoryID);
-                if (changed && m_scene.AvatarFactory != null)
-                    m_scene.AvatarFactory.QueueAppearanceSave(remoteClient.AgentId);
+                    bool changed = presence.Appearance.DetachAttachment(inventoryID);
+                    if (changed && m_scene.AvatarFactory != null)
+                        m_scene.AvatarFactory.QueueAppearanceSave(remoteClient.AgentId);
 
-                presence.RemoveAttachment(so);
-                DetachSceneObjectToGround(so, presence);
+                    presence.RemoveAttachment(so);
+                    DetachSceneObjectToGround(so, presence);
 
-                List<UUID> uuids = new List<UUID>();
-                uuids.Add(inventoryID);
-                m_scene.InventoryService.DeleteItems(remoteClient.AgentId, uuids);
-                remoteClient.SendRemoveInventoryItem(inventoryID);
-            }
+                    List<UUID> uuids = new List<UUID>();
+                    uuids.Add(inventoryID);
+                    m_scene.InventoryService.DeleteItems(remoteClient.AgentId, uuids);
+                    remoteClient.SendRemoveInventoryItem(inventoryID);
+                }
 
-            m_scene.EventManager.TriggerOnAttach(so.LocalId, so.UUID, UUID.Zero);
+                m_scene.EventManager.TriggerOnAttach(so.LocalId, so.UUID, UUID.Zero);
+            }
         }
 
         /// <summary>
@@ -559,6 +629,8 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
         // To LocalId or UUID, *THAT* is the question. How now Brown UUID??
         private void DetachSingleAttachmentToInv(UUID itemID, IScenePresence sp)
         {
+//            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;
 
@@ -567,37 +639,29 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
             EntityBase[] detachEntities = m_scene.GetEntities();
             SceneObjectGroup group;
 
-            foreach (EntityBase entity in detachEntities)
+            lock (sp.AttachmentsSyncLock)
             {
-                if (entity is SceneObjectGroup)
+                foreach (EntityBase entity in detachEntities)
                 {
-                    group = (SceneObjectGroup)entity;
-                    if (group.GetFromItemID() == itemID)
+                    if (entity is SceneObjectGroup)
                     {
-                        m_scene.EventManager.TriggerOnAttach(group.LocalId, itemID, UUID.Zero);
-                        sp.RemoveAttachment(group);
-
-                        // Prepare sog for storage
-                        group.AttachedAvatar = UUID.Zero;
-
-                        group.ForEachPart(
-                            delegate(SceneObjectPart part)
-                            {
-                                // If there are any scripts,
-                                // then always trigger a new object and state persistence in UpdateKnownItem()
-                                if (part.Inventory.ContainsScripts())
-                                    group.HasGroupChanged = true;
-                            }
-                        );
-
-                        group.RootPart.SetParentLocalId(0);
-                        group.IsAttachment = false;
-                        group.AbsolutePosition = group.RootPart.AttachedPos;
-
-                        UpdateKnownItem(sp.ControllingClient, group, group.GetFromItemID(), group.OwnerID);
-                        m_scene.DeleteSceneObject(group, false);
-
-                        return;
+                        group = (SceneObjectGroup)entity;
+                        if (group.GetFromItemID() == itemID)
+                        {
+                            m_scene.EventManager.TriggerOnAttach(group.LocalId, itemID, UUID.Zero);
+                            sp.RemoveAttachment(group);
+
+                            // Prepare sog for storage
+                            group.AttachedAvatar = UUID.Zero;
+                            group.RootPart.SetParentLocalId(0);
+                            group.IsAttachment = false;
+                            group.AbsolutePosition = group.RootPart.AttachedPos;
+
+                            UpdateKnownItem(sp.ControllingClient, group);
+                            m_scene.DeleteSceneObject(group, false);
+
+                            return;
+                        }
                     }
                 }
             }
@@ -628,28 +692,17 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
         /// </remarks>
         /// <param name="remoteClient"></param>
         /// <param name="grp"></param>
-        /// <param name="itemID"></param>
-        /// <param name="agentID"></param>
-        public void UpdateKnownItem(IClientAPI remoteClient, SceneObjectGroup grp, UUID itemID, UUID agentID)
+        private void UpdateKnownItem(IClientAPI remoteClient, SceneObjectGroup grp)
         {
-            if (grp != null)
+            if (grp.HasGroupChanged || grp.ContainsScripts())
             {
-                if (!grp.HasGroupChanged)
-                {
-                    m_log.DebugFormat(
-                        "[ATTACHMENTS MODULE]: Don't need to update asset for unchanged attachment {0}, attachpoint {1}",
-                        grp.UUID, grp.AttachmentPoint);
-
-                    return;
-                }
-
                 m_log.DebugFormat(
                     "[ATTACHMENTS MODULE]: Updating asset for attachment {0}, attachpoint {1}",
                     grp.UUID, grp.AttachmentPoint);
 
                 string sceneObjectXml = SceneObjectSerializer.ToOriginalXmlFormat(grp);
 
-                InventoryItemBase item = new InventoryItemBase(itemID, remoteClient.AgentId);
+                InventoryItemBase item = new InventoryItemBase(grp.GetFromItemID(), remoteClient.AgentId);
                 item = m_scene.InventoryService.GetItem(item);
 
                 if (item != null)
@@ -675,6 +728,12 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
                         remoteClient.SendInventoryItemCreateUpdate(item, 0);
                 }
             }
+            else
+            {
+                m_log.DebugFormat(
+                    "[ATTACHMENTS MODULE]: Don't need to update asset for unchanged attachment {0}, attachpoint {1}",
+                    grp.UUID, grp.AttachmentPoint);
+            }
         } 
         
         /// <summary>
@@ -692,7 +751,8 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
         private void AttachToAgent(
             IScenePresence avatar, SceneObjectGroup so, uint attachmentpoint, Vector3 attachOffset, bool silent)
         {
-//            m_log.DebugFormat("[ATTACHMENTS MODULE]: Adding attachment {0} to avatar {1} in pt {2} pos {3} {4}",
+//            m_log.DebugFormat(
+//                "[ATTACHMENTS MODULE]: Adding attachment {0} to avatar {1} in pt {2} pos {3} {4}",
 //                so.Name, avatar.Name, attachmentpoint, attachOffset, so.RootPart.AttachedPos);
                               
             so.DetachFromBackup();
@@ -745,7 +805,9 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
         /// <returns>The user inventory item created that holds the attachment.</returns>
         private InventoryItemBase AddSceneObjectAsNewAttachmentInInv(IClientAPI remoteClient, SceneObjectGroup grp)
         {
-//            m_log.DebugFormat("[SCENE]: Called AddSceneObjectAsAttachment for object {0} {1} for {2} {3} {4}", grp.Name, grp.LocalId, remoteClient.Name, remoteClient.AgentId, AgentId);
+//            m_log.DebugFormat(
+//                "[ATTACHMENTS MODULE]: Called AddSceneObjectAsAttachment for object {0} {1} for {2}",
+//                grp.Name, grp.LocalId, remoteClient.Name);
 
             Vector3 inventoryStoredPosition = new Vector3
                    (((grp.AbsolutePosition.X > (int)Constants.RegionSize)
@@ -829,4 +891,4 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
             return item;
         }
     }
-}
+}
\ No newline at end of file
diff --git a/OpenSim/Region/CoreModules/Avatar/Attachments/Tests/AttachmentsModuleTests.cs b/OpenSim/Region/CoreModules/Avatar/Attachments/Tests/AttachmentsModuleTests.cs
index 35183b3..ff3358f 100644
--- a/OpenSim/Region/CoreModules/Avatar/Attachments/Tests/AttachmentsModuleTests.cs
+++ b/OpenSim/Region/CoreModules/Avatar/Attachments/Tests/AttachmentsModuleTests.cs
@@ -220,6 +220,38 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments.Tests
             Assert.That(m_presence.Appearance.GetAttachpoint(attItemId), Is.EqualTo(0));
         }
 
+        /// <summary>
+        /// Test that attachments don't hang about in the scene when the agent is closed
+        /// </summary>
+        [Test]
+        public void TestRemoveAttachmentsOnAvatarExit()
+        {
+            TestHelpers.InMethod();
+//            log4net.Config.XmlConfigurator.Configure();
+
+            UUID userId = TestHelpers.ParseTail(0x1);
+            UUID attItemId = TestHelpers.ParseTail(0x2);
+            UUID attAssetId = TestHelpers.ParseTail(0x3);
+            string attName = "att";
+
+            UserAccountHelpers.CreateUserWithInventory(scene, userId);
+            InventoryItemBase attItem
+                = UserInventoryHelpers.CreateInventoryItem(
+                    scene, attName, attItemId, attAssetId, userId, InventoryType.Object);
+
+            AgentCircuitData acd = SceneHelpers.GenerateAgentData(userId);
+            acd.Appearance = new AvatarAppearance();
+            acd.Appearance.SetAttachment((int)AttachmentPoint.Chest, attItem.ID, attItem.AssetID);
+            ScenePresence presence = SceneHelpers.AddScenePresence(scene, acd);
+
+            SceneObjectGroup rezzedAtt = presence.GetAttachments()[0];
+
+            scene.IncomingCloseAgent(presence.UUID);
+
+            // Check that we can't retrieve this attachment from the scene.
+            Assert.That(scene.GetSceneObjectGroup(rezzedAtt.UUID), Is.Null);
+        }
+
         [Test]
         public void TestRezAttachmentsOnAvatarEntrance()
         {
diff --git a/OpenSim/Region/CoreModules/Avatar/AvatarFactory/AvatarFactoryModule.cs b/OpenSim/Region/CoreModules/Avatar/AvatarFactory/AvatarFactoryModule.cs
index b6a1564..0cadd83 100644
--- a/OpenSim/Region/CoreModules/Avatar/AvatarFactory/AvatarFactoryModule.cs
+++ b/OpenSim/Region/CoreModules/Avatar/AvatarFactory/AvatarFactoryModule.cs
@@ -211,8 +211,17 @@ namespace OpenSim.Region.CoreModules.Avatar.AvatarFactory
                 // Process the visual params, this may change height as well
                 if (visualParams != null)
                 {
+//                    string[] visualParamsStrings = new string[visualParams.Length];
+//                    for (int i = 0; i < visualParams.Length; i++)
+//                        visualParamsStrings[i] = visualParams[i].ToString();
+//                    m_log.DebugFormat(
+//                        "[AVFACTORY]: Setting visual params for {0} to {1}",
+//                        client.Name, string.Join(", ", visualParamsStrings));
+
+                    float oldHeight = sp.Appearance.AvatarHeight;
                     changed = sp.Appearance.SetVisualParams(visualParams);
-                    if (sp.Appearance.AvatarHeight > 0)
+
+                    if (sp.Appearance.AvatarHeight != oldHeight && sp.Appearance.AvatarHeight > 0)
                         sp.SetHeight(sp.Appearance.AvatarHeight);
                 }
 
@@ -416,6 +425,13 @@ namespace OpenSim.Region.CoreModules.Avatar.AvatarFactory
 
             // m_log.WarnFormat("[AVFACTORY] avatar {0} save appearance",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
+            // assets and item asset id changes to complete.
+            // I don't think we need to worry about doing this within m_setAppearanceLock since the queueing avoids
+            // multiple save requests.
+            SetAppearanceAssets(sp.UUID, sp.Appearance);
+
             m_scene.AvatarService.SetAppearance(agentid, sp.Appearance);
         }
 
@@ -467,7 +483,7 @@ namespace OpenSim.Region.CoreModules.Avatar.AvatarFactory
                 return;
             }
 
-            // m_log.WarnFormat("[AVFACTORY]: Received request for wearables of {0}", client.AgentId);
+//            m_log.DebugFormat("[AVFACTORY]: Received request for wearables of {0}", client.Name);
 
             client.SendWearables(sp.Appearance.Wearables, sp.Appearance.Serial++);
         }
@@ -502,9 +518,6 @@ namespace OpenSim.Region.CoreModules.Avatar.AvatarFactory
 
             avatAppearance.GetAssetsFrom(sp.Appearance);
 
-            // This could take awhile since it needs to pull inventory
-            SetAppearanceAssets(sp.UUID, ref avatAppearance);
-
             lock (m_setAppearanceLock)
             {
                 // Update only those fields that we have changed. This is important because the viewer
@@ -538,7 +551,7 @@ namespace OpenSim.Region.CoreModules.Avatar.AvatarFactory
             return true;
         }
 
-        private void SetAppearanceAssets(UUID userID, ref AvatarAppearance appearance)
+        private void SetAppearanceAssets(UUID userID, AvatarAppearance appearance)
         {
             IInventoryService invService = m_scene.InventoryService;
 
diff --git a/OpenSim/Region/CoreModules/Avatar/InstantMessage/HGMessageTransferModule.cs b/OpenSim/Region/CoreModules/Avatar/InstantMessage/HGMessageTransferModule.cs
index 7753c25..d294692 100644
--- a/OpenSim/Region/CoreModules/Avatar/InstantMessage/HGMessageTransferModule.cs
+++ b/OpenSim/Region/CoreModules/Avatar/InstantMessage/HGMessageTransferModule.cs
@@ -24,6 +24,7 @@
  * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
  * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
+
 using System;
 using System.Collections;
 using System.Collections.Generic;
@@ -145,14 +146,14 @@ namespace OpenSim.Region.CoreModules.Avatar.InstantMessage
                         scene.Entities[toAgentID] is ScenePresence)
                 {
 //                    m_log.DebugFormat(
-//                        "[INSTANT MESSAGE]: Looking for root agent {0} in {1}", 
+//                        "[HG INSTANT MESSAGE]: Looking for root agent {0} in {1}", 
 //                        toAgentID.ToString(), scene.RegionInfo.RegionName);
                                         
                     ScenePresence user = (ScenePresence) scene.Entities[toAgentID];
                     if (!user.IsChildAgent)
                     {
                         // Local message
-//                        m_log.DebugFormat("[INSTANT MESSAGE]: Delivering IM to root agent {0} {1}", user.Name, toAgentID);
+//                        m_log.DebugFormat("[HG INSTANT MESSAGE]: Delivering IM to root agent {0} {1}", user.Name, toAgentID);
                         user.ControllingClient.SendInstantMessage(im);
 
                         // Message sent
@@ -166,7 +167,7 @@ namespace OpenSim.Region.CoreModules.Avatar.InstantMessage
             foreach (Scene scene in m_Scenes)
             {
 //                m_log.DebugFormat(
-//                    "[INSTANT MESSAGE]: Looking for child of {0} in {1}", toAgentID, scene.RegionInfo.RegionName);
+//                    "[HG INSTANT MESSAGE]: Looking for child of {0} in {1}", toAgentID, scene.RegionInfo.RegionName);
 
                 if (scene.Entities.ContainsKey(toAgentID) &&
                         scene.Entities[toAgentID] is ScenePresence)
@@ -174,7 +175,7 @@ namespace OpenSim.Region.CoreModules.Avatar.InstantMessage
                     // Local message
                     ScenePresence user = (ScenePresence) scene.Entities[toAgentID];
 
-//                    m_log.DebugFormat("[INSTANT MESSAGE]: Delivering IM to child agent {0} {1}", user.Name, toAgentID);
+//                    m_log.DebugFormat("[HG INSTANT MESSAGE]: Delivering IM to child agent {0} {1}", user.Name, toAgentID);
                     user.ControllingClient.SendInstantMessage(im);
 
                     // Message sent
@@ -183,7 +184,7 @@ namespace OpenSim.Region.CoreModules.Avatar.InstantMessage
                 }
             }
 
-//            m_log.DebugFormat("[INSTANT MESSAGE]: Delivering IM to {0} via XMLRPC", im.toAgentID);
+//            m_log.DebugFormat("[HG INSTANT MESSAGE]: Delivering IM to {0} via XMLRPC", im.toAgentID);
             // Is the user a local user?
             UserAccount account = m_Scenes[0].UserAccountService.GetUserAccount(m_Scenes[0].RegionInfo.ScopeID, toAgentID);
             string url = string.Empty;
diff --git a/OpenSim/Region/CoreModules/Avatar/Lure/HGLureModule.cs b/OpenSim/Region/CoreModules/Avatar/Lure/HGLureModule.cs
index d687e6a..bc5c1ff 100644
--- a/OpenSim/Region/CoreModules/Avatar/Lure/HGLureModule.cs
+++ b/OpenSim/Region/CoreModules/Avatar/Lure/HGLureModule.cs
@@ -154,14 +154,17 @@ namespace OpenSim.Region.CoreModules.Avatar.Lure
             if (im.dialog == (byte)InstantMessageDialog.RequestTeleport)
             {
                 UUID sessionID = new UUID(im.imSessionID);
-                m_log.DebugFormat("[HG LURE MODULE]: RequestTeleport sessionID={0}, regionID={1}, message={2}", im.imSessionID, im.RegionID, im.message);
-                m_PendingLures.Add(sessionID, im, 7200); // 2 hours
+
+                if (!m_PendingLures.Contains(sessionID))
+                {
+                    m_log.DebugFormat("[HG LURE MODULE]: RequestTeleport sessionID={0}, regionID={1}, message={2}", im.imSessionID, im.RegionID, im.message);
+                    m_PendingLures.Add(sessionID, im, 7200); // 2 hours
+                }
 
                 // Forward. We do this, because the IM module explicitly rejects
                 // IMs of this type
                 if (m_TransferModule != null)
                     m_TransferModule.SendInstantMessage(im, delegate(bool success) { });
-
             }
         }
 
@@ -177,12 +180,17 @@ namespace OpenSim.Region.CoreModules.Avatar.Lure
 
             m_log.DebugFormat("[HG LURE MODULE]: TP invite with message {0}", message);
 
+            UUID sessionID = UUID.Random();
+
             GridInstantMessage m = new GridInstantMessage(scene, client.AgentId,
                     client.FirstName+" "+client.LastName, targetid,
                     (byte)InstantMessageDialog.RequestTeleport, false,
-                    message, UUID.Random(), false, presence.AbsolutePosition,
+                    message, sessionID, false, presence.AbsolutePosition,
                     new Byte[0]);
             m.RegionID = client.Scene.RegionInfo.RegionID.Guid;
+
+            m_log.DebugFormat("[HG LURE MODULE]: RequestTeleport sessionID={0}, regionID={1}, message={2}", m.imSessionID, m.RegionID, m.message);
+            m_PendingLures.Add(sessionID, m, 7200); // 2 hours
                     
             if (m_TransferModule != null)
             {