feat!: Matchmaking rewrite

[DaniElectra]

Resolves #33
Depends on PretendoNetwork/nex-protocols-go#78
May be affected by PretendoNetwork/nex-go#56

Changes:

This is a rewrite of the matchmaking system to use Postgres, for the reasons explained in the related issue. This only implements the same methods as the previous system and adds more features from those methods, but no new methods have been added on this PR. Some fixes have also been adapted from #28

Notes for server implementers

The public facing API remains mostly the same compared to the previous implementation. The public facing API remains mostly the same compared to the previous implementation. However, now a database manager has to be created with globals.NewMatchmakingManager and assigned to each protocol through the SetManager function inside nex/register_common_secure_server_protocols.go, which will then initialize the database. The initialization order MUST be the following:

• match-making
• match-making-ext
• matchmake-extension

Due to the search of gatherings being now done by Postgres and not within Go land, the GameSpecificMatchmakeSessionSearchCriteriaChecks function has been removed in favor of a different function CleanupMatchmakeSessionSearchCriterias, which works in a similar way to CleanupSearchMatchmakeSession but instead allows the MatchmakeSessionSearchCriterias to be modified.

Usage of this functionality is generally discouraged except for cases of region locking, ranked matchmaking or similar. These cases should be documented, and not be used by default

Technical details

The new matchmaking system has been designed in a way to support different types of Gathering, even types outside of NEX. The table fields take into account the theoretical max size of each field, even the uint64 (which is stored as a numeric(10))

All tables are stored inside the matchmaking schema. The tables are the following:

• gatherings: Stores information about a Gathering. Uses a type column to distinguish between gathering types (MatchmakeSession, Gathering...). This is done to allow interoperability with other Quazal games. The started_time is also stored here for tracking.
• matchmake_sessions: Stores information about a MatchmakeSession.
• persistent_gatherings: Currently unused. Intended to store information about a PersistentGathering.

A tracking schema is also used for logging user actions. At the moment the only user of this schema is the match-making protocol, but other protocols can use this schema too. What is currently being logged is:

• Gathering (un)registrations
• Player joins
• Gathering leaves (graceful)
• Gathering disconnects (non-graceful)
• Host changes
• Ownership changes

All relevant database functions are present on the database directories of each protocol, except for the tracking ones which are instead stored inside the tracking directories. The database functions are exported in order to easily allow server implementers to override matchmaking methods (such as the case of Puyo Puyo which calls CloseParticipation regardless of permissions). Note that some match-making methods import from matchmake-extension/database, so while this wouldn't work out of the box for Quazal games, it would ease the process by simply overriding those methods.

The previous implementation was very prone to race conditions. That has been solved by adding a MatchmakingMutex on the common globals, which is called by the methods. The database functions don't lock by themselves, in order to perform multiple database operations in one method. There may be ways to optimize these queries, so suggestions are open.

The MatchmakeParam params of MatchmakeSession are stored as a map, which isn't easily storable on SQL. The parameter values also aren't easily storable by themselves (they are a Variant), so instead we store the raw bytes from a NEX byte stream as a workaround. This isn't a nice solution and comparison of those params when finding sessions isn't doable, so that has been discarded. Suggestions for better alternatives that allows easy comparison are welcome.

autoMatchmakePostpone previously compared every field, even those that would be considered useless when searching sessions. This has been solved by performing the same checks as a SearchCriteria, and also simplifies the query a bit.

Some SQL queries (specifically the ones used for finding sessions with search criterias) are being generated on the fly due to the search parameters being represented in a format which isn't easily parsable on SQL. Some sections could probably be inlined into the statement, but I haven't put much effort to do so. Most of the big queries have been tested, except for JoinGatheringWithParticipants .

Gatherings can have multiple participants per PID (such as multiple people playing on the same console). This functionality has been imported from Rambo's MK8 server, where the PID is stored as a negative value. Here however we perform a raw conversion from signed to unsigned int32 for compatibility. It is unknown how/if this would work on the Switch, since it has been observed PIDs with values higher than MaxInt64 (at least on NEX 4.4.0 on the 3DS). Here however we simply store the same positive PID as the main user multiple times inside the participants array, and then account for duplicates where necessary.

• I have read and agreed to the Code of Conduct.
• I have read and complied with the contributing guidelines.
• What I'm implementing was an approved issue.
• I have tested all of my changes.

[jonbarrow]

Some general comments before I actually dive into a review:

Due to the search of gatherings being now done by Postgres and not within Go land, the GameSpecificMatchmakeSessionSearchCriteriaChecks function has been removed in favor of a different function CleanupMatchmakeSessionSearchCriterias, which works in a similar way to CleanupSearchMatchmakeSession but instead allows the MatchmakeSessionSearchCriterias to be modified.

The naming convention of this seems a bit unintuitive in my opinion? It seems odd to use a function whose name implies it removes/resets data as a way to filter search criteria

gatherings: Stores information about a Gathering. Uses a type column to distinguish between gathering types (MatchmakeSession, Gathering...). This is done to allow interoperability with other Quazal games. The started_time is also stored here for tracking.

This is a bit flawed I think? Unless I'm misunderstanding something about this implementation

While MatchmakeSession does inherit from Gathering, a session and a gathering are 2 different concepts. I touched on this briefly in the notes I sent on Discord (which may also influence other parts of this design)

A Gathering is a group of users. However they do not necessarily need to be playing together. For instance in cases where a user can register their participation, and stay "participating" even if they are not online

A session, on the other hand, is an active p2p session between online clients. This can be seen in PersistentGathering which stays active for multiple sessions, and the number of sessions are stored in m_MatchmakeSessionCount. Other data types like SimpleCommunity also directly touch on this concept, having a single m_GatheringID to point to the gathering, but m_MatchmakeSessionCount to track the total number of sessions for that gathering

As far as I can tell this even means that a single Gathering can have multiple sessions at the same time? IIRC tournaments do this? Otherwise there would be MASSIVE wait queues in popular games like MK8/D? I may be mistaken about this though (given that Gathering only has a single host field)

matchmake_sessions: Stores information about a MatchmakeSession.
persistent_gatherings: Currently unused. Intended to store information about a PersistentGathering.

If the idea of the gatherings table is to support these, then why are these stored separately as well in other tables?

The previous implementation was very prone to race conditions. That has been solved by adding a MatchmakingMutex on the common globals, which is called by the methods. The database functions don't lock by themselves, in order to perform multiple database operations in one method. There may be ways to optimize these queries, so suggestions are open.

I'm going to ping @wolfendale on this one, since he helped catch several bad queries already

I do wonder if there's a better way for us to handle queries rather than raw-dogging SQL?

The MatchmakeParam params of MatchmakeSession are stored as a map, which isn't easily storable on SQL. The parameter values also aren't easily storable by themselves (they are a Variant), so instead we store the raw bytes from a NEX byte stream as a workaround. This isn't a nice solution and comparison of those params when finding sessions isn't doable, so that has been discarded. Suggestions for better alternatives that allows easy comparison are welcome.

I am fairly against this design, yeah. There's a few potential ways around this. Postgres has support for complex datatypes through its JSON, JSONB, and JSONPATH datatypes. These can be used to marshall Variant into JSON and use that for querying. This could be the most flexible solution. However we'd likely want to rework our types to support struct tags in order to make this marshalling more uniform. I can maybe start this in PretendoNetwork/nex-go#56 with the other changes?

• https://go.dev/ref/spec#Tag
• https://go.dev/wiki/Well-known-struct-tags

Another solution would be to create a new table called session_params or something similar, and store the following columns on it:

• gathering_id to link it back to the gathering
• name
• value_type to store the Variant.TypeID so it can be converted back to it's real type when queries
• value to store the Variant.Type. This would have to be a TEXT datatype, since that's the only type which all the known Variant types can be easily represented as

This would be the easiest solution to implement right now, but it's also limiting. We have to track everything as a solid string, which affects how we can search for them, and it only really works for simple types. Technically since Variant can hold seemingly any type (as seen in WATCH_DOGS, where it can also store a QUUID), it means if any games use a complex type here (like a struct with many fields) then this system starts to break down a bit. I am unaware of any instances of this happening in NEX, so I'm not sure if this should be a big concern to us or not, but it's something to keep in mind

Some SQL queries (specifically the ones used for finding sessions with search criterias) are being generated on the fly due to the search parameters being represented in a format which isn't easily parsable on SQL. Some sections could probably be inlined into the statement, but I haven't put much effort to do so. Most of the big queries have been tested, except for JoinGatheringWithParticipants .

This sounds like maybe we should explore some query builders/ORMs. ORMs are typically frowned upon in Go however, so maybe not that. Interestingly just yesterday someone posted in the r/golang subreddit about a topic very similar to this https://old.reddit.com/r/golang/comments/1dr8qyi/abusing_gos_template_engine_as_sql_builder_and_orm/

Gatherings can have multiple participants per PID (such as multiple people playing on the same console). This functionality has been imported from Rambo's MK8 server, where the PID is stored as a negative value. Here however we perform a raw conversion from signed to unsigned int32 for compatibility. It is unknown how/if this would work on the Switch, since it has been observed PIDs with values higher than MaxInt64 (at least on NEX 4.4.0 on the 3DS).

To be clear, this method of using negative PIDs was first implemented by @SuperMarioDaBom and was then carried over to Rambo's. We don't actually know how this works on official servers, unfortunately. I, too, am unsure how to solve this in this case

[DaniElectra]

The naming convention of this seems a bit unintuitive in my opinion? It seems odd to use a function whose name implies it removes/resets data as a way to filter search criteria

That is fair, the naming got imported from the previous implementation. Maybe something like FilterMatchmakeSessionSearchCriterias would be better.

This is a bit flawed I think? Unless I'm misunderstanding something about this implementation

While MatchmakeSession does inherit from Gathering, a session and a gathering are 2 different concepts. I touched on this briefly in the notes I sent on Discord (which may also influence other parts of this design)

A Gathering is a group of users. However they do not necessarily need to be playing together. For instance in cases where a user can register their participation, and stay "participating" even if they are not online

A session, on the other hand, is an active p2p session between online clients. This can be seen in PersistentGathering which stays active for multiple sessions, and the number of sessions are stored in m_MatchmakeSessionCount. Other data types like SimpleCommunity also directly touch on this concept, having a single m_GatheringID to point to the gathering, but m_MatchmakeSessionCount to track the total number of sessions for that gathering

As far as I can tell this even means that a single Gathering can have multiple sessions at the same time? IIRC tournaments do this? Otherwise there would be MASSIVE wait queues in popular games like MK8/D? I may be mistaken about this though (given that Gathering only has a single host field)

This differs a bit for what I have seen on MK7 communities. The communities have a gathering ID assigned as a PersistentGathering, however this gathering ID is different for each MatchmakeSession. The way that matchmake sessions are linked to the community is by the attribute[0] on the matchmake session (which also explains why there is this m_Attribute_0 field on SimplePlayingSession).

Also, MK8/D tournaments do their own thing afaik, they use a SimpleSearchObject which doesn't inherit from Gathering

If the idea of the gatherings table is to support these, then why are these stored separately as well in other tables?

The gatherings table stores the data that fits into a Gathering, then the structures that inherit from Gathering. Structures that inherit from Gathering store their Gathering data on the gatherings and then the exclusive data goes into the unique tables, including the gathering ID to link back to the Gathering data.

I am fairly against this design, yeah. There's a few potential ways around this. Postgres has support for complex datatypes through its JSON, JSONB, and JSONPATH datatypes. These can be used to marshall Variant into JSON and use that for querying. This could be the most flexible solution. However we'd likely want to rework our types to support struct tags in order to make this marshalling more uniform. I can maybe start this in PretendoNetwork/nex-go#56 with the other changes?

* [go.dev/ref/spec#Tag](https://go.dev/ref/spec#Tag)

* [go.dev/wiki/Well-known-struct-tags](https://go.dev/wiki/Well-known-struct-tags)

Another solution would be to create a new table called session_params or something similar, and store the following columns on it:

* `gathering_id` to link it back to the gathering

* `name`

* `value_type` to store the `Variant.TypeID` so it can be converted back to it's real type when queries

* `value` to store the `Variant.Type`. This would have to be a `TEXT` datatype, since that's the only type which all the known `Variant` types can be easily represented as

This would mean that we have to convert the variant to a string, which isn't directly doable, we would have to add a function that converts the value to a string and vice-versa.

This would be the easiest solution to implement right now, but it's also limiting. We have to track everything as a solid string, which affects how we can search for them, and it only really works for simple types. Technically since Variant can hold seemingly any type (as seen in WATCH_DOGS, where it can also store a QUUID), it means if any games use a complex type here (like a struct with many fields) then this system starts to break down a bit. I am unaware of any instances of this happening in NEX, so I'm not sure if this should be a big concern to us or not, but it's something to keep in mind

What if we store the value as a bytea here? It may not be the most intuitive solution but it would solve the issue, and can be used in comparisons.

To be clear, this method of using negative PIDs was first implemented by SuperMarioDaBom and was then carried over to Rambo's. We don't actually know how this works on official servers, unfortunately. I, too, am unsure how to solve this in this case

I see...

[jonbarrow]

That is fair, the naming got imported from the previous implementation. Maybe something like FilterMatchmakeSessionSearchCriterias would be better.

This sounds like you're filtering the criteria themselves rather than filtering by the criterias. But I do agree this is better

This differs a bit for what I have seen on MK7 communities. The communities have a gathering ID assigned as a PersistentGathering, however this gathering ID is different for each MatchmakeSession. The way that matchmake sessions are linked to the community is by the attribute[0] on the matchmake session (which also explains why there is this m_Attribute_0 field on SimplePlayingSession).

Also, MK8/D tournaments do their own thing afaik, they use a SimpleSearchObject which doesn't inherit from Gathering

I believe we had a miscommunication. Yes, sessions have different dedicated gathering IDs from that of the parent gathering in these cases (I did not intend to imply otherwise). I was talking purely about the semantics of the 2 (a base Gathering being purely an ephemeral collection of participants which may or may not be playing together, a PersistentGathering being purely a collection of participants which may or may not be playing together and which can persist for long periods of time (or even indefinitely), and a MatchmakeSession being an ephemeral gathering of participants which are in a direct p2p connection) and how since they're different concepts how that may affect the database design. Given how you explained the other tables though this seems like not really an issue

Also for what it's worth, the 0th attribute is not always the gathering ID in these cases. This is only the case when m_MatchmakeSystemType is 5 (which should also be in the notes)

You're right about MK8/D, I had forgotten about that

This would mean that we have to convert the variant to a string, which isn't directly doable, we would have to add a function that converts the value to a string and vice-versa.

I had listed 2 solutions in what you quoted

The first one with JSON, JSONB, and JSONPATH datatypes does not need to be converted to a string by us directly. They just need to be able to be marshalled, which most structs can be marshalled as-is they just won't have typical JSON field names (the field names will match 1:1 with the struct names, which is why I mentioned using struct tags)

The second one does require converting to a string (and is the one where I proposed the table layout), but as I said all known subtypes of Variant can do this easily. I figured a real implementation would be to use probably https://github.com/PretendoNetwork/pq-extended to handle these conversions for us. The issue is that you can't do things like numeric comparisons on strings, and this will not work basically at all for Variant instances which may have complex subtypes

What if we store the value as a bytea here? It may not be the most intuitive solution but it would solve the issue, and can be used in comparisons

This would function just the same as how you currently have it would it not? I'm not aware of being able to do direct comparisons on bytea types in Postgres, and if you can't then it would render this change useless (if you can't filter by the value, then might as well not filter at all?)

[DaniElectra]

This sounds like you're filtering the criteria themselves rather than filtering by the criterias. But I do agree this is better

It's technically filtering the criterias though, it's intended to allow the server implementation to change the search parameters on the search criteria

Also for what it's worth, the 0th attribute is not always the gathering ID in these cases. This is only the case when m_MatchmakeSystemType is 5 (which should also be in the notes)

That's good to know!

I had listed 2 solutions in what you quoted

The first one with JSON, JSONB, and JSONPATH datatypes does not need to be converted to a string by us directly. They just need to be able to be marshalled, which most structs can be marshalled as-is they just won't have typical JSON field names (the field names will match 1:1 with the struct names, which is why I mentioned using struct tags)

Ah sorry, that seems reasonable yeah.

I'm not aware of being able to do direct comparisons on bytea types in Postgres, and if you can't then it would render this change useless (if you can't filter by the value, then might as well not filter at all?)

ngl I kinda just assumed it, but if it isn't possible to compare bytea types then you can ignore that.

[ashquarky]

I've had a go at implementing this in Minecraft here: https://github.com/PretendoNetwork/minecraft-wiiu/tree/work/mm-rewrite (Depends on #41)

Feel free to inspect for code style notes. Overall it was pretty painless to do with a few extra globals for the SQL database. I liked how old gatherings have registered=false but otherwise remain in the database; that'll be useful for moderation.

Since this code actually handles PRUDP disconnections, there are a few spurious kicks coming up from the lower layers. Will investigate further.

[DaniElectra]

In sight on the good results that the matchmaking rewrite has in the servers that have it and since it now has the green light, I will go ahead and merge this