Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature: allow calling [Command]s on server-only directly. #3450 #3451

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions Assets/Mirror/Editor/Weaver/Processors/CommandProcessor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,14 @@ public static class CommandProcessor
// generates code like:
public void CmdThrust(float thrusting, int spin)
{
// on server-only, allow calling the original function directly.
if (isServer && !isClient)
{
UserCode_CmdThrust(value);
return;
}

// otherwise send a command message over the network
NetworkWriterPooled writer = NetworkWriterPool.Get();
writer.Write(thrusting);
writer.WritePackedUInt32((uint)spin);
Expand Down Expand Up @@ -38,6 +46,32 @@ public static MethodDefinition ProcessCommandCall(WeaverTypes weaverTypes, Write

NetworkBehaviourProcessor.WriteSetupLocals(worker, weaverTypes);

Instruction skipIfNotServerOnly = worker.Create(OpCodes.Nop);

// Check if isServer && !isClient
// worker.Emit(OpCodes.Ldarg_0); // loads this. for isClient check later
worker.Emit(OpCodes.Call, weaverTypes.NetworkServerGetActive); // TODO isServer?
worker.Emit(OpCodes.Brfalse, skipIfNotServerOnly);

// worker.Emit(OpCodes.Ldarg_0); // loads this. for isClient check later
worker.Emit(OpCodes.Call, weaverTypes.NetworkClientGetActive); // TODO isClient?
worker.Emit(OpCodes.Brtrue, skipIfNotServerOnly);

// Load 'this' reference (Ldarg_0)
worker.Emit(OpCodes.Ldarg_0);

// Load all the remaining arguments (Ldarg_1, Ldarg_2, ...)
for (int i = 1; i < md.Parameters.Count + 1; i++)
worker.Emit(OpCodes.Ldarg, i);

// Call the original function directly (UserCode_CmdTest__Int32)
worker.Emit(OpCodes.Call, cmd);
worker.Emit(OpCodes.Ret);

worker.Append(skipIfNotServerOnly);



// NetworkWriter writer = new NetworkWriter();
NetworkBehaviourProcessor.WriteGetWriter(worker, weaverTypes);

Expand Down
49 changes: 49 additions & 0 deletions Assets/Mirror/Tests/Editor/Rpcs/CommandTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,15 @@ namespace Mirror.Tests.Rpcs
class AuthorityBehaviour : NetworkBehaviour
{
public event Action<int> onSendInt;
public event Action<int, string, bool> onSendMulti;

[Command]
public void SendInt(int someInt) =>
onSendInt?.Invoke(someInt);

[Command]
public void SendMulti(int someInt, string someString, bool someBool) =>
onSendMulti?.Invoke(someInt, someString, someBool);
}

class IgnoreAuthorityBehaviour : NetworkBehaviour
Expand Down Expand Up @@ -278,4 +283,48 @@ public void Command_RequiresAuthorityFalse_ForOtherObjectWithoutConnectionToServ
Assert.That(called, Is.EqualTo(1));
}
}

// need server-only mode for some test
public class CommandTest_ServerOnly : MirrorTest
{
[SetUp]
public override void SetUp()
{
base.SetUp();
// start server without client
NetworkServer.Listen(1);
}

[TearDown]
public override void TearDown() => base.TearDown();

// [Command] functions should be callable on server-only for convenience.
// https://github.com/MirrorNetworking/Mirror/issues/3450
[Test]
public void CommandCalledWhenServerOnly()
{
// spawn
CreateNetworked(out _, out _, out AuthorityBehaviour serverComponent);

// set up a callback and check
int callCount = 0;
serverComponent.onSendMulti += (a, b, c) =>
{
callCount++;
Assert.That(a, Is.EqualTo(42));
Assert.That(b, Is.EqualTo("test"));
Assert.That(c, Is.EqualTo(true));
};

// call [Command] on server.
// test multiple parameters to ensure weaver properly injects all
// LdArg0,1,2, etc. instructions.
//
// this should call the function immediately,
// without processing messages.
serverComponent.SendMulti(42, "test", true);
// ProcessMessages();
Assert.That(callCount, Is.EqualTo(1));
}
}
}