Skip to content

Conversation

@nixxquality
Copy link
Contributor

@nixxquality nixxquality commented Jan 29, 2026

Summary

This PR modifies the Create Game modal to only show the Max players slider if it's meaningful.

Motivation & Context

One of my games, Quality Riichi Mahjong has a minimum players of 4 and a maximum players of 4.
When you get the Create Game popup for this game, you get a Max players slider where both the min and max is 4, and so it does not move.
I feel it's appropriate that if you've set up your project settings so that min and max players are the same, then you should always use that number as the player count.

Screenshots / Videos (if applicable)

Before:

s.box.-.2026-01-29.4-01-29.PM.mp4

After:
2026-01-29_16-01-48

Other games remain unaffected:
2026-01-29_16-02-04

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 🙂

Copilot AI review requested due to automatic review settings January 29, 2026 15:03
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.

Pull request overview

This PR updates the Create Game modal so that the “Max Players” slider is only shown when the configured minimum and maximum player counts differ.

Changes:

  • Wraps the “Max Players” slider in a conditional so it is only rendered when MinPlayers != MaxPlayers.
  • Leaves all backing logic (Players, MaxPlayers, MinPlayers, cookie handling, and OnSave) unchanged.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +56 to +61
@if ( MinPlayers != MaxPlayers )
{
<div class="entry">
<div class="is-label">Max Players</div>
<div class="control">
<SliderControl class="glass with-grow" Value:bind=@Players Min=@MinPlayers Max=@MaxPlayers Step=@(1) ShowTextEntry=@true />
Copy link

Copilot AI Jan 29, 2026

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This would be a separate change, since this can happen with regular use as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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