Stop NPCs losing attachments when the source avatar takes them off.

This was happening because we were using the source avatar's item IDs in the clone appearance.
Switch to using the asset IDs of attachments instead for NPCs.
The InventoryAccessModule and AttachmentModule had to be changed to allow rezzing of an object without an associated inventory item.
Hopefully goes some way towards resolving http://opensimulator.org/mantis/view.php?id=5653

7 files changed, 154 insertions, 70 deletions

diff --git a/OpenSim/Framework/AvatarAppearance.cs b/OpenSim/Framework/AvatarAppearance.cs
index 7c6295d..aed9087 100644
--- a/OpenSim/Framework/AvatarAppearance.cs
+++ b/OpenSim/Framework/AvatarAppearance.cs
@@ -433,6 +433,10 @@ namespace OpenSim.Framework
             if (attachpoint == 0)
                 return false;
 
+//            m_log.DebugFormat(
+//                "[AVATAR APPEARANCE]: Setting attachment at {0} with item ID {1}, asset ID {2}",
+//                 attachpoint, item, asset);
+
             if (item == UUID.Zero)
             {
                 if (m_attachments.ContainsKey(attachpoint))
@@ -459,7 +463,7 @@ namespace OpenSim.Framework
             }
             else
             {
-                ReplaceAttachment(new AvatarAttachment(attachpoint,item,asset));
+                ReplaceAttachment(new AvatarAttachment(attachpoint,item, asset));
             }
             return true;
         }
@@ -608,7 +612,14 @@ namespace OpenSim.Framework
                 {
                     OSDArray attachs = (OSDArray)(data["attachments"]);
                     for (int i = 0; i < attachs.Count; i++)
-                        AppendAttachment(new AvatarAttachment((OSDMap)attachs[i]));
+                    {
+                        AvatarAttachment att = new AvatarAttachment((OSDMap)attachs[i]);
+                        AppendAttachment(att);
+                        
+//                        m_log.DebugFormat(
+//                            "[AVATAR APPEARANCE]: Unpacked attachment itemID {0}, assetID {1}, point {2}",
+//                            att.ItemID, att.AssetID, att.AttachPoint);
+                    }
                 }
             }
             catch (Exception e)
diff --git a/OpenSim/Framework/AvatarAttachment.cs b/OpenSim/Framework/AvatarAttachment.cs
index c68d78d..07dd385 100644
--- a/OpenSim/Framework/AvatarAttachment.cs
+++ b/OpenSim/Framework/AvatarAttachment.cs
@@ -66,11 +66,11 @@ namespace OpenSim.Framework
             return attachdata;
         }
 
-
         public void Unpack(OSDMap args)
         {
             if (args["point"] != null)
                 AttachPoint = args["point"].AsInteger();
+
             ItemID = (args["item"] != null) ? args["item"].AsUUID() : UUID.Zero;
             AssetID = (args["asset"] != null) ? args["asset"].AsUUID() : UUID.Zero;
         }
diff --git a/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs b/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs
index ffe76a8..4881499 100644
--- a/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs
+++ b/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs
@@ -111,10 +111,12 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
             List<AvatarAttachment> attachments = sp.Appearance.GetAttachments();
             foreach (AvatarAttachment attach in attachments)
             {
-                int p = attach.AttachPoint;
-                UUID itemID = attach.ItemID;
+                uint p = (uint)attach.AttachPoint;
+
+//                m_log.DebugFormat(
+//                    "[ATTACHMENTS MODULE]: Doing initial rez of attachment with itemID {0}, assetID {1}, point {2} for {3} in {4}",
+//                    attach.ItemID, attach.AssetID, p, sp.Name, m_scene.RegionInfo.RegionName);
 
-                //UUID assetID = attach.AssetID;
                 // For some reason assetIDs are being written as Zero's in the DB -- need to track tat down
                 // But they're not used anyway, the item is being looked up for now, so let's proceed.
                 //if (UUID.Zero == assetID) 
@@ -125,7 +127,12 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
 
                 try
                 {
-                    RezSingleAttachmentFromInventory(sp.ControllingClient, itemID, (uint)p);
+                    // If we're an NPC then skip all the item checks and manipulations since we don't have an
+                    // inventory right now.
+                    if (sp.PresenceType == PresenceType.Npc)
+                        RezSingleAttachmentFromInventoryInternal(sp, UUID.Zero, attach.AssetID, p);
+                    else
+                        RezSingleAttachmentFromInventory(sp.ControllingClient, attach.ItemID, p);
                 }
                 catch (Exception e)
                 {
@@ -231,7 +238,7 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
             return AttachObject(sp, group, AttachmentPt, silent);
         }
         
-        private bool AttachObject(ScenePresence sp, SceneObjectGroup group, uint attachmentPt, bool silent)
+        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})",
@@ -284,17 +291,21 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
             List<SceneObjectGroup> attachments = sp.GetAttachments(attachmentPt);
 
             // At the moment we can only deal with a single attachment
-            if (attachments.Count != 0)
-                itemID = attachments[0].GetFromItemID();
-
-            if (itemID != UUID.Zero)
-                DetachSingleAttachmentToInv(itemID, sp);
-
-            itemID = group.GetFromItemID();
-            if (itemID == UUID.Zero)
-                itemID = AddSceneObjectAsAttachment(sp.ControllingClient, group).ID;
-
-            ShowAttachInUserInventory(sp, attachmentPt, itemID, group);
+            // We also don't want to do any of the inventory operations for an NPC.
+            if (sp.PresenceType != PresenceType.Npc)
+            {
+                if (attachments.Count != 0)
+                    itemID = attachments[0].GetFromItemID();
+    
+                if (itemID != UUID.Zero)
+                    DetachSingleAttachmentToInv(itemID, sp);
+    
+                itemID = group.GetFromItemID();
+                if (itemID == UUID.Zero)
+                    itemID = AddSceneObjectAsAttachment(sp.ControllingClient, group).ID;
+    
+                ShowAttachInUserInventory(sp, attachmentPt, itemID, group);
+            }
 
             AttachToAgent(sp, group, attachmentPt, attachPos, silent);
 
@@ -312,7 +323,8 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
             }
         }
         
-        public ISceneEntity RezSingleAttachmentFromInventory(IClientAPI remoteClient, UUID itemID, uint AttachmentPt)
+        public ISceneEntity RezSingleAttachmentFromInventory(
+            IClientAPI remoteClient, UUID itemID, uint AttachmentPt)
         {
             return RezSingleAttachmentFromInventory(remoteClient, itemID, AttachmentPt, true);
         }
@@ -338,7 +350,7 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
             // be removed when that functionality is implemented in opensim
             AttachmentPt &= 0x7f;
 
-            SceneObjectGroup att = RezSingleAttachmentFromInventoryInternal(sp, itemID, AttachmentPt);
+            SceneObjectGroup att = RezSingleAttachmentFromInventoryInternal(sp, itemID, UUID.Zero, AttachmentPt);
 
             if (updateInventoryStatus)
             {
@@ -352,14 +364,21 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
         }
 
         private SceneObjectGroup RezSingleAttachmentFromInventoryInternal(
-            ScenePresence sp, UUID itemID, uint attachmentPt)
+            IScenePresence sp, UUID itemID, UUID assetID, uint attachmentPt)
         {
             IInventoryAccessModule invAccess = m_scene.RequestModuleInterface<IInventoryAccessModule>();
             if (invAccess != null)
             {
-                SceneObjectGroup objatt = invAccess.RezObject(sp.ControllingClient,
-                    itemID, Vector3.Zero, Vector3.Zero, UUID.Zero, (byte)1, true,
-                    false, false, sp.UUID, true);
+                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}",
@@ -425,7 +444,7 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
         /// <param name="attachmentPoint"></param>
         /// <returns></returns>
         private UUID ShowAttachInUserInventory(
-            SceneObjectGroup att, ScenePresence sp, UUID itemID, uint attachmentPoint)
+            SceneObjectGroup att, IScenePresence sp, UUID itemID, uint attachmentPoint)
         {
 //            m_log.DebugFormat(
 //                "[ATTACHMENTS MODULE]: Updating inventory of {0} to show attachment of {1} {2} (item ID {3}) at {4}",
@@ -452,7 +471,7 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
         /// <param name="itemID"></param>
         /// <param name="att"></param>
         private void ShowAttachInUserInventory(
-            ScenePresence sp, uint AttachmentPt, UUID itemID, SceneObjectGroup att)
+            IScenePresence sp, uint AttachmentPt, UUID itemID, SceneObjectGroup att)
         {
 //            m_log.DebugFormat(
 //                "[USER INVENTORY]: Updating attachment {0} for {1} at {2} using item ID {3}", 
@@ -574,7 +593,7 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
         
         // 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 DetachSingleAttachmentToInv(UUID itemID, ScenePresence sp)
+        private void DetachSingleAttachmentToInv(UUID itemID, IScenePresence sp)
         {
             if (itemID == UUID.Zero) // If this happened, someone made a mistake....
                 return;
@@ -697,16 +716,17 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
         /// <summary>
         /// Attach this scene object to the given avatar.
         /// </summary>
-        /// 
+        /// <remarks>
         /// This isn't publicly available since attachments should always perform the corresponding inventory 
         /// operation (to show the attach in user inventory and update the asset with positional information).
-        /// 
+        /// </remarks>
         /// <param name="sp"></param>
         /// <param name="so"></param>
         /// <param name="attachmentpoint"></param>
         /// <param name="attachOffset"></param>
         /// <param name="silent"></param>
-        protected void AttachToAgent(ScenePresence avatar, SceneObjectGroup so, uint attachmentpoint, Vector3 attachOffset, bool silent)
+        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}",
 //                so.Name, avatar.Name, attachmentpoint, attachOffset, so.RootPart.AttachedPos);
diff --git a/OpenSim/Region/CoreModules/Framework/InventoryAccess/InventoryAccessModule.cs b/OpenSim/Region/CoreModules/Framework/InventoryAccess/InventoryAccessModule.cs
index 0c0b0c5..4e8466d 100644
--- a/OpenSim/Region/CoreModules/Framework/InventoryAccess/InventoryAccessModule.cs
+++ b/OpenSim/Region/CoreModules/Framework/InventoryAccess/InventoryAccessModule.cs
@@ -669,28 +669,10 @@ namespace OpenSim.Region.CoreModules.Framework.InventoryAccess
             return item;
         }
 
-        /// <summary>
-        /// Rez an object into the scene from the user's inventory
-        /// </summary>
-        /// <remarks>
-        /// FIXME: It would be really nice if inventory access modules didn't also actually do the work of rezzing
-        /// things to the scene.  The caller should be doing that, I think.
-        /// </remarks>
-        /// <param name="remoteClient"></param>
-        /// <param name="itemID"></param>
-        /// <param name="RayEnd"></param>
-        /// <param name="RayStart"></param>
-        /// <param name="RayTargetID"></param>
-        /// <param name="BypassRayCast"></param>
-        /// <param name="RayEndIsIntersection"></param>
-        /// <param name="RezSelected"></param>
-        /// <param name="RemoveItem"></param>
-        /// <param name="fromTaskID"></param>
-        /// <param name="attachment"></param>
-        /// <returns>The SceneObjectGroup rezzed or null if rez was unsuccessful.</returns>
-        public virtual SceneObjectGroup RezObject(IClientAPI remoteClient, UUID itemID, Vector3 RayEnd, Vector3 RayStart,
-                                    UUID RayTargetID, byte BypassRayCast, bool RayEndIsIntersection,
-                                    bool RezSelected, bool RemoveItem, UUID fromTaskID, bool attachment)
+        public virtual SceneObjectGroup RezObject(
+            IClientAPI remoteClient, UUID itemID, Vector3 RayEnd, Vector3 RayStart,
+            UUID RayTargetID, byte BypassRayCast, bool RayEndIsIntersection,
+            bool RezSelected, bool RemoveItem, UUID fromTaskID, bool attachment)
         {
 //            m_log.DebugFormat("[INVENTORY ACCESS MODULE]: RezObject for {0}, item {1}", remoteClient.Name, itemID);
 
@@ -707,13 +689,34 @@ namespace OpenSim.Region.CoreModules.Framework.InventoryAccess
             }
 
             item.Owner = remoteClient.AgentId;
-            AssetBase rezAsset = m_Scene.AssetService.Get(item.AssetID.ToString());
+
+            return RezObject(
+                remoteClient, item, item.AssetID,
+                RayEnd, RayStart, RayTargetID, BypassRayCast, RayEndIsIntersection,
+                RezSelected, RemoveItem, fromTaskID, attachment);
+        }
+
+        public virtual SceneObjectGroup RezObject(
+            IClientAPI remoteClient, InventoryItemBase item, UUID assetID, Vector3 RayEnd, Vector3 RayStart,
+            UUID RayTargetID, byte BypassRayCast, bool RayEndIsIntersection,
+            bool RezSelected, bool RemoveItem, UUID fromTaskID, bool attachment)
+        {
+            AssetBase rezAsset = m_Scene.AssetService.Get(assetID.ToString());
 
             if (rezAsset == null)
             {
-                m_log.WarnFormat(
-                    "[InventoryAccessModule]: Could not find asset {0} for item {1} {2} for {3} in RezObject()",
-                    item.AssetID, item.Name, item.ID, remoteClient.Name);
+                if (item != null)
+                {
+                    m_log.WarnFormat(
+                        "[InventoryAccessModule]: Could not find asset {0} for item {1} {2} for {3} in RezObject()",
+                        assetID, item.Name, item.ID, remoteClient.Name);
+                }
+                else
+                {
+                    m_log.WarnFormat(
+                        "[InventoryAccessModule]: Could not find asset {0} for {1} in RezObject()",
+                        assetID, remoteClient.Name);
+                }
 
                 return null;
             }
@@ -781,7 +784,7 @@ namespace OpenSim.Region.CoreModules.Framework.InventoryAccess
                 }
             }
 
-            if (!DoPreRezWhenFromItem(remoteClient, item, objlist, pos, attachment))
+            if (item != null && !DoPreRezWhenFromItem(remoteClient, item, objlist, pos, attachment))
                 return null;
 
             for (int i = 0; i < objlist.Count; i++)
@@ -829,10 +832,6 @@ namespace OpenSim.Region.CoreModules.Framework.InventoryAccess
 
                     group.AbsolutePosition = pos + veclist[i];
                 }
-                else
-                {
-                    group.SetFromItemID(itemID);
-                }
 
                 SceneObjectPart rootPart = group.RootPart;
 
@@ -855,7 +854,8 @@ namespace OpenSim.Region.CoreModules.Framework.InventoryAccess
 //                            group.Name, group.LocalId, group.UUID, remoteClient.Name);
             }
 
-            DoPostRezWhenFromItem(item, attachment);
+            if (item != null)
+                DoPostRezWhenFromItem(item, attachment);
 
             return group;
         }
@@ -973,6 +973,9 @@ namespace OpenSim.Region.CoreModules.Framework.InventoryAccess
                 }
     
                 rootPart.TrimPermissions();
+
+                if (isAttachment)
+                    so.SetFromItemID(item.ID);
             }
 
             return true;
diff --git a/OpenSim/Region/Framework/Interfaces/IInventoryAccessModule.cs b/OpenSim/Region/Framework/Interfaces/IInventoryAccessModule.cs
index da11e61..1904011 100644
--- a/OpenSim/Region/Framework/Interfaces/IInventoryAccessModule.cs
+++ b/OpenSim/Region/Framework/Interfaces/IInventoryAccessModule.cs
@@ -54,10 +54,58 @@ namespace OpenSim.Region.Framework.Interfaces
         /// FIXME: This is not very useful.  It would be far more useful to return a list of items instead.
         /// </returns>
         UUID CopyToInventory(DeRezAction action, UUID folderID, List<SceneObjectGroup> objectGroups, IClientAPI remoteClient);
-        
-        SceneObjectGroup RezObject(IClientAPI remoteClient, UUID itemID, Vector3 RayEnd, Vector3 RayStart,
-                                    UUID RayTargetID, byte BypassRayCast, bool RayEndIsIntersection,
-                                    bool RezSelected, bool RemoveItem, UUID fromTaskID, bool attachment);
+
+        /// <summary>
+        /// Rez an object into the scene from the user's inventory
+        /// </summary>
+        /// <remarks>
+        /// FIXME: It would be really nice if inventory access modules didn't also actually do the work of rezzing
+        /// things to the scene.  The caller should be doing that, I think.
+        /// </remarks>
+        /// <param name="remoteClient"></param>
+        /// <param name="itemID"></param>
+        /// <param name="RayEnd"></param>
+        /// <param name="RayStart"></param>
+        /// <param name="RayTargetID"></param>
+        /// <param name="BypassRayCast"></param>
+        /// <param name="RayEndIsIntersection"></param>
+        /// <param name="RezSelected"></param>
+        /// <param name="RemoveItem"></param>
+        /// <param name="fromTaskID"></param>
+        /// <param name="attachment"></param>
+        /// <returns>The SceneObjectGroup rezzed or null if rez was unsuccessful.</returns>
+        SceneObjectGroup RezObject(
+            IClientAPI remoteClient, UUID itemID, Vector3 RayEnd, Vector3 RayStart,
+            UUID RayTargetID, byte BypassRayCast, bool RayEndIsIntersection,
+            bool RezSelected, bool RemoveItem, UUID fromTaskID, bool attachment);
+
+        /// <summary>
+        /// Rez an object into the scene from the user's inventory
+        /// </summary>
+        /// <remarks>
+        /// FIXME: It would be really nice if inventory access modules didn't also actually do the work of rezzing
+        /// things to the scene.  The caller should be doing that, I think.
+        /// </remarks>
+        /// <param name="remoteClient"></param>
+        /// <param name="item">
+        /// The item from which the object asset came.  Can be null, in which case pre and post rez item adjustment and checks are not performed.
+        /// </param>
+        /// <param name="assetID">The asset id for the object to rez.</param>
+        /// <param name="RayEnd"></param>
+        /// <param name="RayStart"></param>
+        /// <param name="RayTargetID"></param>
+        /// <param name="BypassRayCast"></param>
+        /// <param name="RayEndIsIntersection"></param>
+        /// <param name="RezSelected"></param>
+        /// <param name="RemoveItem"></param>
+        /// <param name="fromTaskID"></param>
+        /// <param name="attachment"></param>
+        /// <returns>The SceneObjectGroup rezzed or null if rez was unsuccessful.</returns>
+        SceneObjectGroup RezObject(
+            IClientAPI remoteClient, InventoryItemBase item, UUID assetID, Vector3 RayEnd, Vector3 RayStart,
+            UUID RayTargetID, byte BypassRayCast, bool RayEndIsIntersection,
+            bool RezSelected, bool RemoveItem, UUID fromTaskID, bool attachment);
+
         void TransferInventoryAssets(InventoryItemBase item, UUID sender, UUID receiver);
 
         /// <summary>
diff --git a/OpenSim/Region/Framework/Interfaces/IScenePresence.cs b/OpenSim/Region/Framework/Interfaces/IScenePresence.cs
index 91e4bf2..8913133 100644
--- a/OpenSim/Region/Framework/Interfaces/IScenePresence.cs
+++ b/OpenSim/Region/Framework/Interfaces/IScenePresence.cs
@@ -46,6 +46,11 @@ namespace OpenSim.Region.Framework.Interfaces
         IClientAPI ControllingClient { get; }
 
         /// <summary>
+        /// What type of presence is this?  User, NPC, etc.
+        /// </summary>
+        PresenceType PresenceType { get; }
+
+        /// <summary>
         /// Avatar appearance data.
         /// </summary>
         /// <remarks>
diff --git a/OpenSim/Region/Framework/Scenes/ScenePresence.cs b/OpenSim/Region/Framework/Scenes/ScenePresence.cs
index 0d284a5..1c283c7 100644
--- a/OpenSim/Region/Framework/Scenes/ScenePresence.cs
+++ b/OpenSim/Region/Framework/Scenes/ScenePresence.cs
@@ -75,9 +75,6 @@ namespace OpenSim.Region.Framework.Scenes
 
         private static readonly ILog m_log = LogManager.GetLogger(MethodBase.GetCurrentMethod().DeclaringType);
 
-        /// <summary>
-        /// What type of presence is this?  User, NPC, etc.
-        /// </summary>
         public PresenceType PresenceType { get; private set; }
 
 //        private static readonly byte[] DEFAULT_TEXTURE = AvatarAppearance.GetDefaultTexture().GetBytes();