Search Unity

Third Party Photon Server message handling

Discussion in 'Multiplayer' started by Glader, Jan 21, 2014.

  1. Glader

    Glader

    Joined:
    Aug 19, 2013
    Posts:
    456
    Edit: April 20th 2014

    Short version of original post: All these switch cases suck. Isn't there a better way? See most recent post.
     
    Last edited: Apr 23, 2014
  2. Dabeh

    Dabeh

    Joined:
    Oct 26, 2011
    Posts:
    1,614
    I've had my mind on this for a while now, I'm very curious to see what people have come up with.
     
  3. Glader

    Glader

    Joined:
    Aug 19, 2013
    Posts:
    456
    I am as well. It just doesn't seem avoidable though since the type of packet is very much tied to the byte code.
     
    Last edited: Jan 22, 2014
  4. tobiass

    tobiass

    Joined:
    Apr 7, 2009
    Posts:
    3,066
    I don't have a good answer for this (as I never wanted to get rid of the switch-cases) but one thing about the byte-code: You could make one Operation with one code only and the first parameter of that would be something else. You can call it a sub-operation or whatever. In any case: This way you can break the "only 255 codes" barrier.
    Aside from that, the Dictionary of delegates seems like a nice idea. It should be fast enough but maybe a bit difficult to debug once things go wrong.
    You could also simply use an array of delegates. I don't think it's too memory intensive.
     
  5. Glader

    Glader

    Joined:
    Aug 19, 2013
    Posts:
    456
    I came up with a solution for this. If anyone is interested let me know, I'm going to bed for now but if there is interest I'll post the two ways that I just finished writing tonight (no I haven't been working on it the whole time, I just gave up and recently reapproached the problem).
     
  6. tobiass

    tobiass

    Joined:
    Apr 7, 2009
    Posts:
    3,066
    I'm glad to read you found a solution.
    If you could write a few words about it, I think it could wrap up this thread nicely.
    If you've got the time, that is.
     
  7. Glader

    Glader

    Joined:
    Aug 19, 2013
    Posts:
    456
    Sure

    My recommendation for this problem, while I was close with a couple of prototypes when I originally wrote this, may be considered a bit convoluted but imo it's worth it.

    Performance tests for generation of Packet classes, classes that mostly encapsulate data, and calling the required handler function with this method for 10,000,000 packets took about 1100ms on my PC. This is only a 100% performance decrease compared to a switch case that creates and calls the required method for handling as that was about 400ms. This is also a method compatible with 3.5 .Net, which some others I prototyped turned out not to be =/.

    Anyway, the meat comes down to this. I utilize reflection to scrape the assembly, or specific Type as that's more viable if everything is in one location, for a custom attribute I wrote that containing the bytecode for the packet, the class type that the handler method requires and the PeerBase type although the generics I wrote don't contrainst to that as they don't utilize any PeerBase functionality within in them. This is set in the attribute only because multiple PeerBase types may want to call the same packet handler method and your reflection code needs to determine which attribute to reference for the data.

    Ok, so once you've scraped a Type or Assembly for all the methods containing the custom attribute...

    Here is some code to help you do so compatible with 3.5 .Net (4.5 has different syntax but I don't think this is deprecated):

    Code (csharp):
    1.  
    2.         public static IEnumerable<MethodInfo> GetMethodsWithAttribute<AttributeType>(Type type)
    3.         {
    4.             return type.GetMethods().Where(item => item.GetCustomAttributes(typeof(AttributeType), false).Count() > 0);
    5.         }
    6.  
    7.         //TODO: Combine shared functionality with the non additionalCriteria method.
    8.         public static IEnumerable<MethodInfo> GetMethodsWithAttribute<AttributeType>(Assembly assembly)
    9.         {
    10.             IEnumerable<Type> tempTypeList = assembly.GetTypes();
    11.  
    12.             List<MethodInfo> tempMethodInfoList = new List<MethodInfo>(tempTypeList.Count());
    13.  
    14.             foreach (Type T in tempTypeList)
    15.             {
    16.                 tempMethodInfoList.AddRange(GetMethodsWithAttribute<AttributeType>(T));
    17.             }
    18.  
    19.             return tempMethodInfoList;
    20.         }
    21.  
    Now you have a collection of MethodInfos that have PacketMethod, or whatever your attribute is, at your disposal. This is when you iterate over the MethodInfo and use the Activator class to generate a delegate pointing to these handler methods. At the suggestion of Tobiass I used an array of delegates, they take in an IPacket which is implemented on all packet types, instead of a dictionary. It improved preformance by 33%. Anyway, you essentially assign those delegates to the position in the array that corresponds to the bytecode for the packet. This way when you have an IPacket to handle...

    maybe something like this:

    Code (csharp):
    1.  
    2.  
    3. public LoginRequest : IPacket
    4. {
    5.  
    6. }
    7.  
    8.  
    You can pass that into the Handler class and have it call the necessary delegate.


    However, this is only one part of the problem. You need a way to generate these packet classes without a switch or you're back at square one. I personally use Protobuf-net, a .Net implementation of Google's Protocol buffers for serializing data, so your method may be alittle different. However, since we still have, or can get again, the MethodInfos/Custom-Packet-Attribute again we can generate a new array of delegates but this time pointing to a private static creation method.

    This can either contain new YourPacketClass(byteCode, photonDictionaryThingy); or if you're using Protocol buffers it can look something like this (calling a static method off the base class that handlers the deserialization):

    Edit: I've just learned that via an attribute in Protobuf-net you can avoid writing this static method every time and instead just have it in the base class of Packet ^,^ how neat! Although the ProtoInclude attribute sure would get pretty ugly imo.

    Code (csharp):
    1.  
    2.         private static IPacket GenerateNew(byte packetCode, Dictionary<byte, object> data)
    3.         {
    4.             return Packet.GenerateByDictionary<LoginRequest>(packetCode, data);
    5.         }
    6.  
    So, now with this setup you can have a non-traditional factory pattern for all your Packet classes and, again, create them based on the packetcode that the custom attribute contained information about.


    This comes with some caveats though. Handler methods must be static so naturally they must contain a reference to the PeerBase or Listener type and the class that implements IPacket. They must also be marked with the custom attribute mentioned above. It also means that you must have some seemingly odd private static methods that produce class instances so they can be gathered and handled.

    When all is said and done though you no longer have switch case statements that could span for hundreds of lines depending on how many bytecodes you may be handling. And also the preformance decrease is negligible since actual logic to "handle" the packet itself will take up 99.9% of the computation time. The increase in time until you get to the logic is no where near enough of an increase to consider when compared to the cleanliness of your packet handling.

    If anyone is interested or has more questions let me know.

    Edit: Slight update, you may not have to have an array of delegates that create the concrete packet types if you use Protocol buffers. Protocol buffers allows you to use tags with the Protoinclude attribute on a base class to deserialize to that baseclass but actually be the child class.
     
    Last edited: Apr 25, 2014