-
Notifications
You must be signed in to change notification settings - Fork 7
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
feat: Initial support for persistent gatherings/communities #45
base: main
Are you sure you want to change the base?
Conversation
ce1a929
to
5f36476
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks pretty good! Just a couple notes
// TODO - Is this right? Mario Kart 7 sets 0 max participants | ||
if community.MaximumParticipants > 0 { | ||
_, nexError = match_making_database.JoinGathering(commonProtocol.manager, uint32(community.ID), connection, 1, string(strMessage)) | ||
if nexError != nil { | ||
commonProtocol.manager.Mutex.Unlock() | ||
return nil, nexError | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Iirc "0" would mean "no limit". Typically in NEX whenever "0" is set it means disabled/invalid/unset, so in this case I would interpret it as there being no max amount of participants (which makes sense, seeing as persistent gatherings like these could have any number of players)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That could be the case, but it would mean that all MK7 communities would have one "participant" under the current code, since you don't actually join communities as explained in the PR description, you only create matchmake sessions attached to a community
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Smash 4 seems to set a MaximumParticipants count over 0, so I agree with Jon
persistentGathering, nexError := GetPersistentGatheringByGathering(manager, gathering, sourcePID) | ||
if nexError != nil { | ||
return nil, "", nexError | ||
} | ||
|
||
matchmakeSessionCount, nexError := GetPersistentGatheringSessionCount(manager, gatheringID) | ||
if nexError != nil { | ||
return nil, "", nexError | ||
} | ||
|
||
participationCount, nexError := GetPersistentGatheringParticipationCount(manager, gatheringID, sourcePID) | ||
if nexError != nil { | ||
return nil, "", nexError | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doing this in 3 separate calls could lead to race conditions, and it hits the DB with several calls. I get the feeling this can be reduced down to a single call, since all 3 of these are just getting different stats for the same gathering?
I know some of these functions are reused elsewhere, but it may be worth just duplicating the code a few times for specific use cases to get optimal database performance and remove race conditions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are no race conditions since all matchmaking functions are executed within a mutex. Though it makes sense to optimize it, but I would need some help to craft the inlined queries
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might have to call in @binaryoverload for some help too, as I'm by no means a postgres expert
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SELECT community_type,
password,
attribs,
application_buffer,
participation_start_date,
participation_end_date,
(SELECT COUNT(ms.id)
FROM matchmaking.matchmake_sessions AS ms
INNER JOIN matchmaking.gatherings AS g ON ms.id = g.id
WHERE g.registered = true
AND ms.matchmake_system_type = 5
AND ms.attribs[1] = $1) AS persistent_gathering_session_count
(SELECT cp.participation_count
FROM matchmaking.community_participations AS cp
WHERE cp.user_pid = $1
AND cp.gathering_id = $2) as persistent_gathering_participation_count
FROM mh4u.matchmaking.persistent_gatherings
WHERE id = $1;
We can use inline queries like this - allowing the two counts to be set in the same query
This could be all put into the GetPersistentGatheringByGathering
function
I don't have a way to test this query, as the running servers don't have the community_participations table
resultMatchmakeSessionCount, nexError := GetPersistentGatheringSessionCount(manager, uint32(resultPersistentGathering.ID)) | ||
if nexError != nil { | ||
common_globals.Logger.Critical(nexError.Error()) | ||
continue | ||
} | ||
|
||
resultParticipationCount, nexError := GetPersistentGatheringParticipationCount(manager, uint32(resultPersistentGathering.ID), uint64(sourcePID)) | ||
if nexError != nil { | ||
common_globals.Logger.Critical(nexError.Error()) | ||
continue | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment as before about reducing the DB calls (our DB gets pretty hammered as is). I won't leave a new comment for each instance, only doing the 2
Implement methods that are needed for Mario Kart 7 communities to work. Note that support for communities on MK7 is partial since the community statistics don't load because legacy Ranking isn't implemented. Aside from that, players can create communities and join others without issues. Other games which use persistent gatherings may or may not work. In order to support the `ParticipationCount`, we replace matchmake session joins with a wrapper which checks if the session is attached to a community, and if it is, it will increment the participation count of the player in a new table named `community_participations`. The `MatchmakeSessionCount` is handled more easily by checking the sessions that belong to the corresponding community. A new parameter is also added named `PersistentGatheringCreationMax` with a default value of 4, as reported and tested on various games. This allows game servers to change the maximum number of active persistent gatherings that a player can create. For example, Mario Kart 7 supports up to 8 persistent gatherings instead of the default of 4. In Mario Kart 7 there is no limitation on the number of players that can "join" to a community. That is because they don't really join to it but they create matchmake sessions linked to the persistent gathering (in fact, the `MaximumParticipants` parameter on persistent gatherings is set to 0). Thus, the `participants` parameter is unused in communities (at least on MK7) and we instead log community participations with a new tracking table `tracking.participate_community`. Some changes also had to be done in other places like participant disconnection handling or gathering registrations in order to implement persistent gatherings accurately.
5f36476
to
25c233f
Compare
Also reduce the PersistentGathering.Password size to 32 extrapolatinf from the UserPassword on MatchmakeSession
Resolves #XXX
Changes:
Implement methods that are needed for Mario Kart 7 communities to work. Note that support for communities on MK7 is partial since the community statistics don't load because legacy Ranking isn't implemented. Aside from that, players can create communities and join others without issues. Other games which use persistent gatherings may or may not work.
In order to support the
ParticipationCount
, we replace matchmake session joins with a wrapper which checks if the session is attached to a community, and if it is, it will increment the participation count of the player in a new table namedcommunity_participations
. TheMatchmakeSessionCount
is handled more easily by checking the sessions that belong to the corresponding community.A new parameter is also added named
PersistentGatheringCreationMax
with a default value of 4, as reported and tested on various games. This allows game servers to change the maximum number of active persistent gatherings that a player can create. For example, Mario Kart 7 supports up to 8 persistent gatherings instead of the default of 4.In Mario Kart 7 there is no limitation on the number of players that can "join" to a community. That is because they don't really join to it but they create matchmake sessions linked to the persistent gathering (in fact, the
MaximumParticipants
parameter on persistent gatherings is set to 0). Thus, theparticipants
parameter is unused in communities (at least on MK7) and we instead log community participations with a new tracking tabletracking.participate_community
.Some changes also had to be done in other places like participant disconnection handling or gathering registrations in order to implement persistent gatherings accurately.