Received Packet Handler

[Schuchie]

Hey,

currenlty you implemented a switch case for handling different packets. pkg/server/connection.go:138 This works with some packets, but if you add more and more, it will be pretty bad to handle/read/extend. So maybe build it more abstract and move each packet into an own module/handler. At the start of the server, each packet registers itself at the main handler. This should allow to easier extend it in the future. One thing i currently not sure about it, is how to implement a packet broadcast (e.g. send a chat-message to all).

Cheers

[r4g3baby]

Hello,

Thank you for showing interest in the project, however this is still under development and the current way of handling packets is just so I can get a very basic working server for testing. The plan is to eventually add an events system capable of handling all packets whether they are supported or not, and exposing it so anyone can extend it and add their own handling. With that said, I'll mark this issue as an enhancement until I get around to doing this.

As for the packet broadcast, at the current project state you would need to use the ForEachPlayer method to iterate over all the players and send the packet, keep in mind that there is still no chat message packet implemented so you would need to create your own and register it for each version with the proper packet ID for each supported Minecraft version, take a look at the registry for a better understanding.

[Schuchie]

Hey @r4g3baby,

I pulled your source code already and tested it locally. I also saw that there isnt a handler for chat packets. Im currently testing with your code to find a better way for handling the packets more abstract. Also to divide your packets folder into different cateogires as follow: handshake, login, play and status. But with the current implementation I get an import cycle (for obivous reason :D ).

[r4g3baby]

It's like I said, the project is still in the very early stages but I'll probably be working on adding support for more packets soon and the mentioned events system is something I've had on my mind for a while just haven't had the time to work on it yet. Don't see a need to separate the packets by folder, I think it goes against GO's code style rules as in everything that works together should be kept under the same pkg/folder (something like that :joy:), however if you do come up with a decent implementation feel free to open a pull request and I'll consider it.

[Schuchie]

Hello, Okey im currently testing some structures, but as i saw the current implementation has some design issues, which results in circular dependency or bad forced patterns by code. Of course i know its currently all in development :D

So one thing i noticed is that your connection struct shouldn't implement the handling of a packet. It should only read and write. So the handling should happen in the server struct, after the "connection.read()"-function returns something. Also im currently not sure if its so good to implement "business"/bigger logic into the player. It could be make some things harder to implement later. Maybe it would be a good idea to use the player only as dto (I know minecraft/spigot implements such logic into it).

But this are only some thoughts. I'm currently still brainstorming about the packet-handling.

[r4g3baby]

Hey,

Sorry for the late reply.

current implementation has some design issues Could you clarify what the design issues are?

not sure if its so good to implement "business"/bigger logic into the player Not sure what you mean by this either, could you clarify it please.

The external handling of all packets will be handled by an events system, the only handling done at the connection level is what's needed to keep the connection alive and updated. The server struct will handle the events.

[Schuchie]

Hey, so if you would add an event system which is handled by the server struct, the following scenario could happen: So a play is trying to chat. The server receives a chat packet. The server throws a player chat event, which then would trigger the player chat event handler. Normally the message should be broadcasted, which can be only done by the server struct. To achieve this you would need to create use the server struct in the chat event handler. This means that you need to inject the server struct into the chat handler. Which than should throw an import circular error, cause the server struct has an import on the chat event handler and the chat event handler imports the server struct. You could throw a new event from the chat event handler (like a broadcast event) which than the server would broadcast to all connections. But i think you will see some kind of this problem while implementing the event system.

Also what i meant with business logic is for e.g. the Kick-function in your player struct. Currently most of your player is more like an enhanced dto (with some mutex's for thread safeness). If you add an event system i would move the kick logic to an event (cause of event driven design). But this is only a suggestion.

[r4g3baby]

Hello,

I don't think the events system I have in mind will create a circular import, the server struct will handle the dispatch of the events and they will only be handled either on the connection struct or by another package using this as an api.

How would you go about implementing such logic using the events system? The plan is to expose the player struct and let anyone using it control the player (send chat message, kick, teleport, etc).

Also thank you for taking your time to help out, this is my first project in Go and I'm still learning from it.

[Schuchie]

Hey,

and how would the handler (e.g. Chat) broadcast a package? By throwing again a broadcast event?

If you implement the event system, from my site, i just would trigger an event from the player-struct or the server struct should have a kickPlayer-function which than triggers all. I would create for each event or pair of events an own handler. For example for the player kick i would create a PlayerKickEvent which than get handled by the PlayerKickHandler.

This event driven design would also allow to deactivate specific packets per config. If this server is only used for a Lobby, then there wouldnt be a case for Redstone or Block-Break/Place etc.

Im also not a pro at go, but i like to design an application :D

Edit: I found a way to outsource each packet-handler into an own "module/system". One thing im currently not a big fan of is that it still needs to add a import to the "package_registry". In my opinion there should be a way without that registry import (but i dont think that there is a way and its bad golang design). Also one question i got. Do you plan to use only one event-"channel" where all events getting published or one event for each type of packet (like chat, playermove etc.). I could make some performance difference. Maybee only one channel for all could be a bottleneck. https://stackoverflow.com/questions/28001872/golang-events-eventemitter-dispatcher-for-plugin-architecture

Also maybe some simple event driven system like this: https://github.com/gookit/event , but what would be the event-id.

[r4g3baby]

Hey,

how would the handler (e.g. Chat) broadcast a package? By throwing again a broadcast event? Events will only be used to process packets data, pretty much like the Spigot events. So for example we would have a PlayerChatEvent where we can get/set the chat message, this event would be emitted by the connection which would then use the result message from the event to broadcast it.

I found a way to outsource each packet-handler into an own "module/system" I'll just leave the packet handling at the connection level since that's where we will be doing most changes, if it does get complicated in the future we will have to come up with a different approach but I think we should be fine. I don't plan on implementing a full working Minecraft server with this project it's just the bare bones (useful for Lobbies, limbo and minigames servers).

Also maybe some simple event driven system like this: https://github.com/gookit/event , but what would be the event-id. Yeah, I plan on doing something like that however none of the eventbus libraries, including that one, seem to do exactly what I want. I've already started working on my own implementation, I should have a working example in a few days.

Edit: Just to clarify, what I'm trying to do here is create a server that can be fully customizable by whoever is implementing it. I'm currently using this as a "limbo" server with very minimal requirements but the plan is to add enough functionality to be able to create minigames or even a fully functional Minecraft vanilla server using this as a base.

[r4g3baby]

@Schuchie I've added basic support for events, let me know what you think about the way I implemented it.

Edit: I'll be adding an event for each packet read and write, the current chat event was just more of a test.

[Schuchie]

Hey @r4g3baby ,

i took a look into your changes. One thing as i mentioned already above, im personally not a fan of these big switch case's, which will getting bigger on each new packet. And maybe to keep it simpler the packet itself could be the event without any additionally event-struct. This would reduce the needed structs a lot.