Very temporarily, always force append bit for attachments to stop multi-attachments failing on relog.

This is because the AvatarAppearance data structures requires an 0x80 to signal append rather than replace but then fails to preserve this bit.
This will go away again once we can signal append by other means.

OpenSim/Framework/AvatarAppearance.cs

@@ -140,7 +140,7 @@ namespace OpenSim.Framework
 
         public AvatarAppearance(AvatarAppearance appearance, bool copyWearables)
         {
-//            m_log.WarnFormat("[AVATAR APPEARANCE] create from an existing appearance");
+            m_log.DebugFormat("[AVATAR APPEARANCE]: Creating from an existing appearance");
 
             if (appearance == null)
             {
@@ -404,11 +404,30 @@ namespace OpenSim.Framework
             return alist;
         }
 
+        /// <summary>
+        /// Get attachments listed by attach point.
+        /// </summary>
+        /// <returns>
+        /// A dictionary that can be altered by the caller.  This will have no impact on the original dictionary.
+        /// </returns>
+        public Dictionary<int, List<AvatarAttachment>> GetAttachmentsByAttachPoint()
+        {
+            Dictionary<int, List<AvatarAttachment>> returnDict = new Dictionary<int, List<AvatarAttachment>>();
+
+            lock (m_attachments)
+            {
+                foreach (KeyValuePair<int, List<AvatarAttachment>> kvp in m_attachments)
+                    returnDict[kvp.Key] = new List<AvatarAttachment>(kvp.Value);
+            }
+
+            return returnDict;
+        }
+
         internal void AppendAttachment(AvatarAttachment attach)
         {
-//            m_log.DebugFormat(
-//                "[AVATAR APPEARNCE]: Appending itemID={0}, assetID={1} at {2}",
-//                attach.ItemID, attach.AssetID, attach.AttachPoint);
+            m_log.DebugFormat(
+                "[AVATAR APPEARNCE]: Appending itemID={0}, assetID={1} at {2}",
+                attach.ItemID, attach.AssetID, attach.AttachPoint);
 
             lock (m_attachments)
             {
@@ -421,9 +440,9 @@ 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_log.DebugFormat(
+                "[AVATAR APPEARANCE]: Replacing itemID={0}, assetID={1} at {2}",
+                attach.ItemID, attach.AssetID, attach.AttachPoint);
 
             lock (m_attachments)
             {
@@ -449,9 +468,9 @@ namespace OpenSim.Framework
         /// </returns>
         public bool SetAttachment(int attachpoint, UUID item, UUID asset)
         {
-//            m_log.DebugFormat(
-//                "[AVATAR APPEARANCE]: Setting attachment at {0} with item ID {1}, asset ID {2}",
-//                 attachpoint, item, asset);
+            m_log.DebugFormat(
+                "[AVATAR APPEARANCE]: Setting attachment at {0} with item ID {1}, asset ID {2}",
+                 attachpoint, item, asset);
 
             if (attachpoint == 0)
                 return false;
@@ -490,11 +509,18 @@ namespace OpenSim.Framework
             {
                 // strip the append bit
                 int point = attachpoint & 0x7F;
-                AppendAttachment(new AvatarAttachment(point, item, asset));
+
+                // We must check if we already have the item attached.  If we do then we must replace since we may
+                // need to set the asset ID
+                // FIXME: This needs to become more straightforward.
+                if (GetAttachmentForItem(item) == null)
+                    AppendAttachment(new AvatarAttachment(point, item, asset));
+                else
+                    ReplaceAttachment(new AvatarAttachment(point, item, asset));
             }
             else
             {
-                ReplaceAttachment(new AvatarAttachment(attachpoint,item, asset));
+                ReplaceAttachment(new AvatarAttachment(attachpoint, item, asset));
             }
 
             return true;
@@ -614,6 +640,8 @@ namespace OpenSim.Framework
         /// </summary>
         public void Unpack(OSDMap data)
         {
+            m_log.DebugFormat("[AVATAR APPEARANCE]: Unpacking OSDMap appearance data");
+
             if ((data != null) && (data["serial"] != null))
                 m_serial = data["serial"].AsInteger();
             if ((data != null) && (data["height"] != null))

OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs

@@ -172,9 +172,9 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
             {
                 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);
+                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);
 
                 // 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.
@@ -265,7 +265,7 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
     
                 // 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;
+//                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
@@ -297,18 +297,18 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
                 if (sp.PresenceType != PresenceType.Npc)
                 {
                     // Remove any previous attachments
-                    List<SceneObjectGroup> attachments = sp.GetAttachments(attachmentPt);
+//                    List<SceneObjectGroup> attachments = sp.GetAttachments(attachmentPt);
     
                     // At the moment we can only deal with a single attachment
-                    if (attachments.Count != 0)
-                    {
-                        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!",
-                                attachments[0].Name, attachments[0].LocalId, attachmentPt, group.Name, group.LocalId, sp.Name);
-                    }
+//                    if (attachments.Count != 0)
+//                    {
+//                        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!",
+//                                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.FromItemID;
@@ -329,13 +329,18 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
             if (!Enabled)
                 return null;
 
-//            m_log.DebugFormat(
-//                "[ATTACHMENTS MODULE]: RezSingleAttachmentFromInventory to point {0} from item {1} for {2}",
-//                (AttachmentPoint)AttachmentPt, itemID, sp.Name);
+            m_log.DebugFormat(
+                "[ATTACHMENTS MODULE]: RezSingleAttachmentFromInventory to point {0} from item {1} for {2}",
+                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;
+//            AttachmentPt &= 0x7f;
+
+            // Temporarily, always add the append bit to so that existing multi-attachments re-rezzed from inventory
+            // on login are preserved
+            // FIXME: We need to stop having to add this bit just go get AvatarAppearance to append rather than replace.
+            AttachmentPt |= 0x80;
 
             // 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.
@@ -356,9 +361,9 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
 //            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);
+                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;
             }
@@ -860,7 +865,7 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
 
                 // 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;
+//                AttachmentPt &= 0x7f;
 
                 // Calls attach with a Zero position
                 if (AttachObject(sp, part.ParentGroup, AttachmentPt, false))

OpenSim/Services/Interfaces/IAvatarService.cs

@@ -28,10 +28,11 @@
 using System;
 using System.Collections;
 using System.Collections.Generic;
-
-using OpenSim.Framework;
-
+using System.Linq;
+using System.Reflection;
+using log4net;
 using OpenMetaverse;
+using OpenSim.Framework;
 
 namespace OpenSim.Services.Interfaces
 {
@@ -91,6 +92,8 @@ namespace OpenSim.Services.Interfaces
     /// </summary>
     public class AvatarData
     {
+        private static readonly ILog m_log = LogManager.GetLogger(MethodBase.GetCurrentMethod().DeclaringType);
+
         // This pretty much determines which name/value pairs will be
         // present below. The name/value pair describe a part of
         // the avatar. For SL avatars, these would be "shape", "texture1",
@@ -172,11 +175,11 @@ namespace OpenSim.Services.Interfaces
 
             Data["VisualParams"] = String.Join(",", vps);
 
-            // Attachments
-            List<AvatarAttachment> attachments = appearance.GetAttachments();
-            foreach (AvatarAttachment attach in attachments)
+            Dictionary<int, List<AvatarAttachment>> attachments = appearance.GetAttachmentsByAttachPoint();
+
+            foreach (KeyValuePair<int, List<AvatarAttachment>> kvp in attachments)
             {
-                Data["_ap_" + attach.AttachPoint] = attach.ItemID.ToString();
+                Data["_ap_" + kvp.Key] = string.Join(",", kvp.Value.ConvertAll<string>(a => a.ItemID.ToString()));
             }
         }
 
@@ -290,25 +293,93 @@ namespace OpenSim.Services.Interfaces
                     }
                 }
 
-                // Attachments
-                Dictionary<string, string> attchs = new Dictionary<string, string>();
-                foreach (KeyValuePair<string, string> _kvp in Data)
-                    if (_kvp.Key.StartsWith("_ap_"))
-                        attchs[_kvp.Key] = _kvp.Value;
+                Dictionary<int, List<UUID>> attachments = new Dictionary<int, List<UUID>>();
 
-                foreach (KeyValuePair<string, string> _kvp in attchs)
+                foreach (KeyValuePair<string, string> kvp in Data)
                 {
-                    string pointStr = _kvp.Key.Substring(4);
-                    int point = 0;
-                    if (!Int32.TryParse(pointStr, out point))
-                        continue;
+                    if (kvp.Key.StartsWith("_ap_"))
+                    {
+                        // Parse index paraemeter on end of key
+                        string rawAttachPoint = kvp.Key.Substring(4);
+                        int attachPoint = 0;
 
-                    UUID uuid = UUID.Zero;
-                    UUID.TryParse(_kvp.Value, out uuid);
+                        if (!int.TryParse(rawAttachPoint, out attachPoint))
+                        {
+                            m_log.WarnFormat(
+                                "[AVATAR DATA]: Received bad attachment key {0} in {1}", rawAttachPoint, kvp.Key);
+                            continue;
+                        }
 
-                    appearance.SetAttachment(point, uuid, UUID.Zero);
+                        // Parse raw item id strings
+                        UUID itemUuid = UUID.Zero;
+                        string[] rawItemIds = kvp.Value.Split(new char[] { ',' });
+                        if (rawItemIds.Length > 0)
+                        {
+                            attachments[attachPoint] = new List<UUID>();
+                            foreach (string rawItemId in rawItemIds)
+                            {
+                                if (UUID.TryParse(rawItemId, out itemUuid))
+                                {
+                                    m_log.DebugFormat("[AVATAR DATA]: Adding item id {0} at {1}", itemUuid, attachPoint);
+
+                                    attachments[attachPoint].Add(itemUuid);
+                                }
+                                else
+                                {
+                                    m_log.WarnFormat(
+                                        "[AVATAR DATA]: Received bad attachment item UUID {0} as part of {1}",
+                                        rawItemId, kvp.Value);
+
+                                    continue;
+                                }
+                            }
+                        }
+                    }
+//                    else if (kvp.Key.StartsWith("_ap2_"))
+//                    {
+//                        string[] parts = kvp.Key.Split(new char[] { '_' });
+//                        string rawAttachPoint = parts[2];
+//                        int attachPoint;
+//
+//                        if (!int.TryParse(rawAttachPoint, out attachPoint))
+//                        {
+//                            m_log.WarnFormat("[AVATAR DATA]: Received bad attachment key {0} in {1}", parts[1], kvp.Key);
+//                            continue;
+//                        }
+//
+//                        UUID itemUuid = UUID.Zero;
+//                        if (!UUID.TryParse(kvp.Value, out itemUuid))
+//                        {
+//                            m_log.WarnFormat("[AVATAR DATA]: Received bad attachment item UUID {0}", kvp.Value);
+//                            continue;
+//                        }
+//
+//                        if (!attchsAp2.ContainsKey(attachPoint))
+//                            attchsAp2[attachPoint] = new List<UUID>();
+//
+//                        attchsAp2[attachPoint].Add(itemUuid);
+//                    }
                 }
 
+                if (attachments.Count > 0)
+                {
+                    foreach (KeyValuePair<int, List<UUID>> kvp in attachments)
+                    {
+                        foreach (UUID itemUuid in kvp.Value)
+                        {
+                            // | 0x80 is a temporary hack to signal that this is an attachment append (this is what
+                            // the viewer uses to indicate this, but ultimately we will want to do something different
+                            appearance.SetAttachment(kvp.Key | 0x80, itemUuid, UUID.Zero);
+                        }
+                    }
+                }
+//                else
+//                {
+//                    m_log.DebugFormat("[AVATAR DATA]: Using ap1 format attachments");
+//                    foreach (KeyValuePair<int, UUID> kvp in attchsAp1)
+//                        appearance.SetAttachment(kvp.Key, kvp.Value, UUID.Zero);
+//                }
+
                 if (appearance.Wearables[AvatarWearable.BODY].Count == 0)
                     appearance.Wearables[AvatarWearable.BODY].Wear(
                             AvatarWearable.DefaultWearables[

bin/OpenSim.exe.config

@@ -12,7 +12,7 @@
   <log4net>
     <appender name="Console" type="OpenSim.Framework.Console.OpenSimAppender, OpenSim.Framework.Console">
       <layout type="log4net.Layout.PatternLayout">
-        <conversionPattern value="%date{HH:mm:ss} - %message" />
+        <conversionPattern value="%date{HH:mm:ss.fff} - %message" />
         <!-- console log with milliseconds.  Useful for debugging -->
 <!--        <conversionPattern value="%date{HH:mm:ss.fff} - %message" /> -->
       </layout>