Networking See PR #532 #665

jojolepro · Apr 24, 2018

See PR #532 for all comments. This is just the same re-opened because bors won't merge the other one.

This change is

Rhuagh · Apr 24, 2018

Rhuagh approved these changes Apr 24, 2018

View changes

Xaeroxe · Apr 24, 2018

Xaeroxe approved these changes Apr 24, 2018

View changes

bors r+

OvermindDL1 · Apr 24, 2018

OvermindDL1 approved these changes Apr 24, 2018

View changes

Build failed

continuous-integration/travis-ci/push

bors · Apr 24, 2018

Build failed

continuous-integration/travis-ci/push

cache got too big 0_o

bors retry

Xaeroxe · Apr 24, 2018

cache got too big 0_o

bors retry

nice
I guess I won't be doing any big PR anytime soon.

jojolepro · Apr 24, 2018

nice
I guess I won't be doing any big PR anytime soon.

Build failed

continuous-integration/appveyor/branch

bors · Apr 25, 2018

Build failed

continuous-integration/appveyor/branch

bors retry

Xaeroxe · Apr 25, 2018

bors retry

omni-viral · Apr 25, 2018

omni-viral requested changes Apr 25, 2018 • edited

View changes

I suggest to use multi-level event handling.

Designated thread does polling mio and sends tasks to thread-pool.
Thread-pool handles network events.
It may precompute some data, collapse multiple network events into single one and so on.
Events that affect World enqueued into queue which is processed by System.
Network events and input events can be handled by that system together as there are similarities. Like character controlling events from players local and remote.

And I'd want real examples. Like sending pong state over the network.

Also it may be better to use more abstract data sources than sockets.
There are crates for async IO which should be useful here.

omni-viral · Apr 25, 2018

amethyst_network/src/connection_manager.rs

+        }
+
+        for ev in events.buf.read(self.net_event_reader.as_mut().unwrap()) {
+            if self.is_server {


Looks like this system should be split in two.

omni-viral · Apr 25, 2018

amethyst_network/src/net_event.rs

+    /// Notify the clients(including the one being disconnected) that a client has been disconnected from the server.
+    Disconnected {
+        /// The reason of the disconnection.
+        reason: String,


Disconnect and other reasons should be an enum of well-known reasons + custom one with String.

omni-viral · Apr 25, 2018

amethyst_network/src/net_event.rs

+        reason: String,
+    },
+    /// A simple text message event.
+    TextMessage {


How this one must be interpreted by engine user?

info!("")
Those are arbitrary debugging messages.
For example "Client 1. Time is: 3254, ping is: 6451, client state: Connected".

I can't see it from variant name though. Neither engine user will do

I can rename it DebugTextMessage.

Designated thread does polling mio and sends tasks to thread-pool.

We'll have thread starvation issues, as we have with audio already when the game is under heavy load.

Network events and input events can be handled by that system together as there are similarities. Like character controlling events from players local and remote.

You can do that by simply using a EventChannel<NetEvent> in any system you want.

And I'd want real examples. Like sending pong state over the network.

Me too, but I have to wait for specs 0.11. I also have to rethink how I want the entity ownership to work. Mainly, I need to tag components I want to sync (Marked?) and then find a way to deserialize them on the receiving end without actually having the type. Basically the same problem that I had with NetEvent, but I can't use an enum...

Also it may be better to use more abstract data sources than sockets.

I already talked about it. Recap: Its a planned feature, but it will require a completely separate abstraction PR for it. (I'm planning on doing it in the 3rd one, while this is the first one. The second one will be component/entity sync/creation/destruction/ownerships).

There are crates for async IO which should be useful here.

Except for tokio and a couple of lower level subcrates, I didn't see much that were promising and didn't require a completely separate thread. Since most people are against using futures, the choice is pretty limited.

jojolepro · Apr 25, 2018

Designated thread does polling mio and sends tasks to thread-pool.

We'll have thread starvation issues, as we have with audio already when the game is under heavy load.

Network events and input events can be handled by that system together as there are similarities. Like character controlling events from players local and remote.

You can do that by simply using a EventChannel<NetEvent> in any system you want.

And I'd want real examples. Like sending pong state over the network.

Me too, but I have to wait for specs 0.11. I also have to rethink how I want the entity ownership to work. Mainly, I need to tag components I want to sync (Marked?) and then find a way to deserialize them on the receiving end without actually having the type. Basically the same problem that I had with NetEvent, but I can't use an enum...

Also it may be better to use more abstract data sources than sockets.

I already talked about it. Recap: Its a planned feature, but it will require a completely separate abstraction PR for it. (I'm planning on doing it in the 3rd one, while this is the first one. The second one will be component/entity sync/creation/destruction/ownerships).

There are crates for async IO which should be useful here.

Except for tokio and a couple of lower level subcrates, I didn't see much that were promising and didn't require a completely separate thread. Since most people are against using futures, the choice is pretty limited.

Thanks for the review, btw ;)

jojolepro · Apr 25, 2018

Thanks for the review, btw ;)

We'll have thread starvation issues

I see network polling once in 17ms (60 fps) as starved already. Also. If we will have control over that thread we could ensure it does polling frequently enough. With audio we have no control over playback thread.

You can do that by simply using a EventChannel

Is EventChannel<NetEvent> capable to broadcast all of network events under heavy load?

Since most people are against using futures

I can't understand why people are against futures. We tried to use them wrong and went into troubles. Well. Next time we can try to use them right 😏

omni-viral · Apr 25, 2018

We'll have thread starvation issues

I see network polling once in 17ms (60 fps) as starved already. Also. If we will have control over that thread we could ensure it does polling frequently enough. With audio we have no control over playback thread.

You can do that by simply using a EventChannel

Is EventChannel<NetEvent> capable to broadcast all of network events under heavy load?

Since most people are against using futures

I can't understand why people are against futures. We tried to use them wrong and went into troubles. Well. Next time we can try to use them right 😏

Is EventChannel capable to broadcast all of network events under heavy load?

Yes. I made it run at 10k events per frame.

I can't understand why people are against futures. We tried to use them wrong and went into troubles. Well. Next time we can try to use them right

I like futures, but the problem is that they can complete at any time. Forking the execution thread is easy but merging it back into the main execution loops is much harder. The way to solve it is to use the equivalent of unity's coroutines: You run your future normally, but when it completes, it stores the result in some temporary buffer (locking). Then from the main loop (in the sense that it has to happen during an update() frame) you get that result and call the action that is supposed to happen. This is a limitation inherent to the fact that amethyst is frame-based.

Frame based: It starts with a state, applies a set of instructions on it and ends up with a new step.

Futures are better with completely async systems: You can fork the execution as much as you want because you are not required to have a global end state to compute.

(My scala engine I wrote was a completely async, actor + ecs based. The problem with that is that you cannot have dependency cycles at all. You can't have physics depending on the position that depends on physics that depends on AI.)

I didn't mean to write so much on this lol

jojolepro · Apr 25, 2018

Is EventChannel capable to broadcast all of network events under heavy load?

Yes. I made it run at 10k events per frame.

I can't understand why people are against futures. We tried to use them wrong and went into troubles. Well. Next time we can try to use them right

I like futures, but the problem is that they can complete at any time. Forking the execution thread is easy but merging it back into the main execution loops is much harder. The way to solve it is to use the equivalent of unity's coroutines: You run your future normally, but when it completes, it stores the result in some temporary buffer (locking). Then from the main loop (in the sense that it has to happen during an update() frame) you get that result and call the action that is supposed to happen. This is a limitation inherent to the fact that amethyst is frame-based.

Frame based: It starts with a state, applies a set of instructions on it and ends up with a new step.

Futures are better with completely async systems: You can fork the execution as much as you want because you are not required to have a global end state to compute.

(My scala engine I wrote was a completely async, actor + ecs based. The problem with that is that you cannot have dependency cycles at all. You can't have physics depending on the position that depends on physics that depends on AI.)

I didn't mean to write so much on this lol

Its probably possible to have a separate thread to poll the network socket, but the events will be consumed by the ecs only when the update() loop is there. Might as well just poll the socket during the loop at that point. Mio prevents packet loss slow stepping, from what I have seen.

jojolepro · Apr 25, 2018

Its probably possible to have a separate thread to poll the network socket, but the events will be consumed by the ecs only when the update() loop is there. Might as well just poll the socket during the loop at that point. Mio prevents packet loss slow stepping, from what I have seen.

Its a separate topic, but if someone can come up with an architecture that allow using Futures in a stepping engine without requiring locking buffers (or buffers at all), please open a rfc.

jojolepro · Apr 25, 2018

Its a separate topic, but if someone can come up with an architecture that allow using Futures in a stepping engine without requiring locking buffers (or buffers at all), please open a rfc.

Currently refactoring, the solution just jumped in my face. Instead of having things like NetConnectionPool that is a resource containing all the connections, then try to bind that to a NetIdentity or whatever the heck I was doing, I'll just create an entity and put all the things the connection OWNS on the entity. Not sure why I'm so blind that I didn't see that before.

jojolepro · Jul 9, 2018

Currently refactoring, the solution just jumped in my face. Instead of having things like NetConnectionPool that is a resource containing all the connections, then try to bind that to a NetIdentity or whatever the heck I was doing, I'll just create an entity and put all the things the connection OWNS on the entity. Not sure why I'm so blind that I didn't see that before.

Rhuagh added type: feature status: working project: networking labels May 17, 2018

amethyst/amethyst

Join GitHub today

Networking See PR #532 #665

Conversation

jojolepro commented Apr 24, 2018 • edited by torkleyy Edited 1 time torkleyy edited Apr 24, 2018 (most recent)

jojolepro added some commits Feb 1, 2018

Rhuagh approved these changes Apr 24, 2018 View changes

Xaeroxe approved these changes Apr 24, 2018 View changes

OvermindDL1 approved these changes Apr 24, 2018 View changes

bors bot added a commit that referenced this pull request Apr 24, 2018

This comment has been minimized.

bors bot Apr 24, 2018

Build failed

bors bot commented Apr 24, 2018

Build failed

This comment has been minimized.

Xaeroxe commented Apr 24, 2018

This comment has been minimized.

jojolepro commented Apr 24, 2018 • edited Edited 1 time jojolepro edited Apr 24, 2018 (most recent)

bors bot added a commit that referenced this pull request Apr 25, 2018

This comment has been minimized.

bors bot Apr 25, 2018

Build failed

bors bot commented Apr 25, 2018

Build failed

This comment has been minimized.

Xaeroxe commented Apr 25, 2018

omni-viral requested changes Apr 25, 2018 • edited View changes

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

omni-viral Apr 25, 2018 • edited Edited 1 time omni-viral edited Apr 25, 2018 (most recent)

This comment has been minimized.

This comment has been minimized.

jojolepro commented Apr 25, 2018

This comment has been minimized.

jojolepro commented Apr 25, 2018

This comment has been minimized.

omni-viral commented Apr 25, 2018 • edited Edited 1 time omni-viral edited Apr 25, 2018 (most recent)

This comment has been minimized.

jojolepro commented Apr 25, 2018 • edited Edited 1 time jojolepro edited Apr 25, 2018 (most recent)

This comment has been minimized.

jojolepro commented Apr 25, 2018

This comment has been minimized.

jojolepro commented Apr 25, 2018

Rhuagh added type: feature status: working project: networking labels May 17, 2018

jojolepro added some commits Jun 20, 2018

This comment has been minimized.

jojolepro commented Jul 9, 2018

jojolepro added some commits Jul 11, 2018

jojolepro commented Apr 24, 2018 •

edited by torkleyy

Edited 1 time

torkleyy edited Apr 24, 2018 (most recent)

Rhuagh approved these changes Apr 24, 2018

View changes

Xaeroxe approved these changes Apr 24, 2018

View changes

OvermindDL1 approved these changes Apr 24, 2018

View changes

jojolepro commented Apr 24, 2018 •

edited

Edited 1 time

jojolepro edited Apr 24, 2018 (most recent)

omni-viral requested changes Apr 25, 2018 • edited

View changes

omni-viral Apr 25, 2018 •

edited

Edited 1 time

omni-viral edited Apr 25, 2018 (most recent)

omni-viral commented Apr 25, 2018 •

edited

Edited 1 time

omni-viral edited Apr 25, 2018 (most recent)

jojolepro commented Apr 25, 2018 •

edited

Edited 1 time

jojolepro edited Apr 25, 2018 (most recent)