From e6d0dcd4e80275b96322eb10b31a2b339e4a2d17 Mon Sep 17 00:00:00 2001 From: Justin Clark-Casey (justincc) Date: Thu, 3 Apr 2014 00:19:53 +0100 Subject: Fix bug where crossing to a neighbouring region and back again would trigger an exception, and a second recross would stop the user moving until relog Also fixes an issue where sitting avatar counts became inaccurate after any cross. Part of the problem was due to cloning code using MemberwiseClone() but not resetting certain collection structures. Adds regression test for this case. In relation to http://opensimulator.org/mantis/view.php?id=7050 --- .../Region/Framework/Scenes/SceneObjectGroup.cs | 31 ++++-- OpenSim/Region/Framework/Scenes/SceneObjectPart.cs | 6 ++ OpenSim/Region/Framework/Scenes/ScenePresence.cs | 2 +- .../Scenes/Tests/SceneObjectCrossingTests.cs | 107 ++++++++++++++++++++- 4 files changed, 138 insertions(+), 8 deletions(-) (limited to 'OpenSim/Region') diff --git a/OpenSim/Region/Framework/Scenes/SceneObjectGroup.cs b/OpenSim/Region/Framework/Scenes/SceneObjectGroup.cs index b70e9df..4dc724d 100644 --- a/OpenSim/Region/Framework/Scenes/SceneObjectGroup.cs +++ b/OpenSim/Region/Framework/Scenes/SceneObjectGroup.cs @@ -552,12 +552,29 @@ namespace OpenSim.Region.Framework.Scenes av.IsInTransit = true; - CrossAgentToNewRegionDelegate d = entityTransfer.CrossAgentToNewRegionAsync; - d.BeginInvoke(av, val, destination, av.Flying, version, CrossAgentToNewRegionCompleted, d); + // A temporary measure to allow regression tests to work. + // Quite possibly, all BeginInvoke() calls should be replaced by Util.FireAndForget + // or similar since BeginInvoke() always uses the system threadpool to launch + // threads rather than any replace threadpool that we might be using. + if (Util.FireAndForgetMethod == FireAndForgetMethod.RegressionTest) + { + entityTransfer.CrossAgentToNewRegionAsync(av, val, destination, av.Flying, version); + CrossAgentToNewRegionCompleted(av); + } + else + { + CrossAgentToNewRegionDelegate d = entityTransfer.CrossAgentToNewRegionAsync; + d.BeginInvoke( + av, val, destination, av.Flying, version, + ar => CrossAgentToNewRegionCompleted(d.EndInvoke(ar)), null); + } } else + { m_log.DebugFormat("[SCENE OBJECT]: Crossing avatar alreasy in transit {0} to {1}", av.Name, val); + } } + avsToCross.Clear(); return; } @@ -630,11 +647,8 @@ namespace OpenSim.Region.Framework.Scenes set { RootPart.Velocity = value; } } - private void CrossAgentToNewRegionCompleted(IAsyncResult iar) + private void CrossAgentToNewRegionCompleted(ScenePresence agent) { - CrossAgentToNewRegionDelegate icon = (CrossAgentToNewRegionDelegate)iar.AsyncState; - ScenePresence agent = icon.EndInvoke(iar); - //// If the cross was successful, this agent is a child agent if (agent.IsChildAgent) { @@ -1698,10 +1712,15 @@ namespace OpenSim.Region.Framework.Scenes /// public SceneObjectGroup Copy(bool userExposed) { + // FIXME: This is dangerous since it's easy to forget to reset some references when necessary and end up + // with bugs that only occur in some circumstances (e.g. crossing between regions on the same simulator + // but not between regions on different simulators). Really, all copying should be done explicitly. SceneObjectGroup dupe = (SceneObjectGroup)MemberwiseClone(); + dupe.Backup = false; dupe.m_parts = new MapAndArray(); dupe.m_sittingAvatars = new List(); + dupe.m_linkedAvatars = new List(); dupe.CopyRootPart(m_rootPart, OwnerID, GroupID, userExposed); dupe.m_rootPart.LinkNum = m_rootPart.LinkNum; diff --git a/OpenSim/Region/Framework/Scenes/SceneObjectPart.cs b/OpenSim/Region/Framework/Scenes/SceneObjectPart.cs index db4e285..c06175e 100644 --- a/OpenSim/Region/Framework/Scenes/SceneObjectPart.cs +++ b/OpenSim/Region/Framework/Scenes/SceneObjectPart.cs @@ -1753,7 +1753,11 @@ namespace OpenSim.Region.Framework.Scenes /// public SceneObjectPart Copy(uint localID, UUID AgentID, UUID GroupID, int linkNum, bool userExposed) { + // FIXME: This is dangerous since it's easy to forget to reset some references when necessary and end up + // with bugs that only occur in some circumstances (e.g. crossing between regions on the same simulator + // but not between regions on different simulators). Really, all copying should be done explicitly. SceneObjectPart dupe = (SceneObjectPart)MemberwiseClone(); + dupe.m_shape = m_shape.Copy(); dupe.m_regionHandle = m_regionHandle; if (userExposed) @@ -1799,6 +1803,8 @@ namespace OpenSim.Region.Framework.Scenes Array.Copy(Shape.ExtraParams, extraP, extraP.Length); dupe.Shape.ExtraParams = extraP; + dupe.m_sittingAvatars = new HashSet(); + // safeguard actual copy is done in sog.copy dupe.KeyframeMotion = null; dupe.PayPrice = (int[])PayPrice.Clone(); diff --git a/OpenSim/Region/Framework/Scenes/ScenePresence.cs b/OpenSim/Region/Framework/Scenes/ScenePresence.cs index 080cdb4..6386a45 100644 --- a/OpenSim/Region/Framework/Scenes/ScenePresence.cs +++ b/OpenSim/Region/Framework/Scenes/ScenePresence.cs @@ -1023,6 +1023,7 @@ namespace OpenSim.Region.Framework.Scenes else { part.ParentGroup.AddAvatar(UUID); + part.AddSittingAvatar(UUID); if (part.SitTargetPosition != Vector3.Zero) part.SitTargetAvatar = UUID; // ParentPosition = part.GetWorldPosition(); @@ -2848,7 +2849,6 @@ namespace OpenSim.Region.Framework.Scenes part.ParentGroup.TriggerScriptChangedEvent(Changed.LINK); } - public void HandleAgentSit(IClientAPI remoteClient, UUID agentID) { if (IsChildAgent) diff --git a/OpenSim/Region/Framework/Scenes/Tests/SceneObjectCrossingTests.cs b/OpenSim/Region/Framework/Scenes/Tests/SceneObjectCrossingTests.cs index 4d07741..d65b0b6 100644 --- a/OpenSim/Region/Framework/Scenes/Tests/SceneObjectCrossingTests.cs +++ b/OpenSim/Region/Framework/Scenes/Tests/SceneObjectCrossingTests.cs @@ -26,10 +26,12 @@ */ using System; +using System.Collections.Generic; using Nini.Config; using NUnit.Framework; using OpenMetaverse; using OpenSim.Framework; +using OpenSim.Region.CoreModules.Framework; using OpenSim.Region.CoreModules.Framework.EntityTransfer; using OpenSim.Region.CoreModules.ServiceConnectorsOut.Simulation; using OpenSim.Region.CoreModules.World.Land; @@ -101,7 +103,110 @@ namespace OpenSim.Region.Framework.Scenes.Tests /// Test cross with no prim limit module. /// /// - /// XXX: This test may be better off in a specific PrimLimitsModuleTest class in optional module tests in the + /// Possibly this should belong in ScenePresenceCrossingTests, though here it is the object that is being moved + /// where the avatar is just a passenger. + /// + [Test] + public void TestCrossOnSameSimulatorWithSittingAvatar() + { + TestHelpers.InMethod(); +// TestHelpers.EnableLogging(); + + UUID userId = TestHelpers.ParseTail(0x1); + int sceneObjectIdTail = 0x2; + Vector3 so1StartPos = new Vector3(128, 10, 20); + + EntityTransferModule etmA = new EntityTransferModule(); + EntityTransferModule etmB = new EntityTransferModule(); + LocalSimulationConnectorModule lscm = new LocalSimulationConnectorModule(); + + IConfigSource config = new IniConfigSource(); + IConfig modulesConfig = config.AddConfig("Modules"); + modulesConfig.Set("EntityTransferModule", etmA.Name); + modulesConfig.Set("SimulationServices", lscm.Name); + IConfig entityTransferConfig = config.AddConfig("EntityTransfer"); + + // In order to run a single threaded regression test we do not want the entity transfer module waiting + // for a callback from the destination scene before removing its avatar data. + entityTransferConfig.Set("wait_for_callback", false); + + SceneHelpers sh = new SceneHelpers(); + TestScene sceneA = sh.SetupScene("sceneA", TestHelpers.ParseTail(0x100), 1000, 1000); + TestScene sceneB = sh.SetupScene("sceneB", TestHelpers.ParseTail(0x200), 1000, 999); + + SceneHelpers.SetupSceneModules(new Scene[] { sceneA, sceneB }, config, lscm); + SceneHelpers.SetupSceneModules(sceneA, config, new CapabilitiesModule(), etmA); + SceneHelpers.SetupSceneModules(sceneB, config, new CapabilitiesModule(), etmB); + + SceneObjectGroup so1 = SceneHelpers.AddSceneObject(sceneA, 1, userId, "", sceneObjectIdTail); + UUID so1Id = so1.UUID; + so1.AbsolutePosition = so1StartPos; + + AgentCircuitData acd = SceneHelpers.GenerateAgentData(userId); + TestClient tc = new TestClient(acd, sceneA); + List destinationTestClients = new List(); + EntityTransferHelpers.SetupInformClientOfNeighbourTriggersNeighbourClientCreate(tc, destinationTestClients); + + ScenePresence sp1SceneA = SceneHelpers.AddScenePresence(sceneA, tc, acd); + sp1SceneA.AbsolutePosition = so1StartPos; + sp1SceneA.HandleAgentRequestSit(sp1SceneA.ControllingClient, sp1SceneA.UUID, so1.UUID, Vector3.Zero); + + // Cross + sceneA.SceneGraph.UpdatePrimGroupPosition( + so1.LocalId, new Vector3(so1StartPos.X, so1StartPos.Y - 20, so1StartPos.Z), userId); + + SceneObjectGroup so1PostCross; + + { + ScenePresence sp1SceneAPostCross = sceneA.GetScenePresence(userId); + Assert.IsTrue(sp1SceneAPostCross.IsChildAgent, "sp1SceneAPostCross.IsChildAgent unexpectedly false"); + + ScenePresence sp1SceneBPostCross = sceneB.GetScenePresence(userId); + TestClient sceneBTc = ((TestClient)sp1SceneBPostCross.ControllingClient); + sceneBTc.CompleteMovement(); + + Assert.IsFalse(sp1SceneBPostCross.IsChildAgent, "sp1SceneAPostCross.IsChildAgent unexpectedly true"); + Assert.IsTrue(sp1SceneBPostCross.IsSatOnObject); + + Assert.IsNull(sceneA.GetSceneObjectGroup(so1Id), "uck"); + so1PostCross = sceneB.GetSceneObjectGroup(so1Id); + Assert.NotNull(so1PostCross); + Assert.AreEqual(1, so1PostCross.GetSittingAvatarsCount()); + Assert.AreEqual(1, so1PostCross.GetLinkedAvatars().Count); + } + + Vector3 so1PostCrossPos = so1PostCross.AbsolutePosition; + +// Console.WriteLine("CRISSCROSS"); + + // Recross + sceneB.SceneGraph.UpdatePrimGroupPosition( + so1PostCross.LocalId, new Vector3(so1PostCrossPos.X, so1PostCrossPos.Y + 20, so1PostCrossPos.Z), userId); + + { + ScenePresence sp1SceneBPostReCross = sceneB.GetScenePresence(userId); + Assert.IsTrue(sp1SceneBPostReCross.IsChildAgent, "sp1SceneBPostReCross.IsChildAgent unexpectedly false"); + + ScenePresence sp1SceneAPostReCross = sceneA.GetScenePresence(userId); + TestClient sceneATc = ((TestClient)sp1SceneAPostReCross.ControllingClient); + sceneATc.CompleteMovement(); + + Assert.IsFalse(sp1SceneAPostReCross.IsChildAgent, "sp1SceneAPostCross.IsChildAgent unexpectedly true"); + Assert.IsTrue(sp1SceneAPostReCross.IsSatOnObject); + + Assert.IsNull(sceneB.GetSceneObjectGroup(so1Id), "uck2"); + SceneObjectGroup so1PostReCross = sceneA.GetSceneObjectGroup(so1Id); + Assert.NotNull(so1PostReCross); + Assert.AreEqual(1, so1PostReCross.GetSittingAvatarsCount()); + Assert.AreEqual(1, so1PostReCross.GetLinkedAvatars().Count); + } + } + + /// + /// Test cross with no prim limit module. + /// + /// + /// XXX: This test may FCbe better off in a specific PrimLimitsModuleTest class in optional module tests in the /// future (though it is configured as active by default, so not really optional). /// [Test] -- cgit v1.1