Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

About the new node API: How do I keep control ? #69

Closed
Uriopass opened this issue Apr 9, 2021 · 7 comments
Closed

About the new node API: How do I keep control ? #69

Uriopass opened this issue Apr 9, 2021 · 7 comments

Comments

@Uriopass
Copy link
Contributor

Uriopass commented Apr 9, 2021

I'm using message-io as part of my game's networking. Using the previous API I was doing something like this (pseudo code)

let mut game_state = GameState::new();
let mut networking = Networking::new(); // connects to the server

loop {
    let new_inputs = networking.handle_packets();
    game_state.update(new_inputs);
    draw(&game_state);
}


// handle_packets being something like this
fn handle_packets(&mut self) -> Option<Inputs> {
    while let Ok(packet) = self.network.try_receive() {
         // handle packet
    }
    self.new_inputs
}

However, it looks like using the new API I cannot do this as the for_each loop completely takes control of the thread used.

My guess is that I now have to communicate between my game state and the networking code using channels.. I liked that it was abstracted away.

@lemunozm
Copy link
Owner

lemunozm commented Apr 9, 2021

Hi @Uriopass, I knew this issue would show up sooner or later 😄

I could not reach this feature (although I would have liked) because of technical restrictions with the OS Poll.

try_receive over a channel behaves differently over a supposed network try_receive(). In the channel case, you extract an event from the queue and process it, ok, it's fine. In the network case, you wake up from the OS poll with a poll event. This poll event can be traduced in zero or many NetEvents and is lost if it is not totally processed. As an example: the OS wakes up a socket because tells you that there is data to read. If you only read 5 messages from your just waked socket (5 NetEvent::Message events), and you don't read more when it has more data to process, the remaining data is potentially lost. This implies that one call to the supposed network.try_receive() could process, in the worst-case, during an undefined time, because a socket can be saturated with a huge amount of data if the sender is faster.

For that, is not recommended to mix the network thread along with your application thread. If the network saturates, it would stop all your business logic. The "network" is something outside of the "application control zone" and we can not trust it because it handles things that happen out.

A solution? Put your related logic network into the for_each callback and send to the channel (as you say) only the already processed, filtered, and validated data that will be read in the next iteration of your application. Another solution could be to write directly in a shared state with the last updated value. If you receive 3 messages between two game iteration, you will read only the last one. If you do not receive any message, you will continue reading the last one.

I hope I have helped you in some way with the problem.

@lemunozm
Copy link
Owner

lemunozm commented Apr 9, 2021

Thinking about it... Another feasible solution could be using a timed signal with a time of 1/60 to control the time loop.

@Uriopass
Copy link
Contributor Author

Uriopass commented Apr 9, 2021

It's just a bit annoying since being in sync can be really useful. For example, when the local server needs to send the world to a newly connected client, I just pass a reference to the world to the poll method, and then the world can get serialized only if a client connects.
Using channels, I have to make a channel saying "A client connected!" Then I'll asynchronously wait for the "Here's the serialized world" channel and I have to do this for every possible interaction between the game code and the networking code.

@lemunozm
Copy link
Owner

lemunozm commented Apr 9, 2021

Being "sync" in this context (understanding it as reading from the network directly in the game loop) is useful in the same way a global variable is useful and easier to write/read throughout the application. But it has the downside of losing organization and probably losing safety too.

If you need that behavior because it fits well in your current architecture. You can continue using something like the following that behaves in the same way as 0.11 version:

enum DeserializedNetEvent<T> {
    Connected(Endpoint, ResourceId),
    Message(Endpoint, T),
    Disconnected(Endpoint),
}
impl<T> From<NetEvent<'_>> for StoredNetEvent<T> { /* ... */} //Deserialize here to T
//....
listener.for_each(|net_event| event_sender.send(StoredNetEvent::from(net_event)));

loop {
    let stored_net_event = event_receiver.try_receive();
    // rest of game iteration
}

Now, if you want to reach all the potential of the new API. you probably need to use a mutex and write your world object in the mutex data. You will avoid all copies here and there is no need for channels and decouples the network part from the game logic

let world = Arc<Mutex<World>>;
let task = listener.for_each(|net_event| {
 //....
 *world.lock().unwrap() = world_from_event;
);
//...
loop { //game loop
    let world = world.lock.unwrap();
    // rest of game iteration
}
task.wait(); // Wait until node_hadler.stop() be called.

Does this last scheme fit well with your game?

@Uriopass
Copy link
Contributor Author

Uriopass commented Apr 9, 2021

I understand very clearly the advantages to be had being async, I understand the previous API was kind of a "smoke screen" as this is actually a bad practice. I just didn't mind the extra copies, and I didn't want to bother with thread communication stuff, even if Rust makes it safe.
I will probably rewrite the way networking is handled probably via channels or mutex another day, as it will be a lot of work and redesign and bug fixing. 0.11 works for me even though it's kind of "broken" since as you said, it might just get stuck in the networking loop.
Thank you for taking the time to answer and try to solve the design issues I'm facing. I understand your choices and I have more context now.

@Uriopass Uriopass closed this as completed Apr 9, 2021
@lemunozm
Copy link
Owner

Hi @Uriopass,

I added a new method for NodeListener in #80 that probably is useful for your target. It allows managing the library as version 0.11 (if the user wants to use that synchronized mode) but without the performance increment that offers the 0.12 usage. Probably you are interested.

@Uriopass
Copy link
Contributor Author

Nice! I'll take a look when I have time. Thank you for adding this. Having the choice between flexibility and performance is always welcome. :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants