From 714db90832522d8035df744594afd07b227af232 Mon Sep 17 00:00:00 2001 From: "Justin Clark-Casey (justincc)" Date: Wed, 20 Jun 2012 22:46:01 +0100 Subject: [PATCH 1/7] refactor: use SOG.HasPrivateAttachmentPoint in SOP.SendTerseUpdateToClient() instead of attachmentpoint magic numbers. --- OpenSim/Region/Framework/Scenes/SceneObjectPart.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/OpenSim/Region/Framework/Scenes/SceneObjectPart.cs b/OpenSim/Region/Framework/Scenes/SceneObjectPart.cs index 9303ff4801..3d8135826b 100644 --- a/OpenSim/Region/Framework/Scenes/SceneObjectPart.cs +++ b/OpenSim/Region/Framework/Scenes/SceneObjectPart.cs @@ -4431,8 +4431,9 @@ namespace OpenSim.Region.Framework.Scenes if (ParentGroup.IsDeleted) return; - if (ParentGroup.IsAttachment && ((ParentGroup.RootPart != this) || - ((ParentGroup.AttachedAvatar != remoteClient.AgentId) && (ParentGroup.AttachmentPoint >= 31) && (ParentGroup.AttachmentPoint <= 38)))) + if (ParentGroup.IsAttachment + && (ParentGroup.RootPart != this + || ParentGroup.AttachedAvatar != remoteClient.AgentId && ParentGroup.HasPrivateAttachmentPoint)) return; // Causes this thread to dig into the Client Thread Data. From 68ea096f1b1276e09510633d48313d6e870d0d97 Mon Sep 17 00:00:00 2001 From: "Justin Clark-Casey (justincc)" Date: Wed, 20 Jun 2012 23:25:07 +0100 Subject: [PATCH 2/7] Use HasPrivateAttachmentPoint properties in SOG.DeleteGroupFromScene() instead of magic numbers --- OpenSim/Region/Framework/Scenes/SceneObjectGroup.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/OpenSim/Region/Framework/Scenes/SceneObjectGroup.cs b/OpenSim/Region/Framework/Scenes/SceneObjectGroup.cs index 619296ed61..18877c1e3b 100644 --- a/OpenSim/Region/Framework/Scenes/SceneObjectGroup.cs +++ b/OpenSim/Region/Framework/Scenes/SceneObjectGroup.cs @@ -1205,8 +1205,9 @@ namespace OpenSim.Region.Framework.Scenes part.ClearUpdateSchedule(); if (part == m_rootPart) { - if (!IsAttachment || (AttachedAvatar == avatar.ControllingClient.AgentId) || - (AttachmentPoint < 31) || (AttachmentPoint > 38)) + if (!IsAttachment + || AttachedAvatar == avatar.ControllingClient.AgentId + || !HasPrivateAttachmentPoint) avatar.ControllingClient.SendKillObject(m_regionHandle, new List { part.LocalId }); } } From 5709bed548aa4897d84bb616a2c344e158fe89ff Mon Sep 17 00:00:00 2001 From: "Justin Clark-Casey (justincc)" Date: Thu, 21 Jun 2012 02:09:14 +0100 Subject: [PATCH 3/7] Add state file location to errors logged when there's some issue with retrieving state (e.g. exceeds memory limit) --- .../Region/ScriptEngine/Shared/Instance/ScriptInstance.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/OpenSim/Region/ScriptEngine/Shared/Instance/ScriptInstance.cs b/OpenSim/Region/ScriptEngine/Shared/Instance/ScriptInstance.cs index 329e36131b..b8bcb35161 100644 --- a/OpenSim/Region/ScriptEngine/Shared/Instance/ScriptInstance.cs +++ b/OpenSim/Region/ScriptEngine/Shared/Instance/ScriptInstance.cs @@ -331,15 +331,15 @@ namespace OpenSim.Region.ScriptEngine.Shared.Instance else { m_log.ErrorFormat( - "[SCRIPT INSTANCE]: Unable to load script state from assembly {0}: Memory limit exceeded", - assembly); + "[SCRIPT INSTANCE]: Unable to load script state file {0} from assembly {1}: Memory limit exceeded", + savedState, assembly); } } catch (Exception e) { m_log.ErrorFormat( - "[SCRIPT INSTANCE]: Unable to load script state from assembly {0}. XML is {1}. Exception {2}{3}", - assembly, xml, e.Message, e.StackTrace); + "[SCRIPT INSTANCE]: Unable to load script state file {0} from assembly {1}. XML is {2}. Exception {3}{4}", + savedState, assembly, xml, e.Message, e.StackTrace); } } // else From afcabf5244415f2d78391cb03269dc9979b5b3d2 Mon Sep 17 00:00:00 2001 From: "Justin Clark-Casey (justincc)" Date: Thu, 21 Jun 2012 02:13:03 +0100 Subject: [PATCH 4/7] Retrigger build - last jenkins run was glitched --- OpenSim/Region/ScriptEngine/Shared/Instance/ScriptInstance.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/OpenSim/Region/ScriptEngine/Shared/Instance/ScriptInstance.cs b/OpenSim/Region/ScriptEngine/Shared/Instance/ScriptInstance.cs index b8bcb35161..8c53b1fb54 100644 --- a/OpenSim/Region/ScriptEngine/Shared/Instance/ScriptInstance.cs +++ b/OpenSim/Region/ScriptEngine/Shared/Instance/ScriptInstance.cs @@ -354,7 +354,8 @@ namespace OpenSim.Region.ScriptEngine.Shared.Instance public void Init() { - if (!m_startOnInit) return; + if (!m_startOnInit) + return; if (m_startedFromSavedState) { From d24122b706cb3ec4ac9c8d1efc30e511a07e1aa7 Mon Sep 17 00:00:00 2001 From: "Justin Clark-Casey (justincc)" Date: Thu, 21 Jun 2012 02:24:44 +0100 Subject: [PATCH 5/7] Add item id, name, prim name and id to log message when state exists but loading fails. Drop logging about memory limit exceeded to warn from error --- .../ScriptEngine/Shared/Instance/ScriptInstance.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/OpenSim/Region/ScriptEngine/Shared/Instance/ScriptInstance.cs b/OpenSim/Region/ScriptEngine/Shared/Instance/ScriptInstance.cs index 8c53b1fb54..5dfe58e2f9 100644 --- a/OpenSim/Region/ScriptEngine/Shared/Instance/ScriptInstance.cs +++ b/OpenSim/Region/ScriptEngine/Shared/Instance/ScriptInstance.cs @@ -330,16 +330,16 @@ namespace OpenSim.Region.ScriptEngine.Shared.Instance } else { - m_log.ErrorFormat( - "[SCRIPT INSTANCE]: Unable to load script state file {0} from assembly {1}: Memory limit exceeded", - savedState, assembly); + m_log.WarnFormat( + "[SCRIPT INSTANCE]: Unable to load script state file {0} for script {1} {2} in {3} {4} (assembly {5}). Memory limit exceeded", + savedState, ScriptName, ItemID, PrimName, ObjectID, assembly); } } catch (Exception e) { m_log.ErrorFormat( - "[SCRIPT INSTANCE]: Unable to load script state file {0} from assembly {1}. XML is {2}. Exception {3}{4}", - savedState, assembly, xml, e.Message, e.StackTrace); + "[SCRIPT INSTANCE]: Unable to load script state file {0} for script {1} {2} in {3} {4} (assembly {5}). XML is {6}. Exception {7}{8}", + savedState, ScriptName, ItemID, PrimName, ObjectID, assembly, xml, e.Message, e.StackTrace); } } // else From 06617ffd06c743cebffa768bc56a979f473b5b5b Mon Sep 17 00:00:00 2001 From: "Justin Clark-Casey (justincc)" Date: Fri, 22 Jun 2012 00:18:30 +0100 Subject: [PATCH 6/7] Add regression test for updating attachment position --- .../Tests/AttachmentsModuleTests.cs | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/OpenSim/Region/CoreModules/Avatar/Attachments/Tests/AttachmentsModuleTests.cs b/OpenSim/Region/CoreModules/Avatar/Attachments/Tests/AttachmentsModuleTests.cs index 5e89eec25d..94c00304de 100644 --- a/OpenSim/Region/CoreModules/Avatar/Attachments/Tests/AttachmentsModuleTests.cs +++ b/OpenSim/Region/CoreModules/Avatar/Attachments/Tests/AttachmentsModuleTests.cs @@ -303,6 +303,36 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments.Tests Assert.That(presence.Appearance.GetAttachpoint(attItemId), Is.EqualTo((int)AttachmentPoint.Chest)); } + [Test] + public void TestUpdateAttachmentPosition() + { + TestHelpers.InMethod(); + + 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 sp = SceneHelpers.AddScenePresence(scene, acd); + + SceneObjectGroup attSo = sp.GetAttachments()[0]; + + Vector3 newPosition = new Vector3(1, 2, 4); + + scene.SceneGraph.UpdatePrimGroupPosition(attSo.LocalId, newPosition, sp.ControllingClient); + + Assert.That(attSo.AbsolutePosition, Is.EqualTo(sp.AbsolutePosition)); + Assert.That(attSo.RootPart.AttachedPos, Is.EqualTo(newPosition)); + } + // I'm commenting this test because scene setup NEEDS InventoryService to // be non-null //[Test] From 798846c5b6c05f37c661dde70fb9aaf51306e407 Mon Sep 17 00:00:00 2001 From: "Justin Clark-Casey (justincc)" Date: Fri, 22 Jun 2012 00:39:41 +0100 Subject: [PATCH 7/7] refactor AttachmentsModule tests to use a common method for standard attachment item setup --- .../Tests/AttachmentsModuleTests.cs | 139 +++++++++--------- 1 file changed, 69 insertions(+), 70 deletions(-) diff --git a/OpenSim/Region/CoreModules/Avatar/Attachments/Tests/AttachmentsModuleTests.cs b/OpenSim/Region/CoreModules/Avatar/Attachments/Tests/AttachmentsModuleTests.cs index 94c00304de..8f4a807182 100644 --- a/OpenSim/Region/CoreModules/Avatar/Attachments/Tests/AttachmentsModuleTests.cs +++ b/OpenSim/Region/CoreModules/Avatar/Attachments/Tests/AttachmentsModuleTests.cs @@ -58,6 +58,21 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments.Tests private AttachmentsModule m_attMod; private ScenePresence m_presence; + /// + /// Standard user ID + /// + private UUID m_userId = TestHelpers.ParseTail(0x1); + +// /// +// /// Standard attachment item ID +// /// +// private UUID m_attItemId = TestHelpers.ParseTail(0x10); +// +// /// +// /// Standard attachment asset ID +// /// +// private UUID m_attAssetId = TestHelpers.ParseTail(0x11); + [TestFixtureSetUp] public void FixtureInit() { @@ -86,13 +101,27 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments.Tests } /// - /// Add the standard presence for a test. + /// Creates an attachment item in the given user's inventory. Does not attach. /// - private void AddPresence() + /// + /// A user with the given ID and an inventory must already exist. + /// + /// + /// The attachment item. + /// + /// + /// + /// + /// + private InventoryItemBase CreateAttachmentItem(UUID userId, string attName, int rawItemId, int rawAssetId) { - UUID userId = TestHelpers.ParseTail(0x1); - UserAccountHelpers.CreateUserWithInventory(scene, userId); - m_presence = SceneHelpers.AddScenePresence(scene, userId); + return UserInventoryHelpers.CreateInventoryItem( + scene, + attName, + TestHelpers.ParseTail(rawItemId), + TestHelpers.ParseTail(rawAssetId), + userId, + InventoryType.Object); } [Test] @@ -101,7 +130,9 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments.Tests TestHelpers.InMethod(); // TestHelpers.EnableLogging(); - AddPresence(); + UserAccountHelpers.CreateUserWithInventory(scene, m_userId); + m_presence = SceneHelpers.AddScenePresence(scene, m_userId); + string attName = "att"; SceneObjectGroup so = SceneHelpers.AddSceneObject(scene, attName, m_presence.UUID).ParentGroup; @@ -140,24 +171,20 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments.Tests TestHelpers.InMethod(); // log4net.Config.XmlConfigurator.Configure(); - AddPresence(); + UserAccountHelpers.CreateUserWithInventory(scene, m_userId); + m_presence = SceneHelpers.AddScenePresence(scene, m_userId); - UUID attItemId = TestHelpers.ParseTail(0x2); - UUID attAssetId = TestHelpers.ParseTail(0x3); - string attName = "att"; - - UserInventoryHelpers.CreateInventoryItem( - scene, attName, attItemId, attAssetId, m_presence.UUID, InventoryType.Object); + InventoryItemBase attItem = CreateAttachmentItem(m_userId, "att", 0x10, 0x20); m_attMod.RezSingleAttachmentFromInventory( - m_presence, attItemId, (uint)AttachmentPoint.Chest); + m_presence, attItem.ID, (uint)AttachmentPoint.Chest); // Check scene presence status Assert.That(m_presence.HasAttachments(), Is.True); List attachments = m_presence.GetAttachments(); Assert.That(attachments.Count, Is.EqualTo(1)); SceneObjectGroup attSo = attachments[0]; - Assert.That(attSo.Name, Is.EqualTo(attName)); + Assert.That(attSo.Name, Is.EqualTo(attItem.Name)); Assert.That(attSo.AttachmentPoint, Is.EqualTo((byte)AttachmentPoint.Chest)); Assert.That(attSo.IsAttachment); Assert.That(attSo.UsesPhysics, Is.False); @@ -165,7 +192,7 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments.Tests // Check appearance status Assert.That(m_presence.Appearance.GetAttachments().Count, Is.EqualTo(1)); - Assert.That(m_presence.Appearance.GetAttachpoint(attItemId), Is.EqualTo((int)AttachmentPoint.Chest)); + Assert.That(m_presence.Appearance.GetAttachpoint(attItem.ID), Is.EqualTo((int)AttachmentPoint.Chest)); } [Test] @@ -174,17 +201,14 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments.Tests TestHelpers.InMethod(); // log4net.Config.XmlConfigurator.Configure(); - AddPresence(); + UserAccountHelpers.CreateUserWithInventory(scene, m_userId); + m_presence = SceneHelpers.AddScenePresence(scene, m_userId); - UUID attItemId = TestHelpers.ParseTail(0x2); - UUID attAssetId = TestHelpers.ParseTail(0x3); - string attName = "att"; + InventoryItemBase attItem = CreateAttachmentItem(m_userId, "att", 0x10, 0x20); - UserInventoryHelpers.CreateInventoryItem( - scene, attName, attItemId, attAssetId, m_presence.UUID, InventoryType.Object); - - ISceneEntity so = m_attMod.RezSingleAttachmentFromInventory( - m_presence, attItemId, (uint)AttachmentPoint.Chest); + ISceneEntity so + = m_attMod.RezSingleAttachmentFromInventory( + m_presence, attItem.ID, (uint)AttachmentPoint.Chest); m_attMod.DetachSingleAttachmentToGround(m_presence, so.LocalId); // Check scene presence status @@ -196,7 +220,7 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments.Tests Assert.That(m_presence.Appearance.GetAttachments().Count, Is.EqualTo(0)); // Check item status - Assert.That(scene.InventoryService.GetItem(new InventoryItemBase(attItemId)), Is.Null); + Assert.That(scene.InventoryService.GetItem(new InventoryItemBase(attItem.ID)), Is.Null); // Check object in scene Assert.That(scene.GetSceneObjectGroup("att"), Is.Not.Null); @@ -208,18 +232,14 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments.Tests TestHelpers.InMethod(); // log4net.Config.XmlConfigurator.Configure(); - AddPresence(); + UserAccountHelpers.CreateUserWithInventory(scene, m_userId); + m_presence = SceneHelpers.AddScenePresence(scene, m_userId); - UUID attItemId = TestHelpers.ParseTail(0x2); - UUID attAssetId = TestHelpers.ParseTail(0x3); - string attName = "att"; - - UserInventoryHelpers.CreateInventoryItem( - scene, attName, attItemId, attAssetId, m_presence.UUID, InventoryType.Object); + InventoryItemBase attItem = CreateAttachmentItem(m_userId, "att", 0x10, 0x20); m_attMod.RezSingleAttachmentFromInventory( - m_presence, attItemId, (uint)AttachmentPoint.Chest); - m_attMod.DetachSingleAttachmentToInv(m_presence, attItemId); + m_presence, attItem.ID, (uint)AttachmentPoint.Chest); + m_attMod.DetachSingleAttachmentToInv(m_presence, attItem.ID); // Check status on scene presence Assert.That(m_presence.HasAttachments(), Is.False); @@ -227,7 +247,7 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments.Tests Assert.That(attachments.Count, Is.EqualTo(0)); // Check item status - Assert.That(m_presence.Appearance.GetAttachpoint(attItemId), Is.EqualTo(0)); + Assert.That(m_presence.Appearance.GetAttachpoint(attItem.ID), Is.EqualTo(0)); } /// @@ -239,17 +259,10 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments.Tests 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, m_userId); + InventoryItemBase attItem = CreateAttachmentItem(m_userId, "att", 0x10, 0x20); - UserAccountHelpers.CreateUserWithInventory(scene, userId); - InventoryItemBase attItem - = UserInventoryHelpers.CreateInventoryItem( - scene, attName, attItemId, attAssetId, userId, InventoryType.Object); - - AgentCircuitData acd = SceneHelpers.GenerateAgentData(userId); + AgentCircuitData acd = SceneHelpers.GenerateAgentData(m_userId); acd.Appearance = new AvatarAppearance(); acd.Appearance.SetAttachment((int)AttachmentPoint.Chest, attItem.ID, attItem.AssetID); ScenePresence presence = SceneHelpers.AddScenePresence(scene, acd); @@ -268,17 +281,10 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments.Tests 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, m_userId); + InventoryItemBase attItem = CreateAttachmentItem(m_userId, "att", 0x10, 0x20); - UserAccountHelpers.CreateUserWithInventory(scene, userId); - InventoryItemBase attItem - = UserInventoryHelpers.CreateInventoryItem( - scene, attName, attItemId, attAssetId, userId, InventoryType.Object); - - AgentCircuitData acd = SceneHelpers.GenerateAgentData(userId); + AgentCircuitData acd = SceneHelpers.GenerateAgentData(m_userId); acd.Appearance = new AvatarAppearance(); acd.Appearance.SetAttachment((int)AttachmentPoint.Chest, attItem.ID, attItem.AssetID); ScenePresence presence = SceneHelpers.AddScenePresence(scene, acd); @@ -288,7 +294,7 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments.Tests Assert.That(attachments.Count, Is.EqualTo(1)); SceneObjectGroup attSo = attachments[0]; - Assert.That(attSo.Name, Is.EqualTo(attName)); + Assert.That(attSo.Name, Is.EqualTo(attItem.Name)); Assert.That(attSo.AttachmentPoint, Is.EqualTo((byte)AttachmentPoint.Chest)); Assert.That(attSo.IsAttachment); Assert.That(attSo.UsesPhysics, Is.False); @@ -298,9 +304,9 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments.Tests List retreivedAttachments = presence.Appearance.GetAttachments(); Assert.That(retreivedAttachments.Count, Is.EqualTo(1)); Assert.That(retreivedAttachments[0].AttachPoint, Is.EqualTo((int)AttachmentPoint.Chest)); - Assert.That(retreivedAttachments[0].ItemID, Is.EqualTo(attItemId)); - Assert.That(retreivedAttachments[0].AssetID, Is.EqualTo(attAssetId)); - Assert.That(presence.Appearance.GetAttachpoint(attItemId), Is.EqualTo((int)AttachmentPoint.Chest)); + Assert.That(retreivedAttachments[0].ItemID, Is.EqualTo(attItem.ID)); + Assert.That(retreivedAttachments[0].AssetID, Is.EqualTo(attItem.AssetID)); + Assert.That(presence.Appearance.GetAttachpoint(attItem.ID), Is.EqualTo((int)AttachmentPoint.Chest)); } [Test] @@ -308,17 +314,10 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments.Tests { TestHelpers.InMethod(); - UUID userId = TestHelpers.ParseTail(0x1); - UUID attItemId = TestHelpers.ParseTail(0x2); - UUID attAssetId = TestHelpers.ParseTail(0x3); - string attName = "att"; + UserAccountHelpers.CreateUserWithInventory(scene, m_userId); + InventoryItemBase attItem = CreateAttachmentItem(m_userId, "att", 0x10, 0x20); - UserAccountHelpers.CreateUserWithInventory(scene, userId); - InventoryItemBase attItem - = UserInventoryHelpers.CreateInventoryItem( - scene, attName, attItemId, attAssetId, userId, InventoryType.Object); - - AgentCircuitData acd = SceneHelpers.GenerateAgentData(userId); + AgentCircuitData acd = SceneHelpers.GenerateAgentData(m_userId); acd.Appearance = new AvatarAppearance(); acd.Appearance.SetAttachment((int)AttachmentPoint.Chest, attItem.ID, attItem.AssetID); ScenePresence sp = SceneHelpers.AddScenePresence(scene, acd);