NGO - Server/Client RPCs output compiler errors if declared as local methods - is this a bug?

As the title summaries, the following will compile correctly:

class Example {
     void Foo() {
          if (IsServer) FooClientRpc();
          else FooServerRpc();
     }
     
     [ServerRpc]
     void FooServerRpc() {
          //server stuff
     }

     [ClientRpc]
     void FooClientRpc() {
         //client stuff
     }
}

But, when trying to make my code a little easier to collapse, I discovered the following will output the “ServerRpc must have the ServerRpc Suffix” error, as will the ClientRpc.

class Example2 {
     void Foo() {
          if (IsServer) FooClientRpc();
          else FooServerRpc();

          [ServerRpc]
          void FooServerRpc() {
               //server stuff as local function
          }
          
          [ClientRpc]
          void FooClientRpc() {
               //client stuff as local function
          }
     }
}

Personally, I can’t see why Example2 wouldn’t be a valid way to structure my code, since I’m not planning on calling Foo[X]Rpc() anywhere other than in Foo(). Is there a reason this is invalid code? Or is this a bug that should be reported? I should mention I’m using NGO 1.8.1 in unity 2021.38.f1.

The C# compiler produces a hidden method (and a closure when necessary) that represents the local function. The generated method’s name is entirely up to the compiler, which is under no obligation to use the local function’s name - it picks its own (example). Netcode itself is not aware of the original method name, as the hookup occurs as an ILPP which only sees the C# compiler’s output. Due to this limitation, you should just accept usage of normal methods, even though you’re only using these in one method.

Fair enough, thanks for the information - really interesting to hear about the internal side of things like that. It’s a shame that limitation is in place, but it makes more sense now, and realistically it’s no big deal - I can just use regions to collapse the code when I don’t need to read it anyway.

If readability and reducing complexity is the goal, I recommend to:

  • use partial keyword so you can have Example.cs and Example.Rpc.cs containing only the RPC methods
  • create a separate component just for the RPCs

I prefer the latter because I find inlining RPCs in a script both confusing and dangerous, especially if the feature itself is divided between client/owner and server parts. In such cases I highly recommend splitting the component into Client/Owner script and a corresponding Server script. But for the most part, a per-feature Network component is really helpful to keep things clean and to not have the code be heavily intermingled.

For example:

  • Weapon.cs
  • WeaponNetworkFire.cs
  • WeaponNetworkReload.cs

This also makes it clear where the network related code for each given feature belongs into.

Prefer more components over more lines! :wink: