5Mixer / mphx

A little library to let you make multiplayer games easily with Haxe. No longer maintained and better options are available.
MIT License
125 stars 16 forks source link

Awesome event handling #18

Closed 5Mixer closed 8 years ago

5Mixer commented 8 years ago

Currently, an event is a fancy map that connects a string to a function. This causes several issues

Luxe has a pretty impressive event system, where you can group events under cateregories, grouping them. This could be done with regex. But regex sucks!

I came across https://github.com/VerbalExpressions/HaxeVerbalExpressions the other day. It is a library to create regex easily. This could be used.

As always, I want this library to not be limiting. I don't want to force regex onto people if they prefer strings. Others might prefer to have a type/enum for each event. For this, I really just have to make sure that EventManager can easily be swapped out and extended.

Breakfasttt commented 8 years ago

What about a class like:

class NetworkData
{
    var eventName:String;
    var rawData:Dynamic;

    public function new (event : String, rawData : Dynamic)
    {
        this.eventName = event.ToLowerCase();
        this.rawData = rawData;
    }
}

only serialize this class and send it on the network. 1- we can create another class and extends with this. (usefull for completion) 2- only use send (ndata :NetworkData)

5Mixer commented 8 years ago

At the moment, an object is serialised and sent over the network that looks like this:

{
    t: The_Event_Name,
    data: The_Data_Passed_Into_send()
}

I could make this a class, that is extendable, however I don't see why that would be needed, as any custom data can just go in the data field. This issue is more about make eventName more advanced than just a string. Nothing changes, in terms of what is being sent across the network, it just means that it is handled differently once it is received (the code will be changed in EventManager). This kind of addresses your first point. If you want to extend classes, have type safety, that's all fine and is easy to do in your code.

typedef MyPlayerData={x:Int,y:Int};
var dataToSend:MyPlayerData = {x:0,y:0};

Your second point (about making NetworkData typesafe) is fine in theory, but type safety isn't really required here. All the send method does is send it off, it never really accesses the objects fields. I hope you understand this.

anissen commented 8 years ago

I've made an (unfinished!) test of another approach for handling events. The concept is to make the events strongly typed by making the Server and Client classes generic and passing the the event type(s).

I've used enum's as types as I've previously had good experience with using them for encapsulating an event types. They provide a strong type with typed arguments.

Here's my test-commit: https://github.com/anissen/mphx/commit/6310a1bb6251a8a6a4fe40daee7e80f32ce426ba

Here's an example use case:

enum ClientEvent {
    Accepted(playerId :String);
    Moved(playerId: String, pos: { x: Int, y: Int });
}

enum ServerEvent {
    Join;
    Move(playerId: String, pos: { x: Int, y: Int });
}

class Main {
    public static function main() {
        var players = 0;
        var server :mphx.server.Server<ServerEvent> = null;

        function handler(event :ServerEvent, client :mphx.tcp.IConnection) {
            // Handle all events here, bonus: Automatic type and completeness check!
            switch (event) {
                case Join: client.send(Accepted('Player ' + (++players)));
                case Move(playerId, pos): server.broadcast(Moved(playerId, pos));
            }
        }

        server = new mphx.server.Server<ClientEvent, ServerEvent>('127.0.0.1', 8001, handler);
        server.start();
    }
}

The Server class instantiating the Connection class internally complicates things (e.g. the Server needing both the ClientEvent and ServerEvent-types). I would liked to be able to do something like this instead:

connection = mphx.tcp.IConnection<ClientEvent>();
server = new mphx.server.Server<ServerEvent>('127.0.0.1', 8001, handler, connection);

I would really like for the events to be strongly types but it also needs to be practical. Anyway, it's an idea.

yannsucc commented 8 years ago

Could we remove events? For example, by adding an event client.events.on("ping",onPing) for some check and remove it by client.events.remove("ping")

I suppose that client.events.on("ping",null) override the precedents event but there are no checks for null callback.

5Mixer commented 8 years ago

Yep, this needs to be made too. It's alpha alpha alpha. Alpha.

That should be fairly simple though, ~5 lines.

On 17 Mar 2016, at 9:49 pm, yannsucc notifications@github.com wrote:

can we remove an event actually ?

for exemple, add an event client.events.on("ping",onPing) for some check and remove it by client.events.remove("ping")

i supose that client.events.on("ping",null) override the precedents eventn but there are no check for null callback added

— You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub

yannsucc commented 8 years ago

Yep :) i think , i will make some pull request for some help . But i can't before 1or 2 weeks...

yannsucc commented 8 years ago

a little improvement with the actual events system : https://github.com/5Mixer/mphx/pull/21

Now :

5Mixer commented 8 years ago

Latest commit should throw a warning if an event that is unhandled is called, in debug mode.

I like @anissen's example, where type paramaters are used for events.This does complicate things a little for beginners though, who might have never even used type paramaters. It would be nice to have things work out of the box, without needing to understand the reason behind using type parameters. But without using them, it is not typesafe.

Perhaps it could be made typesafe simply by casting the string to whatever type you want to process events as. Casting might seem awful, but really, that's all that will happen using type parameters. It's just making the big string of data received type safe, but without using casts.

Casting has other benefits. By casting it inside callEvent, we don't need to change all the different platforms connections. In my opinion, as little as possible should be done in the individual connection classes. Any code that is not platform specific is probably best not to duplicate.

I think that making the callEvent method swappable is a nice, allowing custom handling of received events. Instead of making a public function callEvent we have a public var callEvent:String->Dynamic->IConnection. This has the upside that we can make this automaticly use the existing function, (modifying it to cast).

Edit: This post was terribly written. I'll make a commit that shows this. I'm actually thinking type paramaters might be the only way to go, but only within EventManager.

5Mixer commented 8 years ago

The event branch shows one possible way of doing events. It's much more complex and harder to setup, but gives all the control you could want. I'm not really sure where to go from here. I'd like both, but that's not too simple. What I'm really wanting is a complex and powerful system that defaults to a simple implementation anyone can use. It's possible I could have two versions of mphx, on different branches, but that's bound to cause issues.

If anyone has any input on the direction they'd like the project to take, shoot.

5Mixer commented 8 years ago

Closing. At this stage it's been too long since progress and I don't see it going anywhere anytime soon. I appreciate inputs into this conversation, but changing it at this point would break everything.

To do this in a way you like, you can just filter your events through some functions that convert your events to strings. So, for example, enums are serialized and their key is the event name, the value is the data. From their, server.send is called. Keep in mind that these enums must be reconstructed once received, if you want complete type safety. It seems almost silly doing something so small after this, but I feel like the issue is not as important as once thought.

If you have something to add, please feel free to comment.