Go back to setting appearance directly in NPCModule.SetAppearance() to fix mantis 5914

The part reverted is from commit 2ebb421.
Unfortunately, IAvatarFactoryModule.SetAppearance() does not transfer attachments.
I'm not sure how to do this separately, unfortunately I'll need to leave it to Dan :)
Regression test added for this case.
Mantis ref: http://opensimulator.org/mantis/view.php?id=5914

2 files changed, 57 insertions, 9 deletions

diff --git a/OpenSim/Region/OptionalModules/World/NPC/NPCModule.cs b/OpenSim/Region/OptionalModules/World/NPC/NPCModule.cs
index 2052cdb..2b8379d 100644
--- a/OpenSim/Region/OptionalModules/World/NPC/NPCModule.cs
+++ b/OpenSim/Region/OptionalModules/World/NPC/NPCModule.cs
@@ -88,22 +88,26 @@ namespace OpenSim.Region.OptionalModules.World.NPC
 
         public bool SetNPCAppearance(UUID agentId, AvatarAppearance appearance, Scene scene)
         {
-            ScenePresence sp = scene.GetScenePresence(agentId);
-            if (sp == null || sp.IsChildAgent)
+            ScenePresence npc = scene.GetScenePresence(agentId);
+            if (npc == null || npc.IsChildAgent)
                 return false;
 
             lock (m_avatars)
                 if (!m_avatars.ContainsKey(agentId))
                     return false;
 
-            // Delete existing sp attachments
-            scene.AttachmentsModule.DeleteAttachmentsFromScene(sp, false);
+            // Delete existing npc attachments
+            scene.AttachmentsModule.DeleteAttachmentsFromScene(npc, false);
 
-            // Set new sp appearance. Also sends to clients.
-            scene.RequestModuleInterface<IAvatarFactoryModule>().SetAppearance(sp, new AvatarAppearance(appearance, true));
+            // XXX: We can't just use IAvatarFactoryModule.SetAppearance() yet since it doesn't transfer attachments
+            AvatarAppearance npcAppearance = new AvatarAppearance(appearance, true);
+            npc.Appearance = npcAppearance;
             
-            // Rez needed sp attachments
-            scene.AttachmentsModule.RezAttachments(sp);
+            // Rez needed npc attachments
+            scene.AttachmentsModule.RezAttachments(npc);
+
+            IAvatarFactoryModule module = scene.RequestModuleInterface<IAvatarFactoryModule>();
+            module.SendAppearance(npc.UUID);
             
             return true;
         }
diff --git a/OpenSim/Region/OptionalModules/World/NPC/Tests/NPCModuleTests.cs b/OpenSim/Region/OptionalModules/World/NPC/Tests/NPCModuleTests.cs
index d507822..36e2d4a 100644
--- a/OpenSim/Region/OptionalModules/World/NPC/Tests/NPCModuleTests.cs
+++ b/OpenSim/Region/OptionalModules/World/NPC/Tests/NPCModuleTests.cs
@@ -139,7 +139,7 @@ namespace OpenSim.Region.OptionalModules.World.NPC.Tests
         }
 
         [Test]
-        public void TestAttachments()
+        public void TestCreateWithAttachments()
         {
             TestHelpers.InMethod();
 //            log4net.Config.XmlConfigurator.Configure();
@@ -179,6 +179,50 @@ namespace OpenSim.Region.OptionalModules.World.NPC.Tests
         }
 
         [Test]
+        public void TestLoadAppearance()
+        {
+            TestHelpers.InMethod();
+//            log4net.Config.XmlConfigurator.Configure();
+
+            UUID userId = TestHelpers.ParseTail(0x1);
+            UserAccountHelpers.CreateUserWithInventory(scene, userId);
+            ScenePresence sp = SceneHelpers.AddScenePresence(scene, userId);
+
+            INPCModule npcModule = scene.RequestModuleInterface<INPCModule>();
+            UUID npcId = npcModule.CreateNPC("John", "Smith", new Vector3(128, 128, 30), UUID.Zero, true, scene, sp.Appearance);
+
+            // Now add the attachment to the original avatar and use that to load a new appearance
+            // TODO: Could also run tests loading from a notecard though this isn't much different for our purposes here
+            UUID attItemId = TestHelpers.ParseTail(0x2);
+            UUID attAssetId = TestHelpers.ParseTail(0x3);
+            string attName = "att";
+
+            UserInventoryHelpers.CreateInventoryItem(scene, attName, attItemId, attAssetId, sp.UUID, InventoryType.Object);
+
+            am.RezSingleAttachmentFromInventory(sp, attItemId, (uint)AttachmentPoint.Chest);
+
+            npcModule.SetNPCAppearance(npcId, sp.Appearance, scene);
+
+            ScenePresence npc = scene.GetScenePresence(npcId);
+
+            // Check scene presence status
+            Assert.That(npc.HasAttachments(), Is.True);
+            List<SceneObjectGroup> attachments = npc.GetAttachments();
+            Assert.That(attachments.Count, Is.EqualTo(1));
+            SceneObjectGroup attSo = attachments[0];
+
+            // 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));
+
+            Assert.That(attSo.AttachmentPoint, Is.EqualTo((byte)AttachmentPoint.Chest));
+            Assert.That(attSo.IsAttachment);
+            Assert.That(attSo.UsesPhysics, Is.False);
+            Assert.That(attSo.IsTemporary, Is.False);
+            Assert.That(attSo.OwnerID, Is.EqualTo(npc.UUID));
+        }
+
+        [Test]
         public void TestMove()
         {
             TestHelpers.InMethod();