FICTURE7 / CoCSharp

Clash of Clans library, proxy and server written in .NET [Unmaintained]
MIT License
109 stars 57 forks source link

Add EndClientTurnPacket #53

Closed devinvisible closed 8 years ago

devinvisible commented 8 years ago

After further analysis, this packet is actually implemented as CommandPacket. This pull request should be rejected.

My addition came after seeing the UnknownPacket comes across the Proxy output and I now realize that this is due to CommandPacket not being part of the ProxyNetworkManager's PacketDictionary. I suppose may give additional control but I see it as a burden and have replaced the InitializePacketDictionary method body with one which dynamically adds all Types which implement IPacket. Let me know you want a pull request for this.

FastFrench commented 8 years ago

You're right, I've done the same. It's much easier to add new messages once using reflection to know them all, instead of manually add messages in a dictionary.
Unfortunately our project is now quite far from CoCSharp, so it's not easy to provide usable extracts from it. For those interested, here is the code we're using for the message factory: (code is similar for commands)

namespace ClashProtocol.Networking { using System; using System.Collections.Generic; using System.Linq;

using Messages;

/// <summary>
/// Provides methods to create <see cref="IMessage"/> instances.
/// </summary>
public static class MessageFactory
{
    private readonly static Dictionary<ushort, Type> _messages;

static MessageFactory()
    {
        _messages = new Dictionary<ushort, Type>();

        var messageInterface = typeof(IMessage);
        var assembly = messageInterface.Assembly;
        foreach (var message in assembly.GetTypes().Where(p => messageInterface.IsAssignableFrom(p) && p.IsClass && p.IsPublic && !p.IsAbstract))
        {
            var id = ((IMessage)Activator.CreateInstance(message)).ID;
            if (_messages.ContainsKey(id))
            {
                Console.ForegroundColor = ConsoleColor.Red;
                string errorMessage = string.Format("MessageFactory.Messages contains a duplicated Key.\r\nThose two messages share the same ID={0} (0x{0:X}): {1} and {2}", id, _messages[id].Name, message.Name);
                Console.WriteLine(errorMessage);
                Console.ResetColor();
                throw new InvalidOperationException(errorMessage);

            }
            _messages[id] = message;
        }
    }

    /// <summary>
    /// Gets the supported Messages.
    /// </summary>
    /// <value>The Messages.</value>
    public static Dictionary<ushort, Type> Messages { get { return _messages; } }

    /// <summary>
    /// Creates a new <see cref="IMessage"/> instance with the specified message ID.
    /// </summary>
    /// <param name="id">The ID of the message to create the instance.</param>
    /// <returns>Instance of <see cref="IMessage"/> created.</returns>
    public static IMessage Create(ushort id)
    {
        var messageType = (Type)null;

        if (!Messages.TryGetValue(id, out messageType))
            return new UnknownMessage();

        return (IMessage)Activator.CreateInstance(messageType); // creates an instance for that message id
    }

    /// <summary>
    /// Creates a new <see cref="IMessage"/> instance with the specified message type.
    /// </summary>
    /// <typeparam name="T">Type of the <see cref="IMessage"/> to create.</typeparam>
    /// <returns>Instance of <see cref="IMessage"/> created.</returns>
    public static T Create<T>() where T : IMessage
    {
        var messageType = typeof(T);
        return (T)Activator.CreateInstance(messageType);
    }

    /// <summary>
    /// Tries to create a new <see cref="IMessage"/> instance with the specified message ID.
    /// </summary>
    /// <param name="id">The ID of the message to create the instance.</param>
    /// <param name="msg">The instance <see cref="IMessage"/> created, returns null if failed to create the instance.</param>
    /// <returns>Returns <see cref="true"/> if the instance was created successfully.</returns>
    public static bool TryCreate(ushort id, out IMessage msg)
    {
        var messageType = (Type)null;

        if (!Messages.TryGetValue(id, out messageType))
        {
            msg = null;
            return false;
        }

        msg = (IMessage)Activator.CreateInstance(messageType);
        return true;
    }
}

}

BTW, you should really use the word "message" instead of "packet" in all the project. Actually, "packet" has a different meaning (one packet can hold the data of only a part of a message or contain several messages. It's the raw chunck of data you get from the network). That makes lot of renames, but make it much clearer.

FICTURE7 commented 8 years ago

I agree with @FastFrench on the terminology of packets and messages and am going to rename the stuff. Using reflection might be the way to do it. I have taken a break from CoCSharp, CoC and programming lately, and I reviewed my code and concluded that some stuff could have been done a lot better. Soooo I am probably going to rewrite a big portion of the code to be a lot more fancier I hope.

devinvisible commented 8 years ago

@FastFrench, have a repository you are interested in collaborating on? I'm new to the scene and found CoCSharp to be clean and designed well (enough for me). I'm looking to continue my research and have some projects in mind. So far redmoon.io hasn't been fruitful and I don't know of any better communities. Feel free to continue this conversation in a private message. Cheers.