OperationMethodInfoCache.RegisterOperation issue

Options
terrainoob
edited October 2011 in DotNet
Hello,

I've been playing around in the MmoDemo app and trying to convert Server.MmoPeer to use OperationMethodInfoCache and OperationDispatcher, but I'm having some trouble.

Here's what I'm trying to do:
        private static readonly OperationMethodInfoCache _operations = new OperationMethodInfoCache();
        private readonly OperationDispatcher _dispatcher;

        public SperoNovaPeer(IRpcProtocol rpcProtocol, IPhotonPeer nativePeer)
            : base(rpcProtocol, nativePeer)
        {
            _dispatcher = new OperationDispatcher(_operations, this);
            this.SetCurrentOperationHandler(this);
            _operations.RegisterOperations(this.CurrentOperationHandler.GetType());
        }

        [Operation(OperationCode = (byte)OperationCode.CreateWorld)]
        public OperationResponse OperationCreateWorld(PeerBase peer, OperationRequest request)
        {
         ...
        }

        [Operation(OperationCode = (byte)OperationCode.EnterWorld)]
        public OperationResponse OperationEnterWorld(PeerBase peer, OperationRequest request, SendParameters sendParameters)
        {
        ...
        }


This gives an error in the log stating
System.ArgumentException: An operation with the same code has already been added. Code 90; Methods MmoPeer.OperationCreateWorld vs MmoPeer.OperationCreateWorld

So then instead of RegisterOperations, I tried:

_operations.RegisterOperation(OperationCreateWorld);
_operations.RegisterOperation(OperationEnterWorld);

But I can't compile this because the OperationEnterWorld does not have the correct signature for RegisterOperation (it includes the extra SendParameters).

I know there's an overload of RegisterOperation, but the documentation doesn't explain at all how to use this and I can't find any examples anywhere.

Is there any way to accomplish what I'm trying to do?

Thanks!

Comments

  • Boris
    Options
    several issues:

    1) Every new SperoNovaPeer tries to register the same operation codes again on the static OperationMethodInfoCache _operations.
    2) You call RegisterOperations after creating the OperationDispatcher -> the OperationDispatcher reads the methods from the OperationMethodInfoCache, but it does not contain any methods yet

    to fix it try this:
    private static readonly OperationMethodInfoCache _operations;
     
    static SperoNovaPeer()
    {
     _operations = new OperationMethodInfoCache();
     _operations.RegisterOperations(typeof(SperoNovaPeer));
    }    
    
     private readonly OperationDispatcher _dispatcher;
    
            public SperoNovaPeer(IRpcProtocol rpcProtocol, IPhotonPeer nativePeer)
                : base(rpcProtocol, nativePeer)
            {
                _dispatcher = new OperationDispatcher(_operations, this);
                this.SetCurrentOperationHandler(this);
            }
    
    public OperationResponse OnOperationRequest(PeerBase peer, OperationRequest operationRequest, SendParameters sendParameters);
    {
    OperationResponse response;
    if (_dispatcher.DispatchOperationRequest(this, request, out response))
    {
        return response;
    }
    return null;
    }
    

    The OperationDispatcher does not accept SendParameters yet, will change this in a future version.
  • Excellent. That seems to have done the trick. (at least the error message has gone away) :D

    One question: if OperationDispatcher does not accept SendParameters yet, does that mean that I can't use the OperationEnterWorld as defined with OperationDispatcher, or just that I can't register it with RegisterOperation (I must use RegisterOperations instead)? In other words, will the code you've just supplied work for OperationEnterWorld, or am I stuck with using a switch statement because of the SendParameters?

    Thanks
  • Boris
    Options
    The OperationMethodInfoCache requires methods with the following signature:
    OperationResponse MyMethod(PeerBase peer, OperationRequest request);

    You should actually get an ArgumentException if you try to register methods with a different signature.
    In order to get it working for OperationEnterWorld you have to remove the SendParameters parameter from the method.
  • Thanks for the clarification!

    I realize in my example with just these two operations, there's really no point to using the OperationMethodInfoCache if only one of them can be registered, but I'm thinking about the scenario where I have a large number of operations and only one or two of them include the SendParameters. Would using a switch to catch those one or two odd operations and then calling the DispatchOperationRequest if I drop out of the switch be the best option for that?

    Also, I was wondering if you could share a quick example of when/how to use the RegisterOperation(func<...>) overload. I'm really not understanding what situations that overload is for.

    Thanks!
  • Just wanted to point out something that I discovered that tripped me up with this. When I implemented the suggested fix (moving the registerOperations call to the static constructor) I got some weird behavior. When I tried to request the CreateWorld operation from my client, the server would send back an OperationNotSupported response. This confused me, because the operation should have been registered (and since there were no errors in the log, I assumed it was).

    Turns out, I needed to also make the OperationCreateWorld method static:
    &#91;Operation(OperationCode = (byte)OperationCode.CreateWorld)&#93;
    public static OperationResponse OperationCreateWorld(PeerBase peer, OperationRequest request)
    {
    ...
    }
    

    This one was pretty difficult for me to track down because there were no compile errors and no errors in the log to indicate I had done something wrong. Would it be possible to get some errors into the log when this condition happens? Or is it not possible for the server to detect this condition?

    Just wanted to pass this along in case someone else runs into it.
  • Boris
    Options
    terrainoob wrote:
    Thanks for the clarification!

    I realize in my example with just these two operations, there's really no point to using the OperationMethodInfoCache if only one of them can be registered, but I'm thinking about the scenario where I have a large number of operations and only one or two of them include the SendParameters. Would using a switch to catch those one or two odd operations and then calling the DispatchOperationRequest if I drop out of the switch be the best option for that?

    We will change it to include the send parameters, then you won't have to worry about it.
    terrainoob wrote:
    Also, I was wondering if you could share a quick example of when/how to use the RegisterOperation(func<...>) overload. I'm really not understanding what situations that overload is for.

    This method is there to register single methods instead of a whole class.
  • Boris
    Options
    terrainoob wrote:
    Just wanted to point out something that I discovered that tripped me up with this. When I implemented the suggested fix (moving the registerOperations call to the static constructor) I got some weird behavior. When I tried to request the CreateWorld operation from my client, the server would send back an OperationNotSupported response. This confused me, because the operation should have been registered (and since there were no errors in the log, I assumed it was).

    Turns out, I needed to also make the OperationCreateWorld method static:
    &#91;Operation(OperationCode = (byte)OperationCode.CreateWorld)&#93;
    public static OperationResponse OperationCreateWorld(PeerBase peer, OperationRequest request)
    {
    ...
    }
    

    This one was pretty difficult for me to track down because there were no compile errors and no errors in the log to indicate I had done something wrong. Would it be possible to get some errors into the log when this condition happens? Or is it not possible for the server to detect this condition?

    Just wanted to pass this along in case someone else runs into it.

    It should work for static and non-static methods.
    When creating the OperationDispatcher instance you need to insert the operation handler instance of the same type to get it working.
    This registers all methods (static and non-static) of type MyClass:
    var operations = new OperationMethodInfoCache();
    operations.RegisterOperations(typeof(MyClass));
    
    and this maps operation codes to instance and class members of the used MyClass instance:
    new OperationDispatcher(operations, new MyClass());
    
  • We will change it to include the send parameters, then you won't have to worry about it.

    Excellent!
    This method [RegisterOperation] is there to register single methods instead of a whole class.

    I understand that in general. What I'm confused about is that there are 2 overloaded signatures for that method, and I can't figure out what the second method signature (the one that includes the (func...) parameter) is for. When/how would I use that parameter signature? I don't understand why that overload exists.

    Thanks!
  • Boris
    Options
    This is what the override looks like:
     public bool RegisterOperation(Func&lt;PeerBase, OperationRequest, OperationResponse&gt; method)
            {
                return this.RegisterOperation(method.Method);
            }
    

    It allows you to do the follwing:
    &#91;Operation(OperationCode = 1)&#93;
     private OperationResponse TestOperation(PeerBase a, OperationRequest b)
            {
                return null;
            }
    
    public void RegisterOperations()
            {
                this.methodCache.RegisterOperation(this.TestOperation);
               ...
            }
    

    Without the override the compiler says:
    Error	1	The best overloaded method match for 'Photon.SocketServer.Rpc.Reflection.OperationMethodInfoCache.RegisterOperation(System.Reflection.MethodInfo)' has some invalid arguments	
    Error	2	Argument 1: cannot convert from 'method group' to 'System.Reflection.MethodInfo'