On a new client circuit, send the initial reply ack to let the client know it's live before sending other data.

This means that avatar/appearance data of other avatars and scene objects for a client will be sent after the ack rather than possibly before.
This may stop some avatars appearing grey on login.
This introduces a new OpenSim.Framework.ISceneAgent to accompany the existing OpenSim.Framework.ISceneObject and ISceneEntity
This allows IClientAPI to handle this as it can't reference OpenSim.Region.Framework.Interfaces

OpenSim/Framework/IClientAPI.cs

@@ -702,6 +702,12 @@ namespace OpenSim.Framework
 
         UUID AgentId { get; }
 
+        /// <summary>
+        /// The scene agent for this client.  This will only be set if the client has an agent in a scene (i.e. if it
+        /// is connected).
+        /// </summary>
+        ISceneAgent SceneAgent { get; }
+
         UUID SessionId { get; }
 
         UUID SecureSessionId { get; }

OpenSim/Framework/IScene.cs

@@ -68,12 +68,16 @@ namespace OpenSim.Framework
         event restart OnRestart;
 
         /// <summary>
-        /// Register the new client with the scene.  The client starts off as a child agent - the later agent crossing
+        /// Add a new client and create a presence for it.  All clients except initial login clients will starts off as a child agent
-        /// will promote it to a root agent.
+        /// - the later agent crossing will promote it to a root agent.
         /// </summary>
         /// <param name="client"></param>
         /// <param name="type">The type of agent to add.</param>
-        void AddNewClient(IClientAPI client, PresenceType type);
+        /// <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>
+        ISceneAgent AddNewClient(IClientAPI client, PresenceType type);
 
         /// <summary>
         /// Remove the given client from the scene.

OpenSim/Framework/ISceneAgent.cs

@@ -0,0 +1,68 @@
+/*
+ * Copyright (c) Contributors, http://opensimulator.org/
+ * See CONTRIBUTORS.TXT for a full list of copyright holders.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above copyright
+ *       notice, this list of conditions and the following disclaimer in the
+ *       documentation and/or other materials provided with the distribution.
+ *     * Neither the name of the OpenSimulator Project nor the
+ *       names of its contributors may be used to endorse or promote products
+ *       derived from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE DEVELOPERS ``AS IS'' AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+ * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL THE CONTRIBUTORS BE LIABLE FOR ANY
+ * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+using System;
+
+namespace OpenSim.Framework
+{
+    /// <summary>
+    /// An agent in the scene.
+    /// </summary>
+    /// <remarks>
+    /// Interface is a work in progress.  Please feel free to add other required properties and methods.
+    /// </remarks>
+    public interface ISceneAgent : ISceneEntity
+    {
+        /// <value>
+        /// The client controlling this presence
+        /// </value>
+        IClientAPI ControllingClient { get; }
+
+        /// <summary>
+        /// What type of presence is this?  User, NPC, etc.
+        /// </summary>
+        PresenceType PresenceType { get; }
+
+        /// <summary>
+        /// Avatar appearance data.
+        /// </summary>
+        /// <remarks>
+        // Because appearance setting is in a module, we actually need
+        // to give it access to our appearance directly, otherwise we
+        // get a synchronization issue.
+        /// </remarks>
+        AvatarAppearance Appearance { get; set; }
+
+        /// <summary>
+        /// Send initial scene data to the client controlling this agent
+        /// </summary>
+        /// <remarks>
+        /// This includes scene object data and the appearance data of other avatars.
+        /// </remarks>
+        void SendInitialDataToMe();
+    }
+}

OpenSim/Region/ClientStack/Linden/UDP/LLClientView.cs

@@ -379,6 +379,7 @@ namespace OpenSim.Region.ClientStack.LindenUDP
             set { m_startpos = value; }
         }
         public UUID AgentId { get { return m_agentId; } }
+        public ISceneAgent SceneAgent { get; private set; }
         public UUID ActiveGroupId { get { return m_activeGroupID; } }
         public string ActiveGroupName { get { return m_activeGroupName; } }
         public ulong ActiveGroupPowers { get { return m_activeGroupPowers; } }
@@ -508,6 +509,7 @@ namespace OpenSim.Region.ClientStack.LindenUDP
 
             // Remove ourselves from the scene
             m_scene.RemoveClient(AgentId, true);
+            SceneAgent = null;
 
             // We can't reach into other scenes and close the connection
             // We need to do this over grid communications
@@ -687,7 +689,7 @@ namespace OpenSim.Region.ClientStack.LindenUDP
 
         public virtual void Start()
         {
-            m_scene.AddNewClient(this, PresenceType.User);
+            SceneAgent = m_scene.AddNewClient(this, PresenceType.User);
 
             RefreshGroupMembership();
         }

OpenSim/Region/ClientStack/Linden/UDP/LLUDPServer.cs

@@ -894,11 +894,17 @@ namespace OpenSim.Region.ClientStack.LindenUDP
             IPEndPoint remoteEndPoint = (IPEndPoint)buffer.RemoteEndPoint;
 
             // Begin the process of adding the client to the simulator
-            AddNewClient((UseCircuitCodePacket)packet, remoteEndPoint);
+            IClientAPI client = AddNewClient((UseCircuitCodePacket)packet, remoteEndPoint);
 
-            // Send ack
+            // Send ack straight away to let the viewer know that the connection is active.
             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)
+                client.SceneAgent.SendInitialDataToMe();
+
             //            m_log.DebugFormat(
 //                "[LLUDPSERVER]: Handling UseCircuitCode request from {0} took {1}ms", 
 //                buffer.RemoteEndPoint, (DateTime.Now - startTime).Milliseconds);
@@ -933,7 +939,15 @@ namespace OpenSim.Region.ClientStack.LindenUDP
             return sessionInfo.Authorised;
         }
 
-        private void AddNewClient(UseCircuitCodePacket useCircuitCode, IPEndPoint remoteEndPoint)
+        /// <summary>
+        /// Add a new client.
+        /// </summary>
+        /// <param name="useCircuitCode"></param>
+        /// <param name="remoteEndPoint"></param>
+        /// <returns>
+        /// The client that was added or null if the client failed authorization or already existed.
+        /// </returns>
+        private IClientAPI AddNewClient(UseCircuitCodePacket useCircuitCode, IPEndPoint remoteEndPoint)
         {
             UUID agentID = useCircuitCode.CircuitCode.ID;
             UUID sessionID = useCircuitCode.CircuitCode.SessionID;
@@ -942,7 +956,7 @@ namespace OpenSim.Region.ClientStack.LindenUDP
             AuthenticateResponse sessionInfo;
             if (IsClientAuthorized(useCircuitCode, out sessionInfo))
             {
-                AddClient(circuitCode, agentID, sessionID, remoteEndPoint, sessionInfo);
+                return AddClient(circuitCode, agentID, sessionID, remoteEndPoint, sessionInfo);
             }
             else
             {
@@ -950,38 +964,44 @@ namespace OpenSim.Region.ClientStack.LindenUDP
                 m_log.WarnFormat(
                     "[LLUDPSERVER]: Connection request for client {0} connecting with unnotified circuit code {1} from {2}",
                     useCircuitCode.CircuitCode.ID, useCircuitCode.CircuitCode.Code, remoteEndPoint);
+
+                return null;
             }
         }
 
-        protected virtual void AddClient(uint circuitCode, UUID agentID, UUID sessionID, IPEndPoint remoteEndPoint, AuthenticateResponse sessionInfo)
+        /// <summary>
+        /// Add a client.
+        /// </summary>
+        /// <param name="circuitCode"></param>
+        /// <param name="agentID"></param>
+        /// <param name="sessionID"></param>
+        /// <param name="remoteEndPoint"></param>
+        /// <param name="sessionInfo"></param>
+        /// <returns>The client if it was added.  Null if the client already existed.</returns>
+        protected virtual IClientAPI AddClient(
+            uint circuitCode, UUID agentID, UUID sessionID, IPEndPoint remoteEndPoint, AuthenticateResponse sessionInfo)
         {
+            IClientAPI client = null;
+
             // In priciple there shouldn't be more than one thread here, ever.
             // But in case that happens, we need to synchronize this piece of code
             // because it's too important
             lock (this) 
             {
-                IClientAPI existingClient;
+                if (!m_scene.TryGetClient(agentID, out client))
-
-                if (!m_scene.TryGetClient(agentID, out existingClient))
                 {
-                    // Create the LLUDPClient
                     LLUDPClient udpClient = new LLUDPClient(this, ThrottleRates, m_throttle, circuitCode, agentID, remoteEndPoint, m_defaultRTO, m_maxRTO);
-                    // Create the LLClientView
+
-                    LLClientView client = new LLClientView(remoteEndPoint, m_scene, this, udpClient, sessionInfo, agentID, sessionID, circuitCode);
+                    client = new LLClientView(remoteEndPoint, m_scene, this, udpClient, sessionInfo, agentID, sessionID, circuitCode);
                     client.OnLogout += LogoutHandler;
 
-                    client.DisableFacelights = m_disableFacelights;
+                    ((LLClientView)client).DisableFacelights = m_disableFacelights;
 
-                    // Start the IClientAPI
                     client.Start();
-
-                }
-                else
-                {
-                    m_log.WarnFormat("[LLUDPSERVER]: Ignoring a repeated UseCircuitCode from {0} at {1} for circuit {2}",
-                        existingClient.AgentId, remoteEndPoint, circuitCode);
                 }
             }
+
+            return client;
         }
 
         private void RemoveClient(LLUDPClient udpClient)

OpenSim/Region/ClientStack/Linden/UDP/Tests/MockScene.cs

@@ -53,9 +53,12 @@ namespace OpenSim.Region.ClientStack.LindenUDP.Tests
         public override void Update() {}
         public override void LoadWorldMap() {}
         
-        public override void AddNewClient(IClientAPI client, PresenceType type)
+        public override ISceneAgent AddNewClient(IClientAPI client, PresenceType type)
         {
             client.OnObjectName += RecordObjectNameCall;
+
+            // FIXME
+            return null;
         }
         
         public override void RemoveClient(UUID agentID, bool someReason) {}

OpenSim/Region/Framework/Interfaces/IScenePresence.cs

@@ -38,28 +38,8 @@ namespace OpenSim.Region.Framework.Interfaces
     /// <remarks>
     /// Interface is a work in progress.  Please feel free to add other required properties and methods.
     /// </remarks>
-    public interface IScenePresence : ISceneEntity
+    public interface IScenePresence : ISceneAgent
     {
-        /// <value>
-        /// The client controlling this presence
-        /// </value>
-        IClientAPI ControllingClient { get; }
-
-        /// <summary>
-        /// What type of presence is this?  User, NPC, etc.
-        /// </summary>
-        PresenceType PresenceType { get; }
-
-        /// <summary>
-        /// Avatar appearance data.
-        /// </summary>
-        /// <remarks>
-        // Because appearance setting is in a module, we actually need
-        // to give it access to our appearance directly, otherwise we
-        // get a synchronization issue.
-        /// </remarks>
-        AvatarAppearance Appearance { get; set; }
-
         /// <summary>
         /// The AttachmentsModule synchronizes on this to avoid race conditions between commands to add and remove attachments.
         /// </summary>

OpenSim/Region/Framework/Scenes/Scene.cs

@@ -2491,13 +2491,13 @@ namespace OpenSim.Region.Framework.Scenes
         /// </summary>
         /// <param name="client"></param>
         /// <param name="type">The type of agent to add.</param>
-        public override void AddNewClient(IClientAPI client, PresenceType type)
+        public override ISceneAgent AddNewClient(IClientAPI client, PresenceType type)
         {
             AgentCircuitData aCircuit = m_authenticateHandler.GetAgentCircuitData(client.CircuitCode);
             bool vialogin = false;
 
             if (aCircuit == null) // no good, didn't pass NewUserConnection successfully
-                return;
+                return null;
 
             vialogin = (aCircuit.teleportFlags & (uint)Constants.TeleportFlags.ViaHGLogin) != 0 || 
                        (aCircuit.teleportFlags & (uint)Constants.TeleportFlags.ViaLogin) != 0;
@@ -2552,16 +2552,7 @@ namespace OpenSim.Region.Framework.Scenes
             if (vialogin)
                 EventManager.TriggerOnClientLogin(client);
 
-            // Send all scene object to the new client
+            return sp;
-            Util.FireAndForget(delegate
-            {
-                EntityBase[] entities = Entities.GetEntities();
-                foreach(EntityBase e in entities)
-                {
-                    if (e != null && e is SceneObjectGroup)
-                        ((SceneObjectGroup)e).SendFullUpdateToClient(client);
-                }
-            });
         }
 
         /// <summary>

OpenSim/Region/Framework/Scenes/SceneBase.cs

@@ -175,7 +175,7 @@ namespace OpenSim.Region.Framework.Scenes
 
         #region Add/Remove Agent/Avatar
 
-        public abstract void AddNewClient(IClientAPI client, PresenceType type);
+        public abstract ISceneAgent AddNewClient(IClientAPI client, PresenceType type);
         public abstract void RemoveClient(UUID agentID, bool closeChildAgents);
 
         public bool TryGetScenePresence(UUID agentID, out object scenePresence)

OpenSim/Region/Framework/Scenes/ScenePresence.cs

@@ -782,15 +782,6 @@ namespace OpenSim.Region.Framework.Scenes
 
             AdjustKnownSeeds();
 
-            // we created a new ScenePresence (a new child agent) in a fresh region.
-            // Request info about all the (root) agents in this region
-            // Note: This won't send data *to* other clients in that region (children don't send)
-
-// MIC: This gets called again in CompleteMovement
-            // SendInitialFullUpdateToAllClients();
-            SendOtherAgentsAvatarDataToMe();
-            SendOtherAgentsAppearanceToMe();
-
             RegisterToEvents();
             SetDirectionVectors();
 
@@ -1191,9 +1182,9 @@ namespace OpenSim.Region.Framework.Scenes
         {
 //            DateTime startTime = DateTime.Now;
             
-            m_log.DebugFormat(
+//            m_log.DebugFormat(
-                "[SCENE PRESENCE]: Completing movement of {0} into region {1}", 
+//                "[SCENE PRESENCE]: Completing movement of {0} into region {1}", 
-                client.Name, Scene.RegionInfo.RegionName);
+//                client.Name, Scene.RegionInfo.RegionName);
 
             Vector3 look = Velocity;
             if ((look.X == 0) && (look.Y == 0) && (look.Z == 0))
@@ -1225,7 +1216,7 @@ namespace OpenSim.Region.Framework.Scenes
             //m_log.DebugFormat("[SCENE PRESENCE] Completed movement");
 
             ControllingClient.MoveAgentIntoRegion(m_scene.RegionInfo, AbsolutePosition, look);
-            SendInitialData();
+            ValidateAndSendAppearanceAndAgentData();
 
             // Create child agents in neighbouring regions
             if (openChildAgents && !IsChildAgent)
@@ -2512,11 +2503,31 @@ namespace OpenSim.Region.Framework.Scenes
             ControllingClient.SendCoarseLocationUpdate(avatarUUIDs, coarseLocations);
         }
 
+        public void SendInitialDataToMe()
+        {
+            // we created a new ScenePresence (a new child agent) in a fresh region.
+            // Request info about all the (root) agents in this region
+            // Note: This won't send data *to* other clients in that region (children don't send)
+            SendOtherAgentsAvatarDataToMe();
+            SendOtherAgentsAppearanceToMe();
+
+            // Send all scene object to the new client
+            Util.FireAndForget(delegate
+            {
+                EntityBase[] entities = Scene.Entities.GetEntities();
+                foreach(EntityBase e in entities)
+                {
+                    if (e != null && e is SceneObjectGroup)
+                        ((SceneObjectGroup)e).SendFullUpdateToClient(ControllingClient);
+                }
+            });
+        }
+
         /// <summary>
         /// Do everything required once a client completes its movement into a region and becomes
         /// a root agent.
         /// </summary>
-        private void SendInitialData()
+        private void ValidateAndSendAppearanceAndAgentData()
         {
             //m_log.DebugFormat("[SCENE PRESENCE] SendInitialData: {0} ({1})", Name, UUID);
             // Moved this into CompleteMovement to ensure that Appearance is initialized before

OpenSim/Region/OptionalModules/Agent/InternetRelayClientView/Server/IRCClientView.cs

@@ -55,6 +55,8 @@ namespace OpenSim.Region.OptionalModules.Agent.InternetRelayClientView.Server
 
         private UUID m_agentID = UUID.Random();
 
+        public ISceneAgent SceneAgent { get; private set; }
+
         private string m_username;
         private string m_nick;
 
@@ -547,6 +549,7 @@ namespace OpenSim.Region.OptionalModules.Agent.InternetRelayClientView.Server
 
             m_connected = false;
             m_client.Close();
+            SceneAgent = null;
         }
 
         public UUID SessionId
@@ -890,12 +893,11 @@ namespace OpenSim.Region.OptionalModules.Agent.InternetRelayClientView.Server
 
         public void Start()
         {
-            Scene.AddNewClient(this, PresenceType.User);
+            SceneAgent = m_scene.AddNewClient(this, PresenceType.User);
 
             // Mimicking LLClientView which gets always set appearance from client.
-            Scene scene = (Scene)Scene;
             AvatarAppearance appearance;
-            scene.GetAvatarAppearance(this, out appearance);
+            m_scene.GetAvatarAppearance(this, out appearance);
             OnSetAppearance(this, appearance.Texture, (byte[])appearance.VisualParams.Clone());
         }
 

OpenSim/Region/OptionalModules/World/NPC/NPCAvatar.cs

@@ -43,7 +43,6 @@ namespace OpenSim.Region.OptionalModules.World.NPC
         private readonly UUID m_uuid = UUID.Random();
         private readonly Scene m_scene;
 
-
         public NPCAvatar(string firstname, string lastname, Vector3 position, Scene scene)
         {
             m_firstname = firstname;
@@ -57,6 +56,8 @@ namespace OpenSim.Region.OptionalModules.World.NPC
             get { return m_scene; }
         }
 
+        public ISceneAgent SceneAgent { get { throw new NotImplementedException(); } }
+
         public void Say(string message)
         {
             SendOnChatFromClient(message, ChatTypeEnum.Say);
@@ -841,6 +842,8 @@ namespace OpenSim.Region.OptionalModules.World.NPC
 
         public void Start()
         {
+            // We never start the client, so always fail.
+            throw new NotImplementedException();
         }
         
         public void Stop()

OpenSim/Tests/Common/Mock/TestClient.cs

@@ -54,7 +54,7 @@ namespace OpenSim.Tests.Common.Mock
         public Scene TeleportTargetScene;
         private TestClient TeleportSceneClient;
 
-        private IScene m_scene;
+        private Scene m_scene;
 
         // Properties so that we can get at received data for test purposes
         public List<UUID> ReceivedOfflineNotifications { get; private set; }
@@ -324,6 +324,8 @@ namespace OpenSim.Tests.Common.Mock
         /// </value>
         private UUID m_agentId;
 
+        public ISceneAgent SceneAgent { get { throw new NotImplementedException(); } }
+
         /// <value>
         /// The last caps seed url that this client was given.
         /// </value>
@@ -437,7 +439,7 @@ namespace OpenSim.Tests.Common.Mock
         /// </summary>
         /// <param name="agentData"></param>
         /// <param name="scene"></param>
-        public TestClient(AgentCircuitData agentData, IScene scene)
+        public TestClient(AgentCircuitData agentData, Scene scene)
         {
             m_agentId = agentData.AgentID;
             m_firstName = agentData.firstname;
@@ -899,6 +901,7 @@ namespace OpenSim.Tests.Common.Mock
 
         public void Start()
         {
+            throw new NotImplementedException();
         }
 
         public void Stop()