-
Notifications
You must be signed in to change notification settings - Fork 67
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
Async I/O based Laminar implementation: Initial Pass. #291
base: master
Are you sure you want to change the base?
Conversation
Second that. I am using |
One potential way to do this is to split out the currently private components (i.e. the packet header parsers) for implementing packet handling into a separate crate (i.e. |
Looks great! The code looks clean and great.
For the async vs sync question. I think we need to find pros' vs cons' when it comes to maintaining two API's. I can imagine how maintaining two APIs will be more work to maintain, more code = possible more errors, forwarding boiler code. At the same time, with a bit of forwarding code, we can make it users easier to work with laminar since they dont have to deal with async if they don't want to. What do you think? I found some crate: https://docs.rs/maybe-async/0.2.6/maybe_async/, never tried it, but seems like an interesting possibility maybe? I know that QUINN, a google quic implementation, goes for something similar as you proposed @james7132. They spit up the protocol from the IO and async layers. Although this introduces some boilerplate code since you need to forward some calls from the IO layer to the protocol layer, it does give us a more flexible design. |
Futures is only needed for testing. Only using it for futures::block_on right now, so I think the dependency surface area could be reduced. bevy-tasks was primarily chosen because it supports a multitude of game platforms, including wasm out of the box. However, the only thing needed is a way to launch futures, and it may be useful to use async-executors to abstract away the execution runtime with feature flags in the future. |
This is an initial crack at #290. This ports over the implementation from
backroll_transport(_udp)
as thelaminar::net::aio
module.BidirectionalAsyncChannel<T>
,Peer
,Peers<T>
from backroll_transport.UdpManager
frombackroll_transport_udp
that has been rewritten to have a similar interface tonet::Socket
with_link_conditioner
toPeer
that converts thePeer
and adds tasks that simulate packet loss and latency. The return value is still aPeer
so the consumer doesn't know of the conditioner.LinkConditioner
to randomly generate additional latency.forward
utility function for doing simple 1:1 channel message forwarding.FakeSocket
at this point. A pair of peers can be created in-memory, andwith_link_conditioner
can be used on both sides to achieve the same result.Potential changes before merging:
bevy_tasks
TaskPool as it's futures executor. This, ideally, should be managed by a feature flag and be runtime agnostic.Current unknowns:
Peer
or one of it's wrappers send one of theEvent
enums? The disconnect notification is already baked into the channel implementation, soSocketEvent::Disconnect
is not going to be needed; however, connection interruption timeouts should definitely be forwarded.