Refactor: Drop typestate pattern for building sockets

[caspark]

The typestate pattern caused a lot of boilerplate w.r.t. both the implementation and the usage of matchbox socket.

There appeared to be only three actual uses of the typestate pattern:

• Preventing client code from calling build() without adding a channel.
• Some convenience methods to read/write from the first channel in the case where where was only one channel.
• Making sure that ggrs::NonBlockingSocket is only implemented for
  WebRtcSocket in the case where it is a single channel socket, to avoid the case where a reliable socket is accidentally used as a GGRS socket (or a unreliable socket is used for GGRS + some other data).

The first and (mostly) the last points can be accomplished at runtime via asserts, and owing to the "set it up and use it from one place" nature of this library I think it's highly unlikely that having a runtime assertion is problematic.

This is a breaking change owing to the removal of the SingleChannel-variant convenience methods, but it should be trivially easy to migrate by looking at modified example code.

---

This is definitely a "change based on personal taste", so I understand if your personal preferences do not align and you do not want to go down this route. But since I've gone to the trouble of writing this PR/micro-essay and you're reading it, well, you may as well keep reading, right? :)

TLDR: In my opinion, the benefits of the typestate pattern here are not justifying its cost.

As a new user approaching this library, one of my stumbling blocks was "why is there this ChannelPlurality generic that's front and center?". Not "what is it" - I understand the idea of making bad states unrepresentable, I'm already familiar with the typestate pattern in particular, I've used Rust plenty and I've shipped prod code with type wizardry in Scala, Typescript, etc - but rather the why, as in what motivates it.

So, I spent a while diving through the codebase to figure out what makes a single channel webrtc socket inherently different to a many channel socket in some way that makes it worth modeling it at a type system level - and after going through the exercise of removing the typestate pattern (I find editing code & following the compile errors a good way to understand it), all I really found was the convenience methods for a single channel WebRtcSocket - and (IMO) these more obscure how the library is structured than aid in understanding it.

So I figured I'd raise my changes as a PR to remove the typestate pattern and see what others thought - the result is at least less boilerplate-y.

I did read #160 to understand the thinking behind the initial introduction of the typestate pattern, and it seems to boil down to "let's make it easier for new users". I think that's a great goal, but I would argue that a somewhat-vaguely-named (and possibly misleading, at least for me) generic parameter is less easy (or at least, less simple) than embracing the concept of channels; the updated example code in this PR shows that dealing with a single channel is straightforward enough.

(I figure backwards compatibility may also have been a concern that motivated the introduction of the convenience methods I mentioned in my second bullet above? But I think the changes from dealing with channels are easy enough to adjust to - a single .channel_mut(0) in the (probably) 2 places where a single-channel user would be reading/writing from/to the socket would be the sum total of it.)

Closes: #332

[johanhelsing]

Thanks for the great write-up. I think I agree, and I also think I was the one arguing hardest for not requiring new users to think about channels, but like you say I guess they still do, due to the generic parameter.

Also, the situation has changed a bit, and I think a big win for this is also that it's easier to maintain and understand for new contributors coming to the library itself.

@garryod what do you think?

[caspark]

Regarding "WebsocketBuilder#add_ggrs_channel() enforces that the ggrs channel is the first channel", another approach is to remove that opinion from matchbox_socket's ggrs feature entirely:

• remove ChannelConfig#ggrs() and WebsocketBuilder#add_ggrs_channel(), and probably MatchboxSocket#new_ggrs too
• update examples to show that ggrs should use a manually created unreliable channel
• remove impl ggrs::NonBlockingSocket<PeerId> for WebRtcSocket in favor of only keeping impl ggrs::NonBlockingSocket<PeerId> for WebRtcChannel

I would go with that approach myself if I were designing this library, mainly because ggrs is very clear that it expects unreliable transport and the various "make a socket/channel for ggrs" helper functions are a) trivial and b) initially made me think that there might be something non-trivial involved in making a ggrs socket/channel. But it is a larger and more opinionated change so I held off doing it for this PR's first cut.

We could also go further and have WebRtcChannel's type encode whether it's a reliable channel or an unreliable channel, and only implement ggrs::NonBlockingSocket for the unreliable channel. I am not sure the juice is worth the squeeze for that though, especially for any non-ggrs users (if they exist). Having WebRtcChannel channel store a unreliable: bool and logging a warning in the ggrs::NonBlockingSocket's impl for reliable channels might be better.

[johanhelsing]

Having thought about it, I agree on all points. Let's drop the implementation on the socket and just have a runtime warning if the channel is not configured as unreliable

[johanhelsing]

Many of the ci failures are unrelated, but fixed now. Feel free to either rebase or merge main into this branch.

[johanhelsing]

I think the remaining ci failures now are actual relevant failures. The bevy signalling server examples need to be updated.

cargo test --features signaling --all-targets

Otherwise, this looks good to merge :)