laminar

laminar copied to clipboard

This is an initial crack at #290. This ports over the implementation from backroll_transport(_udp) as the laminar::net::aio module.

• Includes the BidirectionalAsyncChannel<T>, Peer, Peers<T> from backroll_transport.
• Has a UdpManager from backroll_transport_udp that has been rewritten to have a similar interface to net::Socket
• Added an additional conversion function with_link_conditioner to Peer that converts the Peer and adds tasks that simulate packet loss and latency. The return value is still a Peer so the consumer doesn't know of the conditioner.
• Added a function to LinkConditioner to randomly generate additional latency.
• Includes a forward utility function for doing simple 1:1 channel message forwarding.
• This design completely removes the need for a FakeSocket at this point. A pair of peers can be created in-memory, and with_link_conditioner can be used on both sides to achieve the same result.

Potential changes before merging:

• The UDP implementation is included because the base synchronous Laminar uses it, but we may want to put it behind a default feature flag in the future
• The implementation is closely tied with bevy_tasks TaskPool as it's futures executor. This, ideally, should be managed by a feature flag and be runtime agnostic.
• Unit test the link conditioned peer.
• More documentation.

Current unknowns:

• Should the synchronous interface be retained? This current implementation does not break backwards compatibility.
• If the synchronous interface should be retained, should the underlying implementation be replaced with the async one?
• Should Peer or one of it's wrappers send one of the Event enums? The disconnect notification is already baked into the channel implementation, so SocketEvent::Disconnect is not going to be needed; however, connection interruption timeouts should definitely be forwarded.

• The implementation is closely tied with bevy_tasks TaskPool as it's futures executor. This, ideally, should be managed by a feature flag and be runtime agnostic.

Second that.

I am using laminar purely as a transport protocol (TCP replacement) over webrtc-unreliable . I am handling packet pooling in my own runtime. Pulling in a task runtime would just hurt my compilation time for no gain.

I am using laminar purely as a transport protocol (TCP replacement) over webrtc-unreliable . I am handling packet pooling in my own runtime. Pulling in a task runtime would just hurt my compilation time for no gain.

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. laminar-proto), and then have two separate crates that have a sync/async interface for the actual protocol itself? For now, it's likely easier to just put this all behind an optional feature flag.

Looks great! The code looks clean and great.

• Do we need futures in its whole or can we go for something like future-core?
• Why did you choose for bevy-tasks, I see it is a less-dependency library but it is quite new. Is this a stable library yet, are there better alternatives?

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.