rcelyte / BeatUpRcelyte

A lightweight modded Beat Saber multiplayer server
The Unlicense
16 stars 5 forks source link

GUIDs are not generated for SessionGameId #4

Closed roydejong closed 2 years ago

roydejong commented 2 years ago

Background

When gameplay starts, normally a GUID is generated for each gameplay session.

The server usually sends this in SetGameplaySceneSyncFinishedRpc or SetPlayerDidConnectLateRpc.

Problem

BeatUpServer doesn't seem to do this, instead returning an empty GUID (00000000-0000-0000-0000-000000000000).

https://github.com/rcelyte/BeatUpRcelyte/blob/4f110ee5feae4bf4d5c1c4523df141ca66ab4844/src/instance/instance.c#L452

Unfortunately this breaks the Server Browser's ability to track individual gameplay sessions.

rcelyte commented 2 years ago

Would it be fine to just have GUIDs count up from zero, or do they need to remain unique across server restarts?

roydejong commented 2 years ago

It would be better if it was globally unique, so if there's another instance of BeatUpServer their session IDs don't overlap.

For context, each GUID is supposed to uniquely reflect a single gameplay session and their results, for example: https://bssb.app/results/9e1efb8a-1b53-4526-ab5e-8c3aa39911ed

If you're able to do the equivalent of Guid.NewGuid that would be perfect.

rcelyte commented 2 years ago

I can't really enforce globally unique IDs across different instances (including vanilla & BeatTogether). It would be much more reliable to additionally index them by master server hostname/address.

roydejong commented 2 years ago

I understand you can't 100% guarantee or enforce that, but the point of a randomly generated GUID is that the chance of overlap is effectively zero.

For context:

[..] the number of random version-4 UUIDs which need to be generated in order to have a 50% probability of at least one collision is 2.71 quintillion [..]

With an implementation that counts up, overlap is guaranteed and more likely to cause eventual conflicts even if I do index it by server address (example: if the server resets its counter).

It's a well accepted practice to treat randomly generated GUIDs as unique, and I think that's the most appropriate solution here.