Callback attributes not enforced for NetworkBehaviours created through test scripts

Bug Description [ServerCallback] and [ClientCallback] stop methods on NetworkBehaviours from being called on the wrong end of the network. However, these attributes have false negatives when called during unit tests. This makes it hard to test code that relies on these attributes, because the test environment and the build environment are different.

Minimum Reproduction Test

using Mirror;
using Mirror.Tests;
using NUnit.Framework;

public class TestAttributes : MirrorEditModeTest
{
    NetworkConnectionToClient connectionToClient;
    TestPlayer playerServer;
    TestPlayer playerClient;

    [SetUp]
    public override void SetUp()
    {
        base.SetUp();

        // start server/client
        NetworkServer.Listen(1);
        ConnectClientBlockingAuthenticatedAndReady(out connectionToClient);

        CreateNetworkedAndSpawn(out _, out _, out playerServer,
        out _, out _, out playerClient,
        connectionToClient);
    }

    [Test]
    public void TestServerCallbackAttributes()
    {
        playerClient.CMDFoo();
        ProcessMessages();
    }

    [Test]
    public void TestClientCallbackAttributes()
    {
        playerClient.Foo();
    }
}

Buttons

using UnityEngine;

public class UIManager : MonoBehaviour
{
    public TestPlayer player;

    public void CallFoo()
    {
        player.Foo();
    }

    public void CallCommandFoo()
    {
        player.CMDFoo();
    }
}

Player

using Mirror;
using UnityEngine;

public class TestPlayer : NetworkBehaviour
{
    public override void OnStartLocalPlayer()
    {
        base.OnStartLocalPlayer();
        FindObjectOfType<UIManager>().player = this;
    }

    public void Foo()
    {
        ServerFoo();
        ClientFoo();
    }

    [Command]
    public void CMDFoo()
    {
        ServerFoo();
        ClientFoo();
    }

    [ServerCallback]
    private void ServerFoo()
    {
        Debug.LogError("Server Called");
    }

    [ClientCallback]
    private void ClientFoo()
    {
        Debug.LogError("Client Called");
    }
}

Build (editor as server) Build (editor as client) All buttons were pressed for these examples, but only one ever logged anything (as expected)

Test (TestServerCallbackAttributes) Test (TestClientCallbackAttributes)

Expected behavior I would expect that the client is only called on Foo for both test and build I would expect that the server is only called on CMDFoo for both test and build

Desktop:

OS: Windows 10
Build target: NA
Unity version: 2021.3.11f1 (Current LTS)
Mirror branch: Master

Additional Notes Obviously this is not a problem when using host mode, as one would expect to have both be called. Here is a link to a minimum reproduction project. Hope not having to deal with Assembly definitions helps.

Assigned to vis2k to see if this is a bug or enhancement or if there's a work around or if nothing can be done.

I've done some research The reason that it gives these false positives right now is that the attributes only check if NetworkServer.active and NetworkClient.active depending on the situation. This works in the build because these two things won't be true at the same time unless we're in host mode, but tests break this assumption There's nothing really to be done about this for custom classes and mono-behaviours, but network-behaviours do have isServer and isClient available for checking instead. I tried to add this functionality in myself on ServerClientAttributeProcessor.cs

...

static Type GetMonoType(this TypeReference type)
{
    return Type.GetType(type.FullName + ", " + type.Module.Assembly.FullName, true);
}

static void InjectServerGuard(WeaverTypes weaverTypes, MethodDefinition md, bool logWarning)
{
    ILProcessor worker = md.Body.GetILProcessor();
    Instruction top = md.Body.Instructions[0];

    if(GetMonoType(md.DeclaringType).IsSubclassOf(typeof(NetworkBehaviour)))
    {
        worker.InsertBefore(top, worker.Create(OpCodes.Call, weaverTypes.NetworkBehaviourGetIsServer));
    }
    else
    {
        worker.InsertBefore(top, worker.Create(OpCodes.Call, weaverTypes.NetworkServerGetActive));
    }
    worker.InsertBefore(top, worker.Create(OpCodes.Brtrue, top));

    ...

}

static void InjectClientGuard(WeaverTypes weaverTypes, MethodDefinition md, bool logWarning)
{
    ILProcessor worker = md.Body.GetILProcessor();
    Instruction top = md.Body.Instructions[0];

    if (GetMonoType(md.DeclaringType).IsSubclassOf(typeof(NetworkBehaviour)))
    {
         worker.InsertBefore(top, worker.Create(OpCodes.Call, weaverTypes.NetworkBehaviourGetIsClient));
    }
    else
    {
        worker.InsertBefore(top, worker.Create(OpCodes.Call, weaverTypes.NetworkClientGetActive));
    }
    worker.InsertBefore(top, worker.Create(OpCodes.Brtrue, top));

    ...

}

...

and WeaverTypes.cs

public MethodReference NetworkBehaviourGetIsServer;
public MethodReference NetworkBehaviourGetIsClient;
...
 public WeaverTypes(AssemblyDefinition assembly, Logger Log, ref bool WeavingFailed)
 {
     ...

     TypeReference NetworkBehaviourType = Import<NetworkBehaviour>();

      NetworkBehaviourGetIsClient = Resolvers.ResolveMethod(NetworkBehaviourType, assembly, Log, "get_isClient", ref WeavingFailed);
      NetworkBehaviourGetIsServer = Resolvers.ResolveMethod(NetworkBehaviourType, assembly, Log, "get_isServer", ref WeavingFailed);

     ...
 }

but got System.InvalidProgramException : Invalid IL code on calls from NetworkBehaviours and don't have the tools to debug. Here is my attempt

it's notable that this would add one extra rule to these attributes - they would only work for NetworkBehaviours that have already called awake. This should usually not be a problem, as AddComponent calls awake, but it's an edge case that previously did not exist under this solution

MirrorNetworking / Mirror

Callback attributes not enforced for NetworkBehaviours created through test scripts #3227