feat: Added a universal RPC attribute [MTT-7482] [MTT-7578] [MTT-7579] [MTT-7580] [MTT-7581] by ShadauxCat · Pull Request #2762 · Unity-Technologies/com.unity.netcode.gameobjects

ShadauxCat · 2023-11-14T19:28:59Z

The new attribute is more configurable at both compile time and runtime than the existing ClientRpc and ServerRpc attributes, and also includes support for clients sending to themselves, and for optionally delaying the invocation of local RPCs until the start of the next frame to enable mutually recursive client/server RPCs to function on a host the same as on other clients (as well as mutually recursive Owner/NotOwner RPCs and other similar patterns).

This attribute supersedes ClientRpc and ServerRpc and will be the new default recommendation for creating RPCs. ClientRpc and ServerRpc will eventually be deprecated in favor of the new [Rpc] attribute.

By necessity of the feature, this also adds NetworkManager.PeerClientIds to allow clients to know the IDs of other connected clients, as well as NetworkManager.OnPeerConnectedCallback and NetworkManager.OnPeerDisconnectCallback events that are fired when this list is updated via messages from the server.

resolves #2718
resolves #2615
resolves #2504
resolves #2465
resolves #2448
resolves #2331
resolves #2326

Changelog

Added: Added a new RPC attribute, which is simply Rpc.
- This is a generic attribute that can perform the functions of both Server and Client RPCs, as well as enabling client-to-client RPCs. Includes several default targets: Server, NotServer, Owner, NotOwner, Me, NotMe, ClientsAndHost, and Everyone. Runtime overrides are available for any of these targets, as well as for sending to a specific ID or groups of IDs.
- This attribute also includes the ability to defer RPCs that are sent to the local process to the start of the next frame instead of executing them immediately, treating them as if they had gone across the network. The default behavior is to execute immediately.
- This attribute effectively replaces ServerRpc and ClientRpc. ServerRpc and ClientRpc remain in their existing forms for backward compatibility, but Rpc will be the recommended and most supported option.
Added: NetworkManager.OnConnectionEvent as a unified connection event callback to notify clients and servers of all client connections and disconnections within the session
Added: NetworkManager.ServerIsHost and NetworkBehaviour.ServerIsHost to allow a client to tell if it is connected to a host or to a dedicated server
Changed: NetworkManager.ConnectedClientsIds is now accessible on the client side and will contain the list of all clients in the session, including the host client if the server is operating in host mode

Testing and Documentation

Includes unit tests.
Includes integration tests.
Includes documentation for previously-undocumented public API entry points.

The new attribute is more configurable at both compile time and runtime than the existing ClientRpc and ServerRpc attributes, and also includes support for clients sending to themselves, and for optionally delaying the invocation of local RPCs until the start of the next frame to enable mutually recursive client/server RPCs to function on a host the same as on other clients (as well as mutually recursive Owner/NotOwner RPCs and other similar patterns). This attribute supersedes ClientRpc and ServerRpc and will be the new default recommendation for creating RPCs. ClientRpc and ServerRpc will eventually be deprecated in favor of the new [Rpc] attribute.

…nged files.

samlucas-unity

None of the comments are things which need changing. Mostly asking for context or minor suggestions.

samlucas-unity · 2023-11-15T11:13:07Z

                // processed before the object is fully spawned. This must be the last thing done in the spawn process.
                if (triggers.TryGetValue(key, out var triggerInfo))
                {
+                    triggers.Remove(key);


Why the move? (for context not that I think it shouldn't move :) )

If the handler for a trigger tried to add a new trigger of the same type, the Remove() after executing the trigger would just delete it and the trigger would never actually get executed.

samlucas-unity · 2023-11-15T11:32:59Z

+        {
+            if (m_UnderlyingTarget == null)
+            {
+                if (behaviour.NetworkManager.ConnectionManager.PeerClientIds.Contains(NetworkManager.ServerClientId))


Whats the difference here between Everyone and NotServer. Is the default to not send to self? Why does Everyone not exclude the local client (NotMe)? Oh is this for a host game then so in a dedicated server the ServerClientId will be null or is it just the server id. Then this check if the server counts as a peer as in a host game so in that case the server is a client Oh its in the name ClientAndHost, so this is only to Clients and in a locally hosted game (not dedicated server) that host too.

Its clear now, maybe slap a comment on the if check to say its checking for host session?

Yeah, if the server is a host, then ClientsAndHost means everyone. If the server is a dedicated server, then the server gets excluded. This is basically "send to everyone that runs any kind of client logic", which hosts do but dedicated servers don't.

- Replaced OnPeerConnectedCallback and OnPeerDisconnectCallback with unified OnConnectionEvent

NoelStephensUnity · 2023-11-16T22:16:20Z

+  - This is a generic attribute that can perform the functions of both Server and Client RPCs, as well as enabling client-to-client RPCs. Includes several default targets: `Server`, `NotServer`, `Owner`, `NotOwner`, `Me`, `NotMe`, `ClientsAndHost`, and `Everyone`. Runtime overrides are available for any of these targets, as well as for sending to a specific ID or groups of IDs.
+  - This attribute also includes the ability to defer RPCs that are sent to the local process to the start of the next frame instead of executing them immediately, treating them as if they had gone across the network. The default behavior is to execute immediately.
+  - This attribute effectively replaces `ServerRpc` and `ClientRpc`. `ServerRpc` and `ClientRpc` remain in their existing forms for backward compatibility, but `Rpc` will be the recommended and most supported option.
+- Added `NetworkManager.OnConnectionEvent` as a unified connection event callback to notify clients and servers of all client connections and disconnections within the session (#2762)


NoelStephensUnity

Looks Good To Me!

Awesome!
(Some minor nit-picky suggestions on very minor adjustments made.)

NoelStephensUnity · 2023-11-16T22:36:02Z

            }
 #endif

-


You could remove the file from this PR by adding the CR/LF back. (nit-picky of me)

When I save it in Rider, it just removes the CR/LF again. 😔

- Improved test coverage

zachstronaut · 2023-11-27T23:09:22Z

VERY excited for this, thank you @ShadauxCat

zachstronaut · 2023-11-29T22:54:58Z

I wanted to test this, so opened this repository's testproject in Unity 2022.3.10f1 on Windows.

I tried adding a generic [Rpc] to GrabbableBall.cs

    [Rpc(SendTo.Everyone)]
    private void TestRpc()
    {
        Debug.Log("TestRpc");
    }

The first thing I noticed is that [Rpc] without arguments is not available.

When I ran the SampleScene as Host with no clients and tried to use my new RPC, I got the following error:

MethodAccessException: Method `Unity.Netcode.NetworkBehaviour.__beginSendRpc(uint,Unity.Netcode.RpcParams,Unity.Netcode.RpcAttribute/RpcAttributeParams,Unity.Netcode.SendTo,Unity.Netcode.RpcDelivery)' is inaccessible from method `GrabbableBall.TestRpc()'
GrabbableBall.TestRpc () (at Assets/Scripts/GrabbableBall.cs:113)
GrabbableBall.Update () (at Assets/Scripts/GrabbableBall.cs:64)

Would it be better to open a bug ticket for this?

zachstronaut · 2023-11-30T00:31:29Z

I believe RuntimeAccessModifiersILPP is missing the following lines in the if condition in ProcessNetworkBehaviour()

methodDefinition.Name == nameof(NetworkBehaviour.__beginSendRpc) ||
methodDefinition.Name == nameof(NetworkBehaviour.__endSendRpc) ||

ShadauxCat · 2023-11-30T01:27:34Z

@zachstronaut You are spot on. I'll get a PR with that fix open tomorrow. Thanks for catching that.

I'm also going to be integrating this into BossRoom tomorrow, which should help catch any other issues like this.

ShadauxCat added 3 commits November 14, 2023 12:29

Swapping KnownClientIds to PeerClientIds

8c960ff

Same as last commit, but better in that this one includes all the cha…

de27803

…nged files.

ShadauxCat requested a review from a team as a code owner November 14, 2023 19:29

ShadauxCat added 2 commits November 14, 2023 13:30

Merge branch 'develop' into feat/universal_rpc_attribute

ea4f18b

style and changelog

25d2f18

ShadauxCat changed the title ~~feat: Added a universal RPC attribute~~ feat: Added a universal RPC attribute [MTT-7482] [MTT-7578] [MTT-7579] [MTT-7580] [MTT-7581] Nov 14, 2023

samlucas-unity approved these changes Nov 15, 2023

View reviewed changes

NoelStephensUnity requested changes Nov 15, 2023

View reviewed changes

Comment thread com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs Outdated

ShadauxCat added 3 commits November 16, 2023 15:44

- Merged PeerClientIds into ConnectedClientsIds

ba5d712

- Replaced OnPeerConnectedCallback and OnPeerDisconnectCallback with unified OnConnectionEvent

Minor changelog formatting fixes, and standards fixes

980ac60

Updated some comments for clarity.

ca3eba8

NoelStephensUnity reviewed Nov 16, 2023

View reviewed changes

NoelStephensUnity approved these changes Nov 16, 2023

View reviewed changes

ShadauxCat and others added 8 commits November 16, 2023 18:00

removed some unneeded checks on hashsets

75cabf9

standards + fix broken test

1a85bb4

- Added Not() runtime targets

2e59fb2

- Improved test coverage

Standards

c38ee55

Fix test failures in 2020.3

5c2334d

Fix test failures in 2020.3 but for real this time

56861f9

Merge branch 'develop' into feat/universal_rpc_attribute

0fb5acd

Merge branch 'develop' into feat/universal_rpc_attribute

8f208ae

ShadauxCat merged commit 39d0a2e into develop Nov 27, 2023

ShadauxCat deleted the feat/universal_rpc_attribute branch November 27, 2023 21:38

NoelStephensUnity mentioned this pull request Jan 16, 2024

How to trigger NGO CodeGen via custom buildpipeline? #2747

Open

Uh oh!

Conversation

ShadauxCat commented Nov 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changelog

Testing and Documentation

Uh oh!

samlucas-unity left a comment

Choose a reason for hiding this comment

Uh oh!

samlucas-unity Nov 15, 2023

Choose a reason for hiding this comment

Uh oh!

ShadauxCat Nov 16, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

samlucas-unity Nov 15, 2023

Choose a reason for hiding this comment

Uh oh!

ShadauxCat Nov 16, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

NoelStephensUnity Nov 16, 2023

Choose a reason for hiding this comment

Uh oh!

NoelStephensUnity left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

NoelStephensUnity Nov 16, 2023

Choose a reason for hiding this comment

Uh oh!

ShadauxCat Nov 17, 2023

Choose a reason for hiding this comment

Uh oh!

zachstronaut commented Nov 27, 2023

Uh oh!

zachstronaut commented Nov 29, 2023

Uh oh!

zachstronaut commented Nov 30, 2023

Uh oh!

ShadauxCat commented Nov 30, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ShadauxCat commented Nov 14, 2023 •

edited

Loading

NoelStephensUnity left a comment •

edited

Loading