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

feat(multiset): handle r=ping correctly #2983

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

wescopeland
Copy link
Member

@wescopeland wescopeland commented Dec 26, 2024

This PR modifies how game IDs are resolved between the patch and ping routines to ensure consistent behavior when multiset is enabled.

Key changes:

  • A new action, ResolveRootGameIdAction, has been added. This action encapsulates game ID resolution logic for both endpoints.
  • BuildClientPatchDataAction has been modified to use this action. Tests are unchanged and all remain passing.
  • In dorequest.php, ping has been modified to use this action for game ID resolution if multiset is enabled.

A reminder of how game ID resolution works:

  • For legacy clients, the loaded hash's direct game ID is used.
  • When multiset is disabled, the loaded hash's direct game ID is used.
  • When multiset is enabled:
    • For bonus set game hashes, resolve to the core/base game ID.
    • For specialty/exclusive set game hashes, maintain the subset game ID.
    • If the user has globally opted out of subsets, use the loaded hash's direct game ID.

The changes in this PR ensure that:

  • The active game ID in ping matches the root-level game ID from patch.
  • Legacy clients and users who have opted out of multiset maintain the existing behavior.

It's worth noting the changes in ping are controlled entirely by the VITE_FEATURE_MULTISET feature flag, as clients are already sending both the game ID (g) and the game hash MD5 (x).


Next steps:

  • Handle r=startsession correctly.

@wescopeland wescopeland requested a review from a team December 26, 2024 00:33
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

Successfully merging this pull request may close these issues.

2 participants