Skip to content

Verify CreateGameResults cookie before use#9930

Open
nixxquality wants to merge 2 commits intoFacepunch:masterfrom
nixxquality:creategamemodalcookie
Open

Verify CreateGameResults cookie before use#9930
nixxquality wants to merge 2 commits intoFacepunch:masterfrom
nixxquality:creategamemodalcookie

Conversation

@nixxquality
Copy link
Contributor

Summary

This PR verifies the data stored in the CreateGameResults cookie before use.

Motivation & Context

Copilot made a comment on my other PR #9929. It follows:

Conditionally hiding the Max Players slider when MinPlayers == MaxPlayers solves the UI issue, but it doesn’t actually enforce that fixed player count in the data that gets saved and passed to CreateGameResults. If a user has an existing cookie from when MinPlayers and MaxPlayers differed, Output.MaxPlayers (and thus Players) can still hold an out-of-range value even though the slider is now hidden, which contradicts the intent of always using the fixed player count for these games. Consider explicitly forcing Players to MinPlayers when MinPlayers == MaxPlayers (e.g. on initialization and/or in OnSave) so that stale cookie values cannot override the package’s fixed player count.

I figured that would be a separate change, so I made a separate PR.

Implementation Details

If either value in the cookie has a now-invalid value, they are replaced with the default value.

Checklist

  • Code follows existing style and conventions
  • No unnecessary formatting or unrelated changes
  • Public APIs are documented (if applicable)
  • Unit tests added where applicable and all passing
  • I’m okay with this PR being rejected or requested to change 🙂

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

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.

1 participant