Unity-Technologies / com.unity.netcode.gameobjects

Netcode for GameObjects is a high-level netcode SDK that provides networking capabilities to GameObject/MonoBehaviour workflows within Unity and sits on top of underlying transport layer.
MIT License
2.1k stars 431 forks source link

Unity.Netcode.Editor.CodeGen.NetworkBehaviourILPP: (0,0) when in 1.7.0 #2755

Open flabyee opened 8 months ago

flabyee commented 8 months ago

Description

An error occurs when rpc is used in the child class

Reproduce Steps

public class Parent : NetworkBehaviour
{

}

public class Child : Parent
{
    [ServerRpc]
    public void TestServerRpc()
    {

    }
}

Actual Outcome

Unity.Netcode.Editor.CodeGen.NetworkBehaviourILPP: (0,0): error - System.ArgumentNullException: Value cannot be null. (Parameter 'method')|| at Mono.Cecil.Cil.Instruction.Create(OpCode opcode, MethodReference method)|| at Unity.Netcode.Editor.CodeGen.NetworkBehaviourILPP.ProcessNetworkBehaviour(TypeDefinition typeDefinition, String[] assemblyDefines)|| at System.Collections.Generic.List1.ForEach(Action1 action)|| at Unity.Netcode.Editor.CodeGen.NetworkBehaviourILPP.Process(ICompiledAssembly compiledAssembly) at Mono.Cecil.Cil.Instruction.Create(OpCode opcode, MethodReference method)|| at Unity.Netcode.Editor.CodeGen.NetworkBehaviourILPP.ProcessNetworkBehaviour(TypeDefinition typeDefinition, String[] assemblyDefines)|| at System.Collections.Generic.List1.ForEach(Action1 action)|| at Unity.Netcode.Editor.CodeGen.NetworkBehaviourILPP.Process(ICompiledAssembly compiledAssembly)

PostProcessing failed: Unity.ILPP.Runner.ILPostProcessorDiagnosticsException: Unity.Netcode.Editor.CodeGen.NetworkBehaviourILPP: (0,0): error - System.ArgumentNullException: Value cannot be null. (Parameter 'method')|| at Mono.Cecil.Cil.Instruction.Create(OpCode opcode, MethodReference method)|| at Unity.Netcode.Editor.CodeGen.NetworkBehaviourILPP.ProcessNetworkBehaviour(TypeDefinition typeDefinition, String[] assemblyDefines)|| at System.Collections.Generic.List1.ForEach(Action1 action)|| at Unity.Netcode.Editor.CodeGen.NetworkBehaviourILPP.Process(ICompiledAssembly compiledAssembly) at Mono.Cecil.Cil.Instruction.Create(OpCode opcode, MethodReference method)|| at Unity.Netcode.Editor.CodeGen.NetworkBehaviourILPP.ProcessNetworkBehaviour(TypeDefinition typeDefinition, String[] assemblyDefines)|| at System.Collections.Generic.List1.ForEach(Action1 action)|| at Unity.Netcode.Editor.CodeGen.NetworkBehaviourILPP.Process(ICompiledAssembly compiledAssembly)

Screenshots

image

Environment

Additional Context

There's no problem when it's 1.6.0

benNeuro commented 8 months ago

Also running into this! It is blocking for our team at the moment as we need 1.7.0 for the GlobalObjectId fixes

In my particular case I'm trying to use a generic parent which extends NetworkBehaviour

public abstract class MyGeneric<T> : NetworkBehaviour {}

public class MyImpl : MyGeneric<MyImpl>
{
    [ClientRpc]
    void MyClientRpc() {}
}

And get the error:

(0,0): error  - System.NullReferenceException: Object reference not set to an instance of an object||  at Unity.Netcode.Editor.CodeGen.CodeGenHelpers.MakeGeneric (Mono.Cecil.MethodReference self, Mono.Cecil.TypeReference[] arguments) [0x00001] in <496640fcfd364331999fea04da4a7c7f>:0 ||  at Unity.Netcode.Editor.CodeGen.NetworkBehaviourILPP.ProcessNetworkBehaviour (Mono.Cecil.TypeDefinition typeDefinition, System.String[] assemblyDefines) [0x00824] in <496640fcfd364331999fea04da4a7c7f>:0 ||  at Unity.Netcode.Editor.CodeGen.NetworkBehaviourILPP+<>c__DisplayClass7_0.<Process>b__1 (Mono.Cecil.TypeDefinition b) [0x00012] in <496640fcfd364331999fea04da4a7c7f>:0 ||  at System.Collections.Generic.List`1[T].ForEach (System.Action`1[T] action) [0x00024] in....

(In the non-generic case I get the same error as OP, and no error in < 1.7.0)

Reag commented 8 months ago

Same issue here. We cant update for the better generics behavior because this bug blocks us

BrotherLavarius commented 8 months ago

Same. Need better generics.

LordDarkenon commented 8 months ago

Yes . This is a major 'blocker' bug and needs a fix ASAP.

marqzman commented 8 months ago

👍

aidanhorton commented 8 months ago

Same, major blocker for us too.

ShadauxCat commented 8 months ago

Hi everyone! Thank you for reporting this issue. We are aware of this issue and it has actually already been fixed in our develop branch. A new 1.7.1 patch release will come out soon with this fix in it; however, as our internal release process takes a bit of time to get through, in the mean time, there is a workaround available to unblock you.

We made some changes to how we initialize RPCs in order to support their use within generic classes; unfortunately, we missed a test case in those changes (which is now included in our testing framework) that covers what is reported here. If a class with an RPC inherits from a class (other than NetworkBehaviour) without any RPCs, this issue will occur.

The workaround is to just put an RPC in the parent class. It doesn't have to be used anywhere, it just has to exist. It doesn't matter if it's a client RPC or a server RPC. For example, looking at the case reported in the issue description:

public class Parent : NetworkBehaviour
{

}

public class Child : Parent
{
    [ServerRpc]
    public void TestServerRpc()
    {

    }
}

You can workaround the issue here like this:

public class Parent : NetworkBehaviour
{
    [ServerRpc]
    public void WorkaroundDummyServerRpc()
    {

    }
}

public class Child : Parent
{
    [ServerRpc]
    public void TestServerRpc()
    {

    }
}

We apologize for the inconvenience and will try to get this fix put out as soon as we can, but in the mean time, the above pattern should be able to unblock you.

Laumania commented 8 months ago

I got the same issue, but will try the workaround for now.

However, I'm curious how you guys figured out what was the problem for you? I mean the error doesn't say clearly what the issue is - so just curious if I missed something.

Update: Looked through my use of ServerRpc and ClientRpc and I don't have that scenario as described - still I get the error when upgrading to 1.7.0.

BrotherLavarius commented 8 months ago

Tried the workaround, works. Many thanks o/

On Wed, Nov 8, 2023, 7:13 PM Kitty Draper @.***> wrote:

Hi everyone! Thank you for reporting this issue. We are aware of this issue and it has actually already been fixed in our develop branch. A new 1.7.1 patch release will come out soon with this fix in it; however, as our internal release process takes a bit of time to get through, in the mean time, there is a workaround available to unblock you.

We made some changes to how we initialize RPCs in order to support their use within generic classes; unfortunately, we missed a test case in those changes (which is now included in our testing framework) that covers what is reported here. If a class with an RPC inherits from a class (other than NetworkBehaviour) without any RPCs, this issue will occur.

The workaround is to just put an RPC in the parent class. It doesn't have to be used anywhere, it just has to exist. It doesn't matter if it's a client RPC or a server RPC. For example, looking at the case reported in the issue description:

public class Parent : NetworkBehaviour{ } public class Child : Parent{ [ServerRpc] public void TestServerRpc() {

}}

You can workaround the issue here like this:

public class Parent : NetworkBehaviour{ [ServerRpc] public void WorkaroundDummyServerRpc() {

}}

public class Child : Parent{ [ServerRpc] public void TestServerRpc() {

}}

We apologize for the inconvenience and will try to get this fix put out as soon as we can, but in the mean time, the above pattern should be able to unblock you.

— Reply to this email directly, view it on GitHub https://github.com/Unity-Technologies/com.unity.netcode.gameobjects/issues/2755#issuecomment-1802496423, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB6FJ6CI75CB3NVWG3OUDIDYDPKUBAVCNFSM6AAAAAA64CCVBOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMBSGQ4TMNBSGM . You are receiving this because you are subscribed to this thread.Message ID: <Unity-Technologies/com.unity.netcode. @.***>

gnovos commented 7 months ago

I have tried the workaround but it doesn't seem to fix anything. If there is a chain of subclasses, does there need to one be in each class or just the top parent?

zachstronaut commented 7 months ago

Hello!

1.7.0 also doesn't work for us because of this issue. Our codebase compiles fine under 1.6.0 but doesn't compile under 1.7.0.

We don't have any RPCs in the definitions of any generic NetworkBehaviour derivatives because of: https://github.com/Unity-Technologies/com.unity.netcode.gameobjects/issues/714

That's the only limitation we've run into, and we've mitigated that with the suggested fix on issue 714.

The suggested fix here for this issue (2755) is a bit ambiguous, and also we have a LOT of classes with a LOT of RPCs so it'd be a bit of an undertaking if I'm understanding it correctly.

We have lots of generics and sub classes of generics, that may also themselves be generic. I thought it might be useful for you to see some examples of real inheritance chains we're using in our project:

Thing : NetworkBehaviour, IThing
AnotherThing : Thing
GenericThing<T> : AnotherThing
abstract AbstractThing<T> : GenericThing<T>
abstract SomeThing<T> : AbstractThing<T>, IThis, IThat
RealThing : SomeThing<FooType>
MyClass<A, B, C> : AnotherThing
MyThing : MyClass<Apples, Bananas, Coffee>
CompositeNetBehaviour<RootClass, ServerClass> : NetworkBehaviour 
DooDad : CompositeNetBehaviour<DooDad, DooDadServer>, ILoveInterfaces
abstract NetModel : NetworkBehaviour
abstract NetModelBase : NetModel
abstract NetModelGeneric<TUI> : NetModelBase
MyNetModel : NetModelGeneric<MyUI>
ShadauxCat commented 7 months ago

I have tried the workaround but it doesn't seem to fix anything. If there is a chain of subclasses, does there need to one be in each class or just the top parent?

@gnovos Yes, it has to be every class in the chain. Basically if the codegen finds an RPC, it tries to generate a virtual method to initialize the RPCs and call the parent class's implementation. If the parent class has no RPCs, it will not have generated that virtual method, so the attempt to find and call it will fail with this error. And then when you add an RPC to the parent class, it will also generate this method, and will also try to call its own parent class's implementation, so then its parent also has to have an RPC in it, all the way up to NetworkBehaviour.

ShadauxCat commented 7 months ago

@zachstronaut A NetworkBehaviour being generic is not actually relevant to the issue. Any NetworkBehaviour which has RPCs in it and derives from a parent class that does not have RPCs in it will trigger this error. As mentioned earlier, though, this issue is fixed in develop and we are in the process of trying to roll out a 1.7.1 with this fix ASAP. (Our internal release process limits how quickly we can get a release put out.)

zachstronaut commented 7 months ago

@ShadauxCat Fantastic, thank you for the clarification!