Kehom / GodotAddonPack

A collection of pure GDScript addons for Godot
http://kehomsforge.com
MIT License
183 stars 15 forks source link

Add Websocket Support #9

Closed fairhat closed 3 years ago

fairhat commented 3 years ago

Allows to use "create_websocket_server" to create a websocket server Compression method and max_players is ignored as this is currently not implemented in the websocket implementation of godot

Max_Players could be simulated by counting the number of players manually and refusing new connections once the player count is equal to max_players

Afaik there's no way to get compression working for now

Allows clients to join a websocket server with "join_websocket_server"

This is far from perfect as there is still a bug where client disconnects are still not properly synchronized with other clients (even though the node is removed from the server)

The Websocket Implementation is compatible with Godots High Level Multiplayer so the rest should work fine. However, i don't think rpc_unreliable is actually unreliable since websocket packets are always delivered in order and reliably (due to websocket limitations/being TCP connections)

fairhat commented 3 years ago

Do you have any test runner implemented? If yes, i could also write tests for it

Kehom commented 3 years ago

I have a different idea to implement websocket server support, which is through a project setting. Or you prefer having separate functions to create servers? Personally I don't think it is needed to have multiple functions, specially because one could just change a setting on the ProjectSettings window and no change in the code would be necessary.

Regarding the compression, I was thinking about ditching the automatic compression method and manually do since the addon is directly dealing with PoolByteArrays anyway. There are a few things to keep in mind when manually dealing with compression, though:

Regarding the client disconnection, if the server is being notified then a workaround is possible.

I will have to work a little bit with websockets in order to better understand the "nature of the beast" as I honestly never worked with those.

fairhat commented 3 years ago

I have a different idea to implement websocket server support, which is through a project setting. Or you prefer having separate functions to create servers? Personally I don't think it is needed to have multiple functions, specially because one could just change a setting on the ProjectSettings window and no change in the code would be necessary.

No, Project setting would be even better!

Regarding the compression, I was thinking about ditching the automatic compression method and manually do since the addon is directly dealing with PoolByteArrays anyway. There are a few things to keep in mind when manually dealing with compression, though:

* We loose the RangeCoder method, which is given by ENet.

* It will result in the internal monitor/profiler/debugger to show the actual saving between compression methods. Apparently it is showing only the "raw" data usage, after any compression method got used.

Do you mean it will NOT show the compressed data usage or it will SHOW it ?

* It will possibly work with websocket server.

It should, i asked Faless on IRC the other day and i was told that Compressing PoolByteArrays is the way to go on Websockets

Regarding the client disconnection, if the server is being notified then a workaround is possible.

This might actually be related to something entirely different. Not sure if that was actually a bug or my computer.

I will have to work a little bit with websockets in order to better understand the "nature of the beast" as I honestly never worked with those. As long as you are using the high level multiplayer api and rpc/rset calls (not put_packet or get_packet) there shouldn't be any difference except no compression methods and different constructor calls to join/create servers.

Edit: Clients should also call peer.poll() on every physics tick (browsers do this automatically but the Windows/Linux/Android..etc clients don't)

I'll try to update the PR maybe i can get to the point where it meets your requirements

fairhat commented 3 years ago

Updated PR: Websocket is now configurable over ProjectSettings

Kehom commented 3 years ago

I intend to merge this PR. I just hope you are OK if I do a few changes here and there after the merging. To be more specific:

Regarding the network profiler. I see now my phrase was poorly constructed! The thing is, currently it does not seem to take any compression into account. I have tried all possible methods and the profiler shows the exact same data usage, which I'm sure is not the case. I did dig into the source code and from what I was able to understand, it only takes the "raw data usage". From all of this, if the addon performs the compressions manually, the profiler will end up giving as a better picture of the actual savings as the delivered data will still be compressed. The interesting thing, I was already planning to do something like this, but didn't do because I wanted to keep the RangeCoder as an option.

Clients should also call peer.poll() on every physics tick (browsers do this automatically but the Windows/Linux/Android..etc clients don't)

Isn't this requirement for the low level API? Or the Websocket needs this regardless?

fairhat commented 3 years ago

I intend to merge this PR. I just hope you are OK if I do a few changes here and there after the merging. To be more specific:

* I'm trying to use static typing as much as possible, not using it only when it "can't".

* From the "public" create/join server, call a "private" function based on the setting within the ProjectSettings window, instead of bailing after setting the websocket.

Feel free! 😄

Isn't this requirement for the low level API? Or the Websocket needs this regardless? Might be unintended, but even on high level multiplayer you need to poll Also the server needs to poll, too.

Something i noticed on HTML5 Export (at least on Safari on iOS) is that the server does not trigger "peer_disconnected" if I just close the tab. But i guess this is something that should be fixed in Godot