1.31 Support - Githubissues

Goobwabber / MultiplayerCore

A Beat Saber mod that implements core custom multiplayer functionality.

MIT License

66 stars 36 forks source link

1.31 Support #45

Closed roydejong closed 8 months ago

roydejong commented 1 year ago

This PR adds support for Beat Saber 1.31:

adds DisableSsl patch to bypass SSL / encryption / certificate checks on Ignorance/ENet connections
minor code change for 1.31 compatibility (environmentInfos)
support use_ssl/useSsl in multiplayer status data, add MpStatusRepository to allow BeatTogether to access this
bumps MpCore version to 1.5

roydejong commented 1 year ago

I've made some changes to this and the BeatTogether client PR to address feedback re: encryption and settings.

Regarding the SSL setting: MultiplayerCore / BeatTogether:

You can now pass a use_ssl bool in multiplayer status data (default: false). MultiplayerCore stores this but cannot enforce it, that is currently the responsibility of mods that select/override servers like BeatTogether.
MultiplayerCore now has a MpStatusRepository utility that allows other mods like BeatTogether to practically access this information and receive update events.
BeatTogether will now automatically update its configuration based on the multiplayer status data. If your master server reports "useSsl":true then BeatTogether will remember and apply that setting.

ServerBrowser, unrelated to these changes:

Will always detect & report encryption mode per lobby, and apply it when connecting.

rcelyte commented 1 year ago

(default: false)

I take issue with having defaults incompatible with Official servers. Protocol extensions like this should build on the source seamlessly, not requiring hard-coded edge cases like if(isOfficial) where things defer.

roydejong commented 1 year ago

I take issue with having defaults incompatible with Official servers. Protocol extensions like this should build on the source seamlessly, not requiring hard-coded edge cases like if(isOfficial) where things defer.

I don't disagree from a purist perspective. But pragmatically speaking, this default makes the most sense to me for third-party servers as I expect they will generally not do DTLS.

Extended server status data is not used for official servers at all, because all mods do indeed have special handling for official. I think "if official, disable modded extensions" type logic will always exist and is not unreasonable.

Flipping the default would have no effect except that it might be "more correct" and would require explicit opt-out from modded servers.

rcelyte commented 1 year ago

Modded servers already have an explicit opt-out. The only status responses not containing a use_ssl field would be servers that either expect vanilla behaviour or never supported ENet in the first place (i.e. outdated instances).

aaronjamt commented 8 months ago

I've further updated this PR to 1.34, when will this be merged? I don't want to submit my own PR and take @roydejong's credit for their code until this PR has been merged.

michael-r-elp commented 8 months ago

@roydejong would it be fine if we merge this into our dev branch instead of main, then we can open a new PR for 1.34.0

roydejong commented 8 months ago

Sounds good, merging into dev