Remove unnecessary AgentCircuitData null check from Scene.AddNewClient().

The only caller is the LLUDP stack and this has to validate the UDP circuit itself, so we know that it exists.
This allows us to eliminate another null check elsewhere and simplifies the method contract

4 files changed, 10 insertions, 19 deletions

diff --git a/OpenSim/Framework/IScene.cs b/OpenSim/Framework/IScene.cs
index 6919c48..e0e023d 100644
--- a/OpenSim/Framework/IScene.cs
+++ b/OpenSim/Framework/IScene.cs
@@ -74,9 +74,7 @@ namespace OpenSim.Framework
         /// <param name="client"></param>
         /// <param name="type">The type of agent to add.</param>
         /// <returns>
-        /// The scene agent if the new client was added.
-        /// Null if the required scene agent already existed or no scene agent was added because the required client circuit doesn't exist.
-        /// </returns>
+        /// The scene agent if the new client was added or if an agent that already existed.</returns>
         ISceneAgent AddNewClient(IClientAPI client, PresenceType type);
 
         /// <summary>
diff --git a/OpenSim/Region/ClientStack/Linden/UDP/LLUDPServer.cs b/OpenSim/Region/ClientStack/Linden/UDP/LLUDPServer.cs
index 7db5f6b..4ab8fd0 100644
--- a/OpenSim/Region/ClientStack/Linden/UDP/LLUDPServer.cs
+++ b/OpenSim/Region/ClientStack/Linden/UDP/LLUDPServer.cs
@@ -897,12 +897,12 @@ namespace OpenSim.Region.ClientStack.LindenUDP
             IClientAPI client = AddNewClient((UseCircuitCodePacket)packet, remoteEndPoint);
 
             // Send ack straight away to let the viewer know that the connection is active.
+            // The client will be null if it already exists (e.g. if on a region crossing the client sends a use
+            // circuit code to the existing child agent.  This is not particularly obvious.
             SendAckImmediate(remoteEndPoint, packet.Header.Sequence);
 
-            // FIXME: Nasty - this is the only way we currently know if Scene.AddNewClient() failed to find a
-            // circuit and bombed out early.  That check might be pointless since authorization is established
-            // up here.
-            if (client != null && client.SceneAgent != null)
+            // We only want to send initial data to new clients, not ones which are being converted from child to root.
+            if (client != null)
                 client.SceneAgent.SendInitialDataToMe();
 
             //            m_log.DebugFormat(
diff --git a/OpenSim/Region/Framework/Scenes/Scene.cs b/OpenSim/Region/Framework/Scenes/Scene.cs
index d47536a..11505cc 100644
--- a/OpenSim/Region/Framework/Scenes/Scene.cs
+++ b/OpenSim/Region/Framework/Scenes/Scene.cs
@@ -2486,21 +2486,14 @@ namespace OpenSim.Region.Framework.Scenes
 
         #region Add/Remove Avatar Methods
 
-        /// <summary>
-        /// Add a new client and create a child scene presence for it.
-        /// </summary>
-        /// <param name="client"></param>
-        /// <param name="type">The type of agent to add.</param>
         public override ISceneAgent AddNewClient(IClientAPI client, PresenceType type)
         {
+            // Validation occurs in LLUDPServer
             AgentCircuitData aCircuit = m_authenticateHandler.GetAgentCircuitData(client.CircuitCode);
-            bool vialogin = false;
-
-            if (aCircuit == null) // no good, didn't pass NewUserConnection successfully
-                return null;
 
-            vialogin = (aCircuit.teleportFlags & (uint)Constants.TeleportFlags.ViaHGLogin) != 0 || 
-                       (aCircuit.teleportFlags & (uint)Constants.TeleportFlags.ViaLogin) != 0;
+            bool vialogin
+                = (aCircuit.teleportFlags & (uint)Constants.TeleportFlags.ViaHGLogin) != 0
+                    || (aCircuit.teleportFlags & (uint)Constants.TeleportFlags.ViaLogin) != 0;
 
             CheckHeartbeat();
 
diff --git a/OpenSim/Region/Framework/Scenes/ScenePresence.cs b/OpenSim/Region/Framework/Scenes/ScenePresence.cs
index 6463ab1..8824921 100644
--- a/OpenSim/Region/Framework/Scenes/ScenePresence.cs
+++ b/OpenSim/Region/Framework/Scenes/ScenePresence.cs
@@ -2548,7 +2548,7 @@ namespace OpenSim.Region.Framework.Scenes
             }
             
             // This agent just became root. We are going to tell everyone about it. The process of
-            // getting other avatars information was initiated in the constructor... don't do it 
+            // getting other avatars information was initiated elsewhere immediately after the child circuit connected... don't do it
             // again here... this comes after the cached appearance check because the avatars
             // appearance goes into the avatar update packet
             SendAvatarDataToAllAgents();