refactor: Stop passing both IClientAPI and agentID to friend event listeners, these are redundant. Replace a few magic numbers with FriendRights enum already used elsewhere.

4 files changed, 58 insertions, 27 deletions

diff --git a/OpenSim/Region/ClientStack/Linden/UDP/LLClientView.cs b/OpenSim/Region/ClientStack/Linden/UDP/LLClientView.cs
index 9899669..f79597e 100644
--- a/OpenSim/Region/ClientStack/Linden/UDP/LLClientView.cs
+++ b/OpenSim/Region/ClientStack/Linden/UDP/LLClientView.cs
@@ -5784,7 +5784,6 @@ namespace OpenSim.Region.ClientStack.LindenUDP
             // My guess is this is the folder to stick the calling card into
             List<UUID> callingCardFolders = new List<UUID>();
 
-            UUID agentID = afriendpack.AgentData.AgentID;
             UUID transactionID = afriendpack.TransactionBlock.TransactionID;
 
             for (int fi = 0; fi < afriendpack.FolderData.Length; fi++)
@@ -5795,10 +5794,10 @@ namespace OpenSim.Region.ClientStack.LindenUDP
             FriendActionDelegate handlerApproveFriendRequest = OnApproveFriendRequest;
             if (handlerApproveFriendRequest != null)
             {
-                handlerApproveFriendRequest(this, agentID, transactionID, callingCardFolders);
+                handlerApproveFriendRequest(this, transactionID, callingCardFolders);
             }
-            return true;
 
+            return true;
         }
 
         private bool HandlerDeclineFriendship(IClientAPI sender, Packet Pack)
@@ -5817,7 +5816,6 @@ namespace OpenSim.Region.ClientStack.LindenUDP
             if (OnDenyFriendRequest != null)
             {
                 OnDenyFriendRequest(this,
-                                    dfriendpack.AgentData.AgentID,
                                     dfriendpack.TransactionBlock.TransactionID,
                                     null);
             }
@@ -5837,14 +5835,14 @@ namespace OpenSim.Region.ClientStack.LindenUDP
             }
             #endregion
 
-            UUID listOwnerAgentID = tfriendpack.AgentData.AgentID;
             UUID exFriendID = tfriendpack.ExBlock.OtherID;
             FriendshipTermination TerminateFriendshipHandler = OnTerminateFriendship;
             if (TerminateFriendshipHandler != null)
             {
-                TerminateFriendshipHandler(this, listOwnerAgentID, exFriendID);
+                TerminateFriendshipHandler(this, exFriendID);
                 return true;
             }
+
             return false;
         }
 
@@ -11162,12 +11160,13 @@ namespace OpenSim.Region.ClientStack.LindenUDP
                     return true;
             }
             #endregion
+
             GrantUserFriendRights GrantUserRightsHandler = OnGrantUserRights;
             if (GrantUserRightsHandler != null)
                 GrantUserRightsHandler(this,
-                    GrantUserRights.AgentData.AgentID,
                     GrantUserRights.Rights[0].AgentRelated,
                     GrantUserRights.Rights[0].RelatedRights);
+
             return true;
         }
 
diff --git a/OpenSim/Region/CoreModules/Avatar/Friends/FriendsModule.cs b/OpenSim/Region/CoreModules/Avatar/Friends/FriendsModule.cs
index 0c83679..86e04b9 100644
--- a/OpenSim/Region/CoreModules/Avatar/Friends/FriendsModule.cs
+++ b/OpenSim/Region/CoreModules/Avatar/Friends/FriendsModule.cs
@@ -258,7 +258,7 @@ namespace OpenSim.Region.CoreModules.Avatar.Friends
             client.OnInstantMessage += OnInstantMessage;
             client.OnApproveFriendRequest += OnApproveFriendRequest;
             client.OnDenyFriendRequest += OnDenyFriendRequest;
-            client.OnTerminateFriendship += (thisClient, agentID, exfriendID) => RemoveFriendship(thisClient, exfriendID);
+            client.OnTerminateFriendship += RemoveFriendship;
             client.OnGrantUserRights += OnGrantUserRights;
 
             // We need to cache information for child agents as well as root agents so that friend edit/move/delete
@@ -355,14 +355,13 @@ namespace OpenSim.Region.CoreModules.Avatar.Friends
 
             // Send the friends online
             List<UUID> online = GetOnlineFriends(agentID);
-            if (online.Count > 0)
-            {
-                m_log.DebugFormat(
-                    "[FRIENDS MODULE]: User {0} in region {1} has {2} friends online",
-                    client.Name, client.Scene.RegionInfo.RegionName, online.Count);
 
+//            m_log.DebugFormat(
+//                "[FRIENDS MODULE]: User {0} in region {1} has {2} friends online",
+//                client.Name, client.Scene.RegionInfo.RegionName, online.Count);
+
+            if (online.Count > 0)
                 client.SendAgentOnline(online.ToArray());
-            }
 
             // Send outstanding friendship offers
             List<string> outstanding = new List<string>();
@@ -495,7 +494,7 @@ namespace OpenSim.Region.CoreModules.Avatar.Friends
                 List<FriendInfo> friendList = new List<FriendInfo>();
                 foreach (FriendInfo fi in friends)
                 {
-                    if (((fi.MyFlags & 1) != 0) && (fi.TheirFlags != -1))
+                    if (((fi.MyFlags & (int)FriendRights.CanSeeOnline) != 0) && (fi.TheirFlags != -1))
                         friendList.Add(fi);
                 }
 
@@ -614,7 +613,7 @@ namespace OpenSim.Region.CoreModules.Avatar.Friends
             return (account == null) ? "Unknown" : account.FirstName + " " + account.LastName;
         }
 
-        protected virtual void OnApproveFriendRequest(IClientAPI client, UUID agentID, UUID friendID, List<UUID> callingCardFolders)
+        protected virtual void OnApproveFriendRequest(IClientAPI client, UUID friendID, List<UUID> callingCardFolders)
         {
             m_log.DebugFormat("[FRIENDS]: {0} accepted friendship from {1}", client.AgentId, friendID);
 
@@ -659,18 +658,18 @@ namespace OpenSim.Region.CoreModules.Avatar.Friends
             }
         }
 
-        private void OnDenyFriendRequest(IClientAPI client, UUID agentID, UUID friendID, List<UUID> callingCardFolders)
+        private void OnDenyFriendRequest(IClientAPI client, UUID friendID, List<UUID> callingCardFolders)
         {
-            m_log.DebugFormat("[FRIENDS]: {0} denied friendship to {1}", agentID, friendID);
+            m_log.DebugFormat("[FRIENDS]: {0} denied friendship to {1}", client.AgentId, friendID);
 
-            DeleteFriendship(agentID, friendID);
+            DeleteFriendship(client.AgentId, friendID);
 
             //
             // Notify the friend
             //
 
             // Try local
-            if (LocalFriendshipDenied(agentID, client.Name, friendID))
+            if (LocalFriendshipDenied(client.AgentId, client.Name, friendID))
                 return;
 
             PresenceInfo[] friendSessions = PresenceService.GetAgents(new string[] { friendID.ToString() });
@@ -681,7 +680,7 @@ namespace OpenSim.Region.CoreModules.Avatar.Friends
                 {
                     GridRegion region = GridService.GetRegionByUUID(m_Scenes[0].RegionInfo.ScopeID, friendSession.RegionID);
                     if (region != null)
-                        m_FriendsSimConnector.FriendshipDenied(region, agentID, client.Name, friendID);
+                        m_FriendsSimConnector.FriendshipDenied(region, client.AgentId, client.Name, friendID);
                     else
                         m_log.WarnFormat("[FRIENDS]: Could not find region {0} in locating {1}", friendSession.RegionID, friendID);
                 }
@@ -718,11 +717,15 @@ namespace OpenSim.Region.CoreModules.Avatar.Friends
             }            
         }
 
-        private void OnGrantUserRights(IClientAPI remoteClient, UUID requester, UUID target, int rights)
+        private void OnGrantUserRights(IClientAPI remoteClient, UUID target, int rights)
         {
-            m_log.DebugFormat("[FRIENDS MODULE]: User {0} changing rights to {1} for friend {2}", requester, rights, target);
+            UUID requester = remoteClient.AgentId;
 
-            FriendInfo[] friends = GetFriends(remoteClient.AgentId);
+            m_log.DebugFormat(
+                "[FRIENDS MODULE]: User {0} changing rights to {1} for friend {2}",
+                requester, rights, target);
+
+            FriendInfo[] friends = GetFriends(requester);
             if (friends.Length == 0)
             {
                 return;
@@ -769,7 +772,9 @@ namespace OpenSim.Region.CoreModules.Avatar.Friends
                 }
             }
             else
+            {
                 m_log.DebugFormat("[FRIENDS MODULE]: friend {0} not found for {1}", target, requester);
+            }
         }
 
         protected virtual FriendInfo GetFriend(FriendInfo[] friends, UUID friendID)
@@ -813,7 +818,6 @@ namespace OpenSim.Region.CoreModules.Avatar.Friends
                     ccm.CreateCallingCard(friendID, userID, UUID.Zero);
                 }
 
-
                 // Update the local cache
                 RecacheFriends(friendClient);
 
diff --git a/OpenSim/Region/CoreModules/Avatar/Friends/HGFriendsModule.cs b/OpenSim/Region/CoreModules/Avatar/Friends/HGFriendsModule.cs
index e50a84a..b666dae 100644
--- a/OpenSim/Region/CoreModules/Avatar/Friends/HGFriendsModule.cs
+++ b/OpenSim/Region/CoreModules/Avatar/Friends/HGFriendsModule.cs
@@ -105,12 +105,12 @@ namespace OpenSim.Region.CoreModules.Avatar.Friends
 
         #endregion
 
-        protected override void OnApproveFriendRequest(IClientAPI client, UUID agentID, UUID friendID, List<UUID> callingCardFolders)
+        protected override void OnApproveFriendRequest(IClientAPI client, UUID friendID, List<UUID> callingCardFolders)
         {
             // Update the local cache. Yes, we need to do it right here
             // because the HGFriendsService placed something on the DB
             // from under the sim
-            base.OnApproveFriendRequest(client, agentID, friendID, callingCardFolders);
+            base.OnApproveFriendRequest(client, friendID, callingCardFolders);
         }
 
         protected override bool CacheFriends(IClientAPI client)
diff --git a/OpenSim/Region/CoreModules/Avatar/Friends/Tests/FriendModuleTests.cs b/OpenSim/Region/CoreModules/Avatar/Friends/Tests/FriendModuleTests.cs
index 34c68cc..94a78cb 100644
--- a/OpenSim/Region/CoreModules/Avatar/Friends/Tests/FriendModuleTests.cs
+++ b/OpenSim/Region/CoreModules/Avatar/Friends/Tests/FriendModuleTests.cs
@@ -120,6 +120,34 @@ namespace OpenSim.Region.CoreModules.Avatar.Friends.Tests
             Assert.That(((TestClient)sp1Redux.ControllingClient).ReceivedOnlineNotifications.Count, Is.EqualTo(0));
         }
 
+//        [Test]
+//        public void TestLoginWithOnlineFriends()
+//        {
+//            TestHelpers.InMethod();
+//            log4net.Config.XmlConfigurator.Configure();
+//
+//            UUID user1Id = TestHelpers.ParseTail(0x1);
+//            UUID user2Id = TestHelpers.ParseTail(0x2);
+//
+////            UserAccountHelpers.CreateUserWithInventory(m_scene, user1Id);
+////            UserAccountHelpers.CreateUserWithInventory(m_scene, user2Id);
+////
+////            m_fm.AddFriendship(user1Id, user2Id);
+//
+//            ScenePresence sp1 = SceneHelpers.AddScenePresence(m_scene, user1Id);
+//            SceneHelpers.AddScenePresence(m_scene, user2Id);
+//
+//            m_fm.AddFriendship(sp1.ControllingClient, user2Id);
+////            m_fm.LocalGrantRights
+//
+//            m_scene.RemoveClient(sp1.UUID, true);
+//
+//            ScenePresence sp1Redux = SceneHelpers.AddScenePresence(m_scene, user1Id);
+//
+//            Assert.That(((TestClient)sp1Redux.ControllingClient).ReceivedOfflineNotifications.Count, Is.EqualTo(0));
+//            Assert.That(((TestClient)sp1Redux.ControllingClient).ReceivedOnlineNotifications.Count, Is.EqualTo(1));
+//        }
+
         [Test]
         public void TestAddFriendshipWhileOnline()
         {