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

Support for WebSockets #16

Closed
jwric opened this issue Jul 8, 2024 · 11 comments
Closed

Support for WebSockets #16

jwric opened this issue Jul 8, 2024 · 11 comments

Comments

@jwric
Copy link

jwric commented Jul 8, 2024

Hello @UkoeHB , I'm trying to use renet2 as part of a Godot 3.5 project with gdnative bindings. I would like to compile for the web, and as far as I've seen, Godot 3.5 only supports WebSockets and WebRTC for html5 exports. A WebSocket implementation for renet2 seems relatively simple as most of the logic for an http based protocol is already implemented in the current WebTransport one.

I think it should work if I can get a custom renet2 transport client that works using gdnative's WebSocketClient bindings.

I suggest to add at least a server implementation of WebSockets to renet2. As for a client implementation to add to renet2, maybe there could be one for native builds that uses tungstenite, and another for wasm builds that uses wasm-bindgen bindings similar to the WebTransport client impl.

@mirsella
Copy link

another reason to support websocket, is that webtransport is not supported for now on safari. Which mean no support for default mac and ios AFAIK

@UkoeHB
Copy link
Owner

UkoeHB commented Dec 20, 2024

@mirsella It's a valid use-case, but is it actually possible to reliably detect if a browser won't work with WebTransport? Other than trying to ping a webtransport server. It seems that due to user privacy concerns, browser name and version are not available.

@mirsella
Copy link

mirsella commented Dec 20, 2024

only use websocket, or at minimum detect the OS (ios/macos, or other) is still possible ?
its possible to have a js function that check if webtransport is supported directly no ? like check if the class exist or something like that ?

this would not be something part of this crate, but user code anyway

@UkoeHB
Copy link
Owner

UkoeHB commented Dec 20, 2024

only use websocket, or at minimum detect the OS (ios/macos, or other) is still possible ?

I'd rather only use websocket as a last resort, because it is not very compatible with the netcode protocol.

its possible to have a js function that check if webtransport is supported directly no ? like check if the class exist or something like that ?

I'd also want to check if ServerCertHash is supported, since for my project I want to use that.

this would not be something part of this crate, but user code anyway

Agreed, and the lack of solution isn't a reason to block a websocket impl. I might add it this weekend. But I was wondering if you had a nice solution :)

@mirsella
Copy link

mirsella commented Dec 20, 2024

yeah websocket would just be a fallback is webtransport is not supported on some devices.

to check if its supported, i think literally just if ('WebTransport' in window) should work ? i didn't test it as i dont have ios devices, but logically it should work, and thats what i seems to find on internet.
imo it's a clean solution

edit: there's even probably a way to check for we transport support using web-sys or js-sys crate

@UkoeHB
Copy link
Owner

UkoeHB commented Dec 21, 2024

Ok I did some research, and looks like the web-sys WebTransport constructor will fail if webtransport is not supported. On top of that, if serverCertificateHashes are not supported, then the constructor will also error.

So it should be sufficient to attempt to construct a fake WebTransport (either with or without cert hashes) and if that fails fall back to websockets. I will add some helpers for testing this without extra effort.

@UkoeHB
Copy link
Owner

UkoeHB commented Dec 21, 2024

Added detection functions in 3e7125d. Unfortunately it won't detect browser bugs like BiagioFesta/wtransport#241.

@mirsella
Copy link

looking at the issue you linked, from what I understood it's not a bug but the latest version of browser are blocking self signed certificate something like that I think for "security"

@UkoeHB
Copy link
Owner

UkoeHB commented Dec 21, 2024

looking at the issue you linked, from what I understood it's not a bug but the latest version of browser are blocking self signed certificate something like that I think for "security"

I don't think so. If they didn't want to support self-signed certs then they would not support the serverCertificateHashes feature at all, but they do claim to support it.

@mirsella
Copy link

well it's still possible to use self signed certificate by enabling a feature flag in the settings, so they're not removing it, just blocking it by default. removing serverCertificatesHashes would break a lot of things imo.

anyway, I don't think this is important for this issue.
for example, the aeronet crate examples all use encryption, without even setting up self signed certificate

@UkoeHB
Copy link
Owner

UkoeHB commented Dec 22, 2024

Implemented in 4acb9bc

@UkoeHB UkoeHB closed this as completed Dec 22, 2024
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

3 participants