Fix issue where only one of multiple attachments on the same attachpoint for NPCs would be seen by other viewers.

It appears that at least Singularity 1.8.5 (but probably others) rely on attachment FromItemIDs being different to display more than one.
This commit resolves this by generating random IDs instead of always using UUID.Zero for NPCs.
Resolves http://opensimulator.org/mantis/view.php?id=7110

4 files changed, 74 insertions, 7 deletions

diff --git a/OpenSim/Region/ClientStack/Linden/UDP/LLClientView.cs b/OpenSim/Region/ClientStack/Linden/UDP/LLClientView.cs
index 3ab837d..7cb5b53 100644
--- a/OpenSim/Region/ClientStack/Linden/UDP/LLClientView.cs
+++ b/OpenSim/Region/ClientStack/Linden/UDP/LLClientView.cs
@@ -5304,10 +5304,22 @@ namespace OpenSim.Region.ClientStack.LindenUDP
             //update.JointType = 0;
             update.Material = data.Material;
             update.MediaURL = Utils.EmptyBytes; // FIXME: Support this in OpenSim
+
             if (data.ParentGroup.IsAttachment)
             {
-                update.NameValue = Util.StringToBytes256("AttachItemID STRING RW SV " + data.ParentGroup.FromItemID);
+                update.NameValue 
+                    = Util.StringToBytes256(
+                        string.Format("AttachItemID STRING RW SV {0}", data.ParentGroup.FromItemID));
+
                 update.State = (byte)((data.ParentGroup.AttachmentPoint % 16) * 16 + (data.ParentGroup.AttachmentPoint / 16));
+
+//                m_log.DebugFormat(
+//                    "[LLCLIENTVIEW]: Sending NameValue {0} for {1} {2} to {3}",
+//                    Util.UTF8.GetString(update.NameValue), data.Name, data.LocalId, Name);
+//
+//                m_log.DebugFormat(
+//                    "[LLCLIENTVIEW]: Sending state {0} for {1} {2} to {3}",
+//                    update.State, data.Name, data.LocalId, Name);
             }
             else
             {
@@ -5318,10 +5330,6 @@ namespace OpenSim.Region.ClientStack.LindenUDP
                 update.State = data.ParentGroup.RootPart.Shape.State;
             }
 
-//                m_log.DebugFormat(
-//                    "[LLCLIENTVIEW]: Sending state {0} for {1} {2} to {3}",
-//                    update.State, data.Name, data.LocalId, Name);
-
             update.ObjectData = objectData;
             update.ParentID = data.ParentID;
             update.PathBegin = data.Shape.PathBegin;
diff --git a/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs b/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs
index c3846d9..e9b2f4f 100644
--- a/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs
+++ b/OpenSim/Region/CoreModules/Avatar/Attachments/AttachmentsModule.cs
@@ -991,6 +991,13 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments
 
                 return null;
             }
+            else if (itemID == UUID.Zero)
+            {
+                // We need to have a FromItemID for multiple attachments on a single attach point to appear.  This is 
+                // true on Singularity 1.8.5 and quite possibly other viewers as well.  As NPCs don't have an inventory
+                // we will satisfy this requirement by inserting a random UUID.
+                objatt.FromItemID = UUID.Random();
+            }
 
             if (DebugLevel > 0)
                 m_log.DebugFormat(
diff --git a/OpenSim/Region/OptionalModules/World/NPC/NPCModule.cs b/OpenSim/Region/OptionalModules/World/NPC/NPCModule.cs
index 8a2da6e..03c6265 100644
--- a/OpenSim/Region/OptionalModules/World/NPC/NPCModule.cs
+++ b/OpenSim/Region/OptionalModules/World/NPC/NPCModule.cs
@@ -178,8 +178,7 @@ namespace OpenSim.Region.OptionalModules.World.NPC
             acd.lastname = lastname;
             acd.ServiceURLs = new Dictionary<string, object>();
 
-            AvatarAppearance npcAppearance = new AvatarAppearance(appearance,
-                    true);
+            AvatarAppearance npcAppearance = new AvatarAppearance(appearance, true);
             acd.Appearance = npcAppearance;
 
             /*
diff --git a/OpenSim/Region/OptionalModules/World/NPC/Tests/NPCModuleTests.cs b/OpenSim/Region/OptionalModules/World/NPC/Tests/NPCModuleTests.cs
index ddd4525..e82351e 100644
--- a/OpenSim/Region/OptionalModules/World/NPC/Tests/NPCModuleTests.cs
+++ b/OpenSim/Region/OptionalModules/World/NPC/Tests/NPCModuleTests.cs
@@ -199,6 +199,59 @@ namespace OpenSim.Region.OptionalModules.World.NPC.Tests
         }
 
         [Test]
+        public void TestCreateWithMultiAttachments()
+        {
+            TestHelpers.InMethod();
+//            TestHelpers.EnableLogging();
+
+            SetUpScene();
+//            m_attMod.DebugLevel = 1;
+
+            UUID userId = TestHelpers.ParseTail(0x1);
+            UserAccountHelpers.CreateUserWithInventory(m_scene, userId);
+            ScenePresence sp = SceneHelpers.AddScenePresence(m_scene, userId);
+
+            InventoryItemBase att1Item 
+                = UserInventoryHelpers.CreateInventoryItem(
+                    m_scene, "att1", TestHelpers.ParseTail(0x2), TestHelpers.ParseTail(0x3), sp.UUID, InventoryType.Object);
+            InventoryItemBase att2Item 
+                = UserInventoryHelpers.CreateInventoryItem(
+                    m_scene, "att2", TestHelpers.ParseTail(0x12), TestHelpers.ParseTail(0x13), sp.UUID, InventoryType.Object);
+
+            m_attMod.RezSingleAttachmentFromInventory(sp, att1Item.ID, (uint)AttachmentPoint.Chest);
+            m_attMod.RezSingleAttachmentFromInventory(sp, att2Item.ID, (uint)AttachmentPoint.Chest | 0x80);
+
+            UUID npcId = m_npcMod.CreateNPC("John", "Smith", new Vector3(128, 128, 30), UUID.Zero, true, m_scene, sp.Appearance);
+
+            ScenePresence npc = m_scene.GetScenePresence(npcId);
+
+            // Check scene presence status
+            Assert.That(npc.HasAttachments(), Is.True);
+            List<SceneObjectGroup> attachments = npc.GetAttachments();
+            Assert.That(attachments.Count, Is.EqualTo(2));
+
+            // Just for now, we won't test the name since this is (wrongly) the asset part name rather than the item
+            // name.  TODO: Do need to fix ultimately since the item may be renamed before being passed on to an NPC.
+//            Assert.That(attSo.Name, Is.EqualTo(attName));
+
+            TestAttachedObject(attachments[0], AttachmentPoint.Chest, npc.UUID);
+            TestAttachedObject(attachments[1], AttachmentPoint.Chest, npc.UUID);
+
+            // Attached objects on the same point must have different FromItemIDs to be shown to other avatars, at least
+            // on Singularity 1.8.5.  Otherwise, only one (the first ObjectUpdate sent) appears.
+            Assert.AreNotEqual(attachments[0].FromItemID, attachments[1].FromItemID);
+        }
+
+        private void TestAttachedObject(SceneObjectGroup attSo, AttachmentPoint attPoint, UUID ownerId)
+        {
+            Assert.That(attSo.AttachmentPoint, Is.EqualTo((byte)attPoint));
+            Assert.That(attSo.IsAttachment);
+            Assert.That(attSo.UsesPhysics, Is.False);
+            Assert.That(attSo.IsTemporary, Is.False);
+            Assert.That(attSo.OwnerID, Is.EqualTo(ownerId));
+        }
+
+        [Test]
         public void TestLoadAppearance()
         {
             TestHelpers.InMethod();