fix: notauthority da mode urpc sending to self (#3000)

* fix

NotAuthority should not include the service observer.

* fix

ProxyRpcTargetGroup should not send anything if there is no one to send to.

* style

Adding comments about why this was changed

* test

adding partial validation of this fix where it still has to be manually tested with a service connection.

* update

updating the changelog entry.

Author: NoelStephensUnity

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -12,6 +12,9 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 ### Fixed
 
+- Fixed issue where `NotAuthorityTarget` would include the service observer in the list of targets to send the RPC to as opposed to excluding the service observer as it should. (#3000)
+- Fixed issue where `ProxyRpcTargetGroup` could attempt to send a message if there were no targets to send to. (#3000)
+
 ### Changed
 
 

com.unity.netcode.gameobjects/Runtime/Messaging/RpcTargets/NotAuthorityRpcTarget.cs

@@ -29,6 +29,12 @@ internal override void Send(NetworkBehaviour behaviour, ref RpcMessage message,
                         {
                             continue;
                         }
+
+                        // The CMB-Service holds ID 0 and should not be added to the targets
+                        if (clientId == NetworkManager.ServerClientId && m_NetworkManager.CMBServiceConnection)
+                        {
+                            continue;
+                        }
                         m_GroupSendTarget.Add(clientId);
                     }
                 }
@@ -41,6 +47,12 @@ internal override void Send(NetworkBehaviour behaviour, ref RpcMessage message,
                             continue;
                         }
 
+                        // The CMB-Service holds ID 0 and should not be added to the targets
+                        if (clientId == NetworkManager.ServerClientId && m_NetworkManager.CMBServiceConnection)
+                        {
+                            continue;
+                        }
+
                         if (clientId == m_NetworkManager.LocalClientId)
                         {
                             m_LocalSendRpcTarget.Send(behaviour, ref message, delivery, rpcParams);

com.unity.netcode.gameobjects/Runtime/Messaging/RpcTargets/ProxyRpcTargetGroup.cs

@@ -17,6 +17,11 @@ internal class ProxyRpcTargetGroup : BaseRpcTarget, IDisposable, IGroupRpcTarget
 
         internal override void Send(NetworkBehaviour behaviour, ref RpcMessage message, NetworkDelivery delivery, RpcParams rpcParams)
         {
+            // If there are no targets then don't attempt to send anything.
+            if (TargetClientIds.Length == 0 && Ids.Count == 0)
+            {
+                return;
+            }
             var proxyMessage = new ProxyMessage { Delivery = delivery, TargetClientIds = TargetClientIds.AsArray(), WrappedMessage = message };
 #if DEVELOPMENT_BUILD || UNITY_EDITOR || UNITY_MP_TOOLS_NET_STATS_MONITOR_ENABLED_IN_RELEASE
             var size =

com.unity.netcode.gameobjects/Tests/Runtime/DistributedAuthority/RpcProxyMessageTesting.cs

@@ -0,0 +1,97 @@
+using System.Collections;
+using System.Collections.Generic;
+using System.Text;
+using NUnit.Framework;
+using Unity.Netcode.TestHelpers.Runtime;
+
+namespace Unity.Netcode.RuntimeTests
+{
+    /// <summary>
+    /// This test validates PR-3000 where it would invoke
+    /// TODO:
+    /// We really need to get the service running during tests
+    /// so we can validate these issues. While this test does
+    /// partially validate it we still need to manually validate
+    /// with a service connection.
+    /// </summary>
+    [TestFixture(HostOrServer.Host)]
+    [TestFixture(HostOrServer.DAHost)]
+    public class RpcProxyMessageTesting : NetcodeIntegrationTest
+    {
+        protected override int NumberOfClients => 2;
+
+        private List<RpcProxyText> m_ProxyTestInstances = new List<RpcProxyText>();
+
+        private StringBuilder m_ValidationLogger = new StringBuilder();
+
+        public RpcProxyMessageTesting(HostOrServer hostOrServer) : base(hostOrServer) { }
+
+        protected override IEnumerator OnSetup()
+        {
+            m_ProxyTestInstances.Clear();
+            return base.OnSetup();
+        }
+
+        protected override void OnCreatePlayerPrefab()
+        {
+            m_PlayerPrefab.AddComponent<RpcProxyText>();
+            base.OnCreatePlayerPrefab();
+        }
+
+
+        private bool ValidateRpcProxyRpcs()
+        {
+            m_ValidationLogger.Clear();
+            foreach (var proxy in m_ProxyTestInstances)
+            {
+                if (proxy.ReceivedRpc.Count < NumberOfClients)
+                {
+                    m_ValidationLogger.AppendLine($"Not all clients received RPC from Client-{proxy.OwnerClientId}!");
+                }
+                foreach (var clientId in proxy.ReceivedRpc)
+                {
+                    if (clientId == proxy.OwnerClientId)
+                    {
+                        m_ValidationLogger.AppendLine($"Client-{proxy.OwnerClientId} sent itself an Rpc!");
+                    }
+                }
+            }
+            return m_ValidationLogger.Length == 0;
+        }
+
+
+        public IEnumerator ProxyDoesNotInvokeOnSender()
+        {
+            m_ProxyTestInstances.Add(m_ServerNetworkManager.LocalClient.PlayerObject.GetComponent<RpcProxyText>());
+            foreach (var client in m_ClientNetworkManagers)
+            {
+                m_ProxyTestInstances.Add(client.LocalClient.PlayerObject.GetComponent<RpcProxyText>());
+            }
+
+            foreach (var clientProxyTest in m_ProxyTestInstances)
+            {
+                clientProxyTest.SendToEveryOneButMe();
+            }
+
+            yield return WaitForConditionOrTimeOut(ValidateRpcProxyRpcs);
+            AssertOnTimeout(m_ValidationLogger.ToString());
+        }
+
+        public class RpcProxyText : NetworkBehaviour
+        {
+            public List<ulong> ReceivedRpc = new List<ulong>();
+
+            public void SendToEveryOneButMe()
+            {
+                var baseTarget = NetworkManager.DistributedAuthorityMode ? RpcTarget.NotAuthority : RpcTarget.NotMe;
+                TestRpc(baseTarget);
+            }
+
+            [Rpc(SendTo.SpecifiedInParams)]
+            private void TestRpc(RpcParams rpcParams = default)
+            {
+                ReceivedRpc.Add(rpcParams.Receive.SenderClientId);
+            }
+        }
+    }
+}