search

godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
91.47k stars 21.26k forks source link

[TRACKER] Networking improvements #13947

Closed mhilbrunner closed 4 years ago

mhilbrunner commented 6 years ago

This issue is to collect and discuss improvements to the networking in Godot 3.

groud commented 6 years ago
  1. Why not, but I wonder how you can manage to do that without dirty workarounds.
  2. Would make sense, not to use the ICMP protocol, but having infos on the network makes sense to me.
  3. For LAN that would be cool
  4. Why not, but it is probably really hard to design a smart API for that.
  5. Don't know for this one.
  6. Like launching both from the editor ? Makes sense. 7 & 8. I don't think UPnP is meant to be used for games. Maybe it would be better as a plugin. Anyway, we're not here to provide networking solution, we'd better go for simple IP based API and let the network engineers provide plugins for specific needs.

Most of the ideas seems cool :)

mhilbrunner commented 6 years ago

@groud: For exposing ENet: It just seems a shame that multiple people had to work on their own Godot ENet implementation when it is already there, just not exposed sufficiently.

E.g. https://github.com/PerduGames/gdnet3, https://github.com/Faless/godot-enet-better etc.

Thanks for the feedback!

mhilbrunner commented 6 years ago

Regarding 2: Would the best way be to implement a "heartbeat", a packet that gets periodically sent by each peer at a configurable interval to measure latency (and make that information then available on the peer/SceneTree)?

Calinou commented 6 years ago

I think "Support for movement interpolation & reconciliation (lag compensation, see article)" can be highly game-specific, and probably can't be implemented as-is, since the implementation methods may vary a lot depending on the game genre.

Providing built-in support for Hermite interpolation could be a good idea, and it can be used in areas other than networking, see this article for more details.

mhilbrunner commented 6 years ago

According to reduz, ENet channels are not supported on mobile. So if they can be exposed at all, this probably has to be taken into account.

nasdf commented 6 years ago

I'd like to help with launching multiple instances for server and clients. Does anyone have a good idea of how this should work? I can think of two solutions for this issue.

  1. Have a button to run an instance in server mode separate from the main scene
  2. Allowing the NetworkedMultiplayerENet to be attached to a child node instead of the scene tree
Faless commented 6 years ago

Opening some old wounds :wink: ...

  1. Can we? We should check OS capability but I don't think we can, IPv6 link-local are already excluded (as they don't really works as expected), how can we identify loopback address in a cross platform way?
  2. Will need to expand the multiplayer protocol for that, but sounds like a good idea, should be too hard.
  3. That sounds too implementation specific.
  4. Interpolation should be easy to implement for physics, reconciliation and lag compensation is complex, and sounds too implementation specific. Also, it's an entry point for cheaters so at least is should be possible to disable it.
  5. No ENet DOES support channels on mobile. It's other API that does not (i.e. the Google Multiplayer API). I was a strong supporter of having this exposed to the multiplayer API itself, and just do custom implementation or just signal an error when using them with library that does not support them. It just seems not smart to hinder many implementation (WebRTC has channels too) just because few less used libraries does not have it.

  6. 7008 is about launching two different instances, what you seems to want is to have both server and client in the same instance. Another old wound, I deeply believe that the network thing should not be on the SceneTree but on a specific MultiplayerNetworkNode, that behaves the same as SceneTree does now, but only handling its children. At some point I had a working implementation of this (probably broken now). See https://github.com/Faless/godot-enet-better/tree/rpc_enable . And the patch to godot: https://github.com/Faless/godot-enet-better/blob/rpc_enable/node_rpc.patch . This is important for games that wants to run both server and client in the same instance, a typical example is Quake III arena which used to run a server on localhost even when playing single player. (allows for less code duplication.)

  7. Would be really nice to have, I did start studying it at some point, but never implemented something useful due to lack of time. This could also solve 3 in a way that does not clutter the engine too much.
  8. That is technically already done by the multiplayer server peer as the API itself is not fully peer to peer (so to allow for authoritative servers). This is very implementation specific, depends on the underlying layer (eg. WebRTC with Websockets STUN servers) etc. Wouldn't make much sense to have a Godot instance running just as a STUN server basically.

@reduz sorry about bringing this up again, I was kinda dragged to it, but I really care about point 6 (and a to point 5 too :wink: ).

nasdf commented 6 years ago

@Faless Your patch for issue 6 is exactly what I was looking for. It's currently a huge pain point when developing games with dedicated servers.

reduz commented 6 years ago

Some answers I guess

  1. No idea how to do this
  2. Seems implementation specific,
  3. Seems implementation specific, and likely outside the scope of multiplaye API
  4. Seems game specific
  5. Not against channels, but will not work for blutooth/online network on Google/Apple APIs. If you want to port your game there and you depend on the feature, it won't work.
  6. Too game specific, but maybe a tool to launch multiple godot instances would be useful
  7. Indeed would be nice
  8. This is how it works currently.
mhilbrunner commented 6 years ago

@reduz Just to clarify, can you explain what you mean by "implementation specific"?

reduz commented 6 years ago

By the way, nothing prevents from adding extra functions to MultiplayerNetworkPeerENet, even if not in the base class. Something like set_send_channel() may work (not sure if there is any point on checking what channel the data has been received on, since RPC already handles this)

reduz commented 6 years ago

you could do something like.

peer.set_send_channel(10)
rpc("blabla")
peer.set_send_channel(0)
reduz commented 6 years ago

@mhilbrunner Not sure if all APIs measure ping, latency, packet loss, etc. the same way. If ENet has methods for this, i would just expose them in the ENet object, not in the base class. For a more generic way, you can do it by user code (just send a packet and see how long it takes to come back).

Discovery is also completely implementation specific. For IPV4 you can send packets to all your subnet to test, or broadcast. IPV6 is likely different, but you have already other network classes to do this, does not need to be part of the multiplayer API.

mhilbrunner commented 6 years ago

@reduz The above idea is interesting. But couldn't it potentially cause weird behaviour/race conditions in case of multithreading?

Just to compare to another engine, Unity has the following (the others are not applicable):

2 (Ping): https://docs.unity3d.com/ScriptReference/Network.GetAveragePing.html

3 (Discovery): https://docs.unity3d.com/Manual/UNetDiscovery.html

4 (Interpolation): https://docs.unity3d.com/ScriptReference/Networking.NetworkTransform.html

6 (Start Network+Client in same app): https://docs.unity3d.com/ScriptReference/Networking.NetworkManager.StartHost.html

7 & #8 (NAT traversal/punchthrough/relay servers): Supported in UNet + via plugins

The story for UE4 likely looks similar; I know it can launch a configurable number of clients at "Play" and supports interpolation, at least.

For now, I'll look into ping, discovery and the channels. I personally don't need NAT traversal currently :) If someone wants to chime in on any of these or help out, PRs welcome, of course :) As well as helpful pointers.

ENet seems to already have support for average pings, which may not be optimal but could be a starting point.

reduz commented 6 years ago

@mhilbrunner no, because the networking API is not meant to be used from multiple threads

Regarding those functions, ping may make sense given you need it to work on both ends, which is why I suggested checking if ENet has something like this.

But discovery, interpolation and host, we can provide a script in the Asset Library, given there isn't a single way to do it.

Faless commented 6 years ago

Regarding those functions, ping may make sense given you need it to work on both ends, which is why I suggested checking if ENet has something like this.

ENet does something internally, but I've read around it's not to be used as a good latency metric.

Addiontally, we might want to have it on other libraries too (e.g. Websocket, WebRTC).

Given that we already have a small protocol for managing peers (see SYSMSG_ADD_PEER, SYSMSG_REMOVE_PEER), adding SYSMSG_PING and SYSMSG_PONG and calculating avarage over time should be really simple.

  1. Too game specific, but maybe a tool to launch multiple godot instances would be useful

@reduz , this is something that at the same time adds a new feature, makes debug easier, unclutters SceneTree, unclutters Object and most of all moves the Multiplayer related code in it's own class/classes where it belongs, I really can't understand why you are against it.

I'd love to do it myself and show it to you if there's the smallest chance you would change your mind (pretty please :wink: ).

reduz commented 6 years ago

Current API is meant to be simple, and I see no problem in adding a tool for launching (or even debugging) multiple instances to test. Making a worse API just for the hack of testing all scenes together does not seem good to me.

Again, feel free to make a separate library via gdnative if you want something more specific.

Faless commented 6 years ago

Current API is meant to be simple

And that is not going to change. The idea is to keep the same API, with the small addition of also allowing a node to the same work that is now done by SceneTree. The API itself will not change, current demos will keep working as they do now with no change.

feel free to make a separate library via gdnative if you want something more specific.

That is not going to be possible without huge and unmaintainable copy-paste from core files, unless we separate the Multiplayer protocol from the core.

I guess I'll try to make a PR that adds no more than few lines of code and respect those two conditions (same API, separated protocol) to at least allow for a gdnative implementation without rewriting a lot of code, I'm confident that once you see it, you will like it, as again there will be no change to the API (nor the current SceneTree behaviour).

reduz commented 6 years ago

Changes need to be made on a use case basis. If this work is only to make it easier to test multiplayer code, then It's definitely undesired.

Faless commented 6 years ago

Changes need to be made on a use case basis

The many developers that kindly asked to allow for running both client and server in the same instance because their game requires it, or because it makes their code way better decoupled and not redundant.

EDIT: And I brought the example of Quake III, but any FPS or even turn based game would benefit from it

reduz commented 6 years ago

The current design is not meant to run as client and server with two simultaneous instances. The server, as things are now, can run as a client without any difficulty and without being redundant. If debugging is the problem, then the debugger can be modified to support launching and debugging multiple instances at the same time (it should be pretty easy to do this).

Again, what you are proposing is considerably more complex than what is in there and I don't really see any advantage. Supplying it externally should really not be a problem.

reduz commented 6 years ago

If all your miss are the RPC functions, a wrapper could easily be done so they call your node instead of SceneTree, then you can handle them as you want.

reduz commented 6 years ago

Regarding Quake III, this is really ancient multiplayer design. I'm not sure why you would want to go with that approach. Reminds me of Torque, which was completely unusable.

Faless commented 6 years ago

Regarding Quake III, this is really ancient multiplayer design

EDIT: As I said at the beginning, it's an old wound, I don't want to argue about this,

I'll see if I can at least unclutter SceneTree and Node to have the networking part separated in the code. We'll see then if it's worth merging or not, at this point I won't mind any wasted work

reduz commented 6 years ago

Let me rephrase myself. I see zero advantage over the current approach, which is simpler and can perfectly act as server authoritative.

Running both a client and a server in the same process only makes implementation more complex and provides nothing in exchange. Why should a considerably more bothersome approach need to be supported when the default one provided serves fine for large majority of use cases?

Faless commented 6 years ago

Why should a considerably more bothersome approach need to be supported when the default one provided serves fine for large majority of use cases?

Because it's not considerably more bothersome, because the current situation makes your life impossible in some (minority) of use cases and makes people that hates singletons like me cry :sob: .

:wink:

reduz commented 6 years ago

What minority of use cases are the problem? Why are singletons a problem?

mhilbrunner commented 6 years ago

Yes, running the client and server in the same process is some additional complexity. However, for some apps, its really needed. If you are creating a game where everyone can create a server (and you don't host master servers), its really nice to be able to launch both in the same app.

Otherwise, you'd have to tell your customer "Uh, you need to first start this other application in the background and keep it running". There is a reason Unity supports it.

For example, both Minecraft and Klei's Don't Starve games do this for multiplayer: The server is basically always running in the app, and if you play single-player, you just connect as a local client in the same app; if others want to join and you allow it, it just opens your local server up.

Additionally, I really agree with @Faless that decoupling networking from SceneTree a little more would make the code cleaner, too.

But yes, being able to start server and client (or server and multiple client) instances via pressingPlay in the editor would be a nice start.

reduz commented 6 years ago

@mhilbrunner I think you are misunderstanding the situation. Currently it works as you describe, the server is also a client and you don't need to launch two processes. What faless proposes is making the server not be a regular peer but a separate entity, and bundle both it and a client together.

This is how networking in old engines like Torque used to work. It's more complicated and there really isn't any advantage.

mhilbrunner commented 6 years ago

@reduz Oh, sorry, then I retract the above :) Are there docs on this somewhere? Still, launching multiple Godot instances via play would be cool.

reduz commented 6 years ago

@mhilbrunner Yeah, definitely need to add a way to launch and debug multiple instances

Calinou commented 6 years ago

@Faless's desired behavior could be implemented by making the client start a dedicated server, which will be possible once the headless server platform is re-implemented. That said, it's a convoluted way of doing things as the workflow @reduz mentioned works just as well in practice.

Faless commented 6 years ago

@Calinou @reduz I'm not talking about changing the current behaviour, I'm asking for something like this (i.e. moving the network code from SceneTree and Node to a MultiplayerAPI class, while allowing for customizations when needed). That stubby patch is fully compatible with the current system, you can try with the multiplayer bomber demo. (Although I have to say I'd prefer to see all net related signals and functions moved from get_tree() to get_tree()->get_multiplayer_api() which will also allow for a single point of documentation instead of having it amid SceneTree functions). At the same time it allows gdnative/gdscript/whatever to customize it. In ~100 lines more, mainly headers and bindings.

See customization with gdscript here: multiplayer_demo.zip (combo.tscn is the customized version, standalone.tscn shows the compatible version).

reduz commented 6 years ago

@Faless I understand perfectly what you want to do, but the problem is not the amount of lines of code required to implement your approach, but the approach itself. Your arguments are solely about personal taste, not limitations on the current design that could justify making it more complex.

It's not a "this type of game can't be made with the current approach" or "this type of game is more difficult to do with the current approach". Your argument is is simply "I would like to run server and client on the same game instance", which sounds cool but in practice it's pretty useless. Then you shift your arguments to pure distaste for singletons, which is not really an argument with technical merit. Afterwards you try to make focus on the documentation, which is still not really a problem.

To sum up, I feel you are proposing something that doesn't have any practical use (you seem to be coming up with different reasons for it every time, which is even less convincing to me) and is more confusing than the current approach for no reason.

Please understand you are not giving me any meaningful reasons to change the current approach.

Faless commented 6 years ago

I haven't changed reasons, the rationale has always been the same: allow customization from scripting language (either for creating client/server or protocol extensions or whatever) while having better organized code.

You keep saying it's a more complex approach, I fail to see that, as it won't change anything for people who uses the get_tree() system (as I wrote, fully compatible with the current demos). This would only allow for more customization IF and only IF a gamedev want, at practically no cost.

I don't see why we shouldn't allow that when it actually changes nothing for other regular users and makes the code more structured.

But alas, I feel I tried hard enough to convince you, I guess some things just can't be changed.

On Dec 25, 2017 10:54, "Juan Linietsky" notifications@github.com wrote:

@Faless https://github.com/faless I understand perfectly what you want to do, but the problem is not the amount of lines of code required to implement your approach, but the approach itself. Your arguments are solely about personal taste, not limitations on the current design that could justify making it more complex.

It's not a "this type of game can't be made with the current approach" or "this type of game is more difficult to do with the current approach". Your argument is is simply "I would like to run server and client on the same game instance", which sounds cool but in practice it's pretty useless.

To sum up, I feel you are proposing something that doesn't have any practical use (you seem to be coming up with different reasons for it every time, which is even less convincing to me) and is more confusing than the current approach for no reason.

Please understand you are not giving me any meaningful reasons to change the current approach.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/godotengine/godot/issues/13947#issuecomment-353855955, or mute the thread https://github.com/notifications/unsubscribe-auth/ABnBbuovIFMsYwYp04mMTrSKvY6BNS6sks5tD3DMgaJpZM4Q0IbV .

reduz commented 6 years ago

@Faless I still think you don't understand my point of view. Let's agree on the following:

As such, what I propose is making sure you have elements that would allow you to implement something like this, externally. The only thing that comes to mind is that you may want to reuse the rpc functions. If so, it may be possible to add some abstraction in the middle that allows you to intercept those.

But what is clear to me is:

Faless commented 6 years ago

The feature you want does not need to be used often,

Agreed

The feature you want, (be it slightly or a lot) adds complexity to the current system

I totally disagree. No changes at all from a gamedev prospective, from an engine prospective if anything it lowers the complexity, as it keeps the same algorithm and protocol while moving all network related variables and functions in a single place which means easier to debug, easier to maintain.

I don't see any reason to make this core, as it's clearly more code and more complex API for a highly corner case.

Agreed, no need to make this core, just expose the functions to allow users to do it via script, since as of now it's IMPOSSIBLE to do, unless you want to copy paste those 800+ lines of code from SceneTree and Node (and patch them).

I don't see any reason why this can't be implemented externally to Godot, via GDNative. If hooks are missing to allow you reuse more API, this can be added.

That's exactly what the diff I linked does. Group functions, expose them so they can be used from GDNative/GDScript. No change other than that! (i.e. no protocol change, no user API change, full compatibility with current system, just grouped and exposed code)

thimenesup commented 6 years ago

I agree that we could have the ENet lib a bit more exposed, for example having raw access to the peer IP and port would let you build your own match discovery and/or NAT Traversal/Punchthrough

Nodragem commented 6 years ago

As a gamedev, I think the major points in your original todo-list are:

@reduz seems to argue that the second point is "game specific"... that is true, but it is unclear whether this is an argument against implementing it?? If so, I would say that rendering in 2D or 3D is game specific and you offer both.

Anyway, thank you all for your work, can't wait to see the next improvements to the multiplayer support.

Neicul commented 6 years ago

But discovery, interpolation and host, we can provide a script in the Asset Library, given there isn't a single way to do it.

That would be great, to at least give a starting point for inexperienced network programmers like me 🤓

kyledayton commented 6 years ago

I would like to see options in the editor to simulate latency, packet loss, jitter, bandwidth restrictions, etc. Being able to easily test various network conditions locally would be immensely useful.

Enhex commented 6 years ago

@kyledayton There's a standalone tool for that called Clumsy: https://jagt.github.io/clumsy/

Malkverbena commented 6 years ago

Not against channels, but will not work for blutooth/online network on Google/Apple APIs. If you want to port your game there and you depend on the feature, it won't work.

This is a relevant feature of Enet and very useful. Would not it be possible to enable the channels for those who need it and disable it for those who do not? Or maybe expose this in GDScript? Something like "multiplayer_api.enable_channels".

mhilbrunner commented 6 years ago

@Malkverbena Yes this is possible for ENet. Looking into it.

mhilbrunner commented 6 years ago

ENet's channels are now exposed: https://github.com/godotengine/godot/pull/18827

mhilbrunner commented 6 years ago

fales did an awesome job removing lots of duplicated code by unifying the socket implementation across platforms: https://github.com/godotengine/godot/pull/21692

Nodragem commented 6 years ago

does the unification across platforms means that the networking will work for html5 games?

Faless commented 6 years ago

@Nodragem networking in general works in HTML5, high level networking is only supported by the websocket module (only suitable for non-realtime/turn based games). An amazing work on WebRTC (for real time games) has been done by @brandonmakin during this summer, but it's still unmerged as it still needs some polishing. We were discussing the possibility of including it in 3.1 as an "experimental feature" to get better user feedback on it, but it might as well be pushed to 3.2.

See #14627 .

Ploppy3 commented 5 years ago

The above improvements seems to be pushed to 3.2

Since UPNP is not reliable, what are the alternatives?

Faless commented 5 years ago

The above improvements seems to be pushed to 3.2

@Ploppy3 if you mean WebRTC, yeah, sadly I didn't have enough time to finish clean it up and PR it in time ( :-1: for me) and it's postponed to 3.2.

Since UPNP is not reliable, what are the alternatives?

Not sure I understand you, UPNP is used mainly for discovery (and few other things) but it's not meant as a "transport protocol for user data", so I'm not sure what you mean by unreliable (beside that it uses UDP for discovery, but then again, you shouldn't bother about it, because you are not transmitting your game data with it).

Right now, there is no unreliable high level multiplayer peer for HTML5, only a reliable one: WebSocket (see docs). So you can already use the RPC system in HTML5 exports, but it's not really suitable for real time games (it's worth noticing that a game like WoW used TCP, so it's still possible in same cases to have close to real time games using reliable transport. although I wouldn't recommend that in general).