On the later forms of teleport failure, tell the user if this was because viewer couldn't/didn't connect with destination or if destination didn't signal teleport completion.

Also adds regression test for the case where the viewer couldn't connect with the destination region.
Also refactoring of regression test support code associated with entity transfer in order to make this test possible and the code less obscure.

4 files changed, 150 insertions, 40 deletions

diff --git a/OpenSim/Region/CoreModules/Avatar/Attachments/Tests/AttachmentsModuleTests.cs b/OpenSim/Region/CoreModules/Avatar/Attachments/Tests/AttachmentsModuleTests.cs
index 4cd03da..c8c594d 100644
--- a/OpenSim/Region/CoreModules/Avatar/Attachments/Tests/AttachmentsModuleTests.cs
+++ b/OpenSim/Region/CoreModules/Avatar/Attachments/Tests/AttachmentsModuleTests.cs
@@ -829,7 +829,13 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments.Tests
                 sceneB, config, new CapabilitiesModule(), etmB, attModB, new BasicInventoryAccessModule());
 
             UserAccount ua1 = UserAccountHelpers.CreateUserWithInventory(sceneA, 0x1);
-            ScenePresence beforeTeleportSp = SceneHelpers.AddScenePresence(sceneA, ua1.PrincipalID, sh.SceneManager);
+
+            AgentCircuitData acd = SceneHelpers.GenerateAgentData(ua1.PrincipalID);
+            TestClient tc = new TestClient(acd, sceneA, sh.SceneManager);
+            List<TestClient> destinationTestClients = new List<TestClient>();
+            EntityTransferHelpers.SetUpInformClientOfNeighbour(tc, destinationTestClients);
+
+            ScenePresence beforeTeleportSp = SceneHelpers.AddScenePresence(sceneA, tc, acd, sh.SceneManager);
             beforeTeleportSp.AbsolutePosition = new Vector3(30, 31, 32);
 
             InventoryItemBase attItem = CreateAttachmentItem(sceneA, ua1.PrincipalID, "att", 0x10, 0x20);
@@ -848,7 +854,7 @@ namespace OpenSim.Region.CoreModules.Avatar.Attachments.Tests
                 teleportLookAt,
                 (uint)TeleportFlags.ViaLocation);
 
-            ((TestClient)beforeTeleportSp.ControllingClient).CompleteTeleportClientSide();
+            destinationTestClients[0].CompleteMovement();
 
             // Check attachments have made it into sceneB
             ScenePresence afterTeleportSceneBSp = sceneB.GetScenePresence(ua1.PrincipalID);
diff --git a/OpenSim/Region/CoreModules/Framework/EntityTransfer/EntityTransferModule.cs b/OpenSim/Region/CoreModules/Framework/EntityTransfer/EntityTransferModule.cs
index 9b1b69a..58a6654 100644
--- a/OpenSim/Region/CoreModules/Framework/EntityTransfer/EntityTransferModule.cs
+++ b/OpenSim/Region/CoreModules/Framework/EntityTransfer/EntityTransferModule.cs
@@ -517,12 +517,6 @@ namespace OpenSim.Region.CoreModules.Framework.EntityTransfer
                     "[ENTITY TRANSFER MODULE]: Failed validation of all attachments for teleport of {0} from {1} to {2}.  Continuing.",
                     sp.Name, sp.Scene.RegionInfo.RegionName, finalDestination.RegionName);
 
-//                if (!sp.ValidateAttachments())
-//                {
-//                    sp.ControllingClient.SendTeleportFailed("Inconsistent attachment state");
-//                    return;
-//                }
-
             string reason;
             string version;
             if (!Scene.SimulationService.QueryAccess(
@@ -583,6 +577,7 @@ namespace OpenSim.Region.CoreModules.Framework.EntityTransfer
             }
 
             // Let's create an agent there if one doesn't exist yet. 
+            // NOTE: logout will always be false for a non-HG teleport.
             bool logout = false;
             if (!CreateAgent(sp, reg, finalDestination, agentCircuit, teleportFlags, out reason, out logout))
             {
@@ -625,11 +620,13 @@ namespace OpenSim.Region.CoreModules.Framework.EntityTransfer
 
                 if (m_eqModule != null)
                 {
+                    // The EnableSimulator message makes the client establish a connection with the destination
+                    // simulator by sending the initial UseCircuitCode UDP packet to the destination containing the
+                    // correct circuit code.
                     m_eqModule.EnableSimulator(destinationHandle, endPoint, sp.UUID);
 
-                    // ES makes the client send a UseCircuitCode message to the destination, 
-                    // which triggers a bunch of things there.
-                    // So let's wait
+                    // XXX: Is this wait necessary?  We will always end up waiting on UpdateAgent for the destination
+                    // simulator to confirm that it has established communication with the viewer.
                     Thread.Sleep(200);
 
                     // At least on LL 3.3.4 for teleports between different regions on the same simulator this appears
@@ -640,6 +637,9 @@ namespace OpenSim.Region.CoreModules.Framework.EntityTransfer
                 }
                 else
                 {
+                    // XXX: This is a little misleading since we're information the client of its avatar destination,
+                    // which may or may not be a neighbour region of the source region.  This path is probably little
+                    // used anyway (with EQ being the one used).  But it is currently being used for test code.
                     sp.ControllingClient.InformClientOfNeighbour(destinationHandle, endPoint);
                 }
             }
@@ -657,14 +657,17 @@ namespace OpenSim.Region.CoreModules.Framework.EntityTransfer
 
             //sp.ControllingClient.SendTeleportProgress(teleportFlags, "Updating agent...");
 
+            // A common teleport failure occurs when we can send CreateAgent to the 
+            // destination region but the viewer cannot establish the connection (e.g. due to network issues between
+            // the viewer and the destination).  In this case, UpdateAgent timesout after 10 seconds, although then
+            // there's a further 10 second wait whilst we attempt to tell the destination to delete the agent in Fail().
             if (!UpdateAgent(reg, finalDestination, agent, sp))
             {
-                // Region doesn't take it
                 m_log.WarnFormat(
-                    "[ENTITY TRANSFER MODULE]: UpdateAgent failed on teleport of {0} to {1} from {2}.  Returning avatar to source region.",
+                    "[ENTITY TRANSFER MODULE]: UpdateAgent failed on teleport of {0} to {1} from {2}.  Keeping avatar in source region.",
                     sp.Name, finalDestination.RegionName, sp.Scene.RegionInfo.RegionName);
                 
-                Fail(sp, finalDestination, logout);
+                Fail(sp, finalDestination, logout, "Connection between viewer and destination region could not be established.");
                 return;
             }
 
@@ -705,7 +708,7 @@ namespace OpenSim.Region.CoreModules.Framework.EntityTransfer
                     "[ENTITY TRANSFER MODULE]: Teleport of {0} to {1} from {2} failed due to no callback from destination region.  Returning avatar to source region.",
                     sp.Name, finalDestination.RegionName, sp.Scene.RegionInfo.RegionName);
                 
-                Fail(sp, finalDestination, logout);
+                Fail(sp, finalDestination, logout, "Destination region did not signal teleport completion.");
                 return;
             }
 
@@ -788,12 +791,15 @@ namespace OpenSim.Region.CoreModules.Framework.EntityTransfer
         /// <param name='sp'></param>
         /// <param name='finalDestination'></param>
         /// <param name='logout'></param>
-        protected virtual void Fail(ScenePresence sp, GridRegion finalDestination, bool logout)
+        /// <param name='reason'>Human readable reason for teleport failure.  Will be sent to client.</param>
+        protected virtual void Fail(ScenePresence sp, GridRegion finalDestination, bool logout, string reason)
         {
             CleanupAbortedInterRegionTeleport(sp, finalDestination);
 
             sp.ControllingClient.SendTeleportFailed(
-                string.Format("Problems connecting to destination {0}", finalDestination.RegionName));
+                string.Format(
+                    "Problems connecting to destination {0}, reason: {1}", finalDestination.RegionName, reason));
+
             sp.Scene.EventManager.TriggerTeleportFail(sp.ControllingClient, logout);
         }
 
diff --git a/OpenSim/Region/Framework/Scenes/Tests/ScenePresenceCrossingTests.cs b/OpenSim/Region/Framework/Scenes/Tests/ScenePresenceCrossingTests.cs
index 81a2fcc..b67b8b9 100644
--- a/OpenSim/Region/Framework/Scenes/Tests/ScenePresenceCrossingTests.cs
+++ b/OpenSim/Region/Framework/Scenes/Tests/ScenePresenceCrossingTests.cs
@@ -66,7 +66,7 @@ namespace OpenSim.Region.Framework.Scenes.Tests
         public void TestCrossOnSameSimulator()
         {
             TestHelpers.InMethod();
-//            TestHelpers.EnableLogging();
+            TestHelpers.EnableLogging();
 
             UUID userId = TestHelpers.ParseTail(0x1);
 
@@ -94,7 +94,12 @@ namespace OpenSim.Region.Framework.Scenes.Tests
 //            SceneHelpers.SetupSceneModules(sceneA, config, new CapabilitiesModule(), etmA, eqmA);
             SceneHelpers.SetupSceneModules(sceneB, config, new CapabilitiesModule(), etmB);
 
-            ScenePresence originalSp = SceneHelpers.AddScenePresence(sceneA, userId, sh.SceneManager);
+            AgentCircuitData acd = SceneHelpers.GenerateAgentData(userId);
+            TestClient tc = new TestClient(acd, sceneA, sh.SceneManager);
+            List<TestClient> destinationTestClients = new List<TestClient>();
+            EntityTransferHelpers.SetUpInformClientOfNeighbour(tc, destinationTestClients);
+
+            ScenePresence originalSp = SceneHelpers.AddScenePresence(sceneA, tc, acd, sh.SceneManager);
             originalSp.AbsolutePosition = new Vector3(128, 32, 10);
 
 //            originalSp.Flying = true;
diff --git a/OpenSim/Region/Framework/Scenes/Tests/ScenePresenceTeleportTests.cs b/OpenSim/Region/Framework/Scenes/Tests/ScenePresenceTeleportTests.cs
index 8dd1f3d..d6bc313 100644
--- a/OpenSim/Region/Framework/Scenes/Tests/ScenePresenceTeleportTests.cs
+++ b/OpenSim/Region/Framework/Scenes/Tests/ScenePresenceTeleportTests.cs
@@ -26,7 +26,10 @@
  */
 
 using System;
-using System.Reflection;
+using System.Collections.Generic;
+using System.IO;
+using System.Net;
+using System.Text;
 using Nini.Config;
 using NUnit.Framework;
 using OpenMetaverse;
@@ -40,8 +43,6 @@ using OpenSim.Region.CoreModules.ServiceConnectorsOut.Simulation;
 using OpenSim.Region.CoreModules.World.Permissions;
 using OpenSim.Tests.Common;
 using OpenSim.Tests.Common.Mock;
-using System.IO;
-using System.Text;
 
 namespace OpenSim.Region.Framework.Scenes.Tests
 {
@@ -68,7 +69,7 @@ namespace OpenSim.Region.Framework.Scenes.Tests
         }
 
         [Test]
-        public void TestSameRegionTeleport()
+        public void TestSameRegion()
         {
             TestHelpers.InMethod();
 //            log4net.Config.XmlConfigurator.Configure();
@@ -106,10 +107,10 @@ namespace OpenSim.Region.Framework.Scenes.Tests
         }
 
         [Test]
-        public void TestSameSimulatorSeparatedRegionsTeleport()
+        public void TestSameSimulatorSeparatedRegions()
         {
             TestHelpers.InMethod();
-//            log4net.Config.XmlConfigurator.Configure();
+//            TestHelpers.EnableLogging();
 
             UUID userId = TestHelpers.ParseTail(0x1);
 
@@ -141,9 +142,8 @@ namespace OpenSim.Region.Framework.Scenes.Tests
             ScenePresence sp = SceneHelpers.AddScenePresence(sceneA, userId, sh.SceneManager);
             sp.AbsolutePosition = new Vector3(30, 31, 32);
 
-            // XXX: A very nasty hack to tell the client about the destination scene without having to crank the whole
-            // UDP stack (?)
-//            ((TestClient)sp.ControllingClient).TeleportTargetScene = sceneB;
+            List<TestClient> destinationTestClients = new List<TestClient>();
+            EntityTransferHelpers.SetUpInformClientOfNeighbour((TestClient)sp.ControllingClient, destinationTestClients);
 
             sceneA.RequestTeleportLocation(
                 sp.ControllingClient,
@@ -152,7 +152,10 @@ namespace OpenSim.Region.Framework.Scenes.Tests
                 teleportLookAt,
                 (uint)TeleportFlags.ViaLocation);
 
-            ((TestClient)sp.ControllingClient).CompleteTeleportClientSide();
+            // SetupInformClientOfNeighbour() will have handled the callback into the target scene to setup the child
+            // agent.  This call will now complete the movement of the user into the destination and upgrade the agent
+            // from child to root.
+            destinationTestClients[0].CompleteMovement();
 
             Assert.That(sceneA.GetScenePresence(userId), Is.Null);
 
@@ -177,7 +180,7 @@ namespace OpenSim.Region.Framework.Scenes.Tests
         /// Test teleport procedures when the target simulator returns false when queried about access.
         /// </summary>
         [Test]
-        public void TestSameSimulatorSeparatedRegionsQueryAccessFails()
+        public void TestSameSimulatorSeparatedRegions_DeniedOnQueryAccess()
         {
             TestHelpers.InMethod();
 //            TestHelpers.EnableLogging();
@@ -261,7 +264,7 @@ namespace OpenSim.Region.Framework.Scenes.Tests
         /// Test teleport procedures when the target simulator create agent step is refused.
         /// </summary>
         [Test]
-        public void TestSameSimulatorSeparatedRegionsCreateAgentFails()
+        public void TestSameSimulatorSeparatedRegions_DeniedOnCreateAgent()
         {
             TestHelpers.InMethod();
 //            TestHelpers.EnableLogging();
@@ -333,13 +336,101 @@ namespace OpenSim.Region.Framework.Scenes.Tests
 //            TestHelpers.DisableLogging();
         }
 
+        /// <summary>
+        /// Test teleport when the destination region does not process (or does not receive) the connection attempt
+        /// from the viewer.
+        /// </summary>
+        /// <remarks>
+        /// This could be quite a common case where the source region can connect to a remove destination region
+        /// (for CreateAgent) but the viewer cannot reach the destination region due to network issues.
+        /// </remarks>
         [Test]
-        public void TestSameSimulatorNeighbouringRegionsTeleport()
+        public void TestSameSimulatorSeparatedRegions_DestinationDidNotProcessViewerConnection()
         {
             TestHelpers.InMethod();
 //            TestHelpers.EnableLogging();
 
             UUID userId = TestHelpers.ParseTail(0x1);
+            Vector3 preTeleportPosition = new Vector3(30, 31, 32);
+
+            EntityTransferModule etmA = new EntityTransferModule();
+            EntityTransferModule etmB = new EntityTransferModule();
+
+            LocalSimulationConnectorModule lscm = new LocalSimulationConnectorModule();
+
+            IConfigSource config = new IniConfigSource();
+            config.AddConfig("Modules");
+            config.Configs["Modules"].Set("EntityTransferModule", etmA.Name);
+            config.Configs["Modules"].Set("SimulationServices", lscm.Name);
+
+            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.
+            config.Configs["EntityTransfer"].Set("wait_for_callback", false);
+
+//            config.AddConfig("Startup");
+//            config.Configs["Startup"].Set("serverside_object_permissions", true);
+
+            SceneHelpers sh = new SceneHelpers();
+            TestScene sceneA = sh.SetupScene("sceneA", TestHelpers.ParseTail(0x100), 1000, 1000);
+            TestScene sceneB = sh.SetupScene("sceneB", TestHelpers.ParseTail(0x200), 1002, 1000);
+
+            SceneHelpers.SetupSceneModules(sceneA, config, etmA );
+
+            // We need to set up the permisions module on scene B so that our later use of agent limit to deny
+            // QueryAccess won't succeed anyway because administrators are always allowed in and the default
+            // IsAdministrator if no permissions module is present is true.
+            SceneHelpers.SetupSceneModules(sceneB, config, new object[] { new PermissionsModule(), etmB });
+
+            // Shared scene modules
+            SceneHelpers.SetupSceneModules(new Scene[] { sceneA, sceneB }, config, lscm);
+
+            Vector3 teleportPosition = new Vector3(10, 11, 12);
+            Vector3 teleportLookAt = new Vector3(20, 21, 22);
+
+            ScenePresence sp = SceneHelpers.AddScenePresence(sceneA, userId, sh.SceneManager);
+            sp.AbsolutePosition = preTeleportPosition;
+
+            sceneA.RequestTeleportLocation(
+                sp.ControllingClient,
+                sceneB.RegionInfo.RegionHandle,
+                teleportPosition,
+                teleportLookAt,
+                (uint)TeleportFlags.ViaLocation);
+
+            // FIXME: Not setting up InformClientOfNeighbour on the TestClient means that it does not initiate 
+            // communication with the destination region.  But this is a very non-obvious way of doing it - really we
+            // should be forced to expicitly set this up.
+
+            Assert.That(sceneB.GetScenePresence(userId), Is.Null);
+
+            ScenePresence sceneASp = sceneA.GetScenePresence(userId);
+            Assert.That(sceneASp, Is.Not.Null);
+            Assert.That(sceneASp.Scene.RegionInfo.RegionName, Is.EqualTo(sceneA.RegionInfo.RegionName));
+            Assert.That(sceneASp.AbsolutePosition, Is.EqualTo(preTeleportPosition));
+
+            Assert.That(sceneA.GetRootAgentCount(), Is.EqualTo(1));
+            Assert.That(sceneA.GetChildAgentCount(), Is.EqualTo(0));
+            Assert.That(sceneB.GetRootAgentCount(), Is.EqualTo(0));
+            Assert.That(sceneB.GetChildAgentCount(), Is.EqualTo(0));
+
+            // TODO: Add assertions to check correct circuit details in both scenes.
+
+            // Lookat is sent to the client only - sp.Lookat does not yield the same thing (calculation from camera
+            // position instead).
+//            Assert.That(sp.Lookat, Is.EqualTo(teleportLookAt));
+
+//            TestHelpers.DisableLogging();
+        }
+
+        [Test]
+        public void TestSameSimulatorNeighbouringRegions()
+        {
+            TestHelpers.InMethod();
+            TestHelpers.EnableLogging();
+
+            UUID userId = TestHelpers.ParseTail(0x1);
 
             EntityTransferModule etmA = new EntityTransferModule();
             EntityTransferModule etmB = new EntityTransferModule();
@@ -366,10 +457,14 @@ namespace OpenSim.Region.Framework.Scenes.Tests
             Vector3 teleportPosition = new Vector3(10, 11, 12);
             Vector3 teleportLookAt = new Vector3(20, 21, 22);
 
-            ScenePresence originalSp = SceneHelpers.AddScenePresence(sceneA, userId, sh.SceneManager);
-            originalSp.AbsolutePosition = new Vector3(30, 31, 32);
+            AgentCircuitData acd = SceneHelpers.GenerateAgentData(userId);
+            TestClient tc = new TestClient(acd, sceneA, sh.SceneManager);
+            List<TestClient> destinationTestClients = new List<TestClient>();
+            EntityTransferHelpers.SetUpInformClientOfNeighbour(tc, destinationTestClients);
+
+            ScenePresence beforeSceneASp = SceneHelpers.AddScenePresence(sceneA, tc, acd, sh.SceneManager);
+            beforeSceneASp.AbsolutePosition = new Vector3(30, 31, 32);
 
-            ScenePresence beforeSceneASp = sceneA.GetScenePresence(userId);
             Assert.That(beforeSceneASp, Is.Not.Null);
             Assert.That(beforeSceneASp.IsChildAgent, Is.False);
 
@@ -377,10 +472,8 @@ namespace OpenSim.Region.Framework.Scenes.Tests
             Assert.That(beforeSceneBSp, Is.Not.Null);
             Assert.That(beforeSceneBSp.IsChildAgent, Is.True);
 
-            // XXX: A very nasty hack to tell the client about the destination scene without having to crank the whole
-            // UDP stack (?)
-//            ((TestClient)beforeSceneASp.ControllingClient).TeleportTargetScene = sceneB;
-
+            // In this case, we will not receieve a second InformClientOfNeighbour since the viewer already knows
+            // about the neighbour region it is teleporting to.
             sceneA.RequestTeleportLocation(
                 beforeSceneASp.ControllingClient,
                 sceneB.RegionInfo.RegionHandle,
@@ -388,7 +481,7 @@ namespace OpenSim.Region.Framework.Scenes.Tests
                 teleportLookAt,
                 (uint)TeleportFlags.ViaLocation);
 
-            ((TestClient)beforeSceneASp.ControllingClient).CompleteTeleportClientSide();
+            destinationTestClients[0].CompleteMovement();
 
             ScenePresence afterSceneASp = sceneA.GetScenePresence(userId);
             Assert.That(afterSceneASp, Is.Not.Null);