Skip to content

Add extra time per player in simul #12576

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 7 commits into from
Mar 28, 2023
Merged

Add extra time per player in simul #12576

merged 7 commits into from
Mar 28, 2023

Conversation

@drybalka
Copy link
Contributor

@drybalka drybalka commented Mar 25, 2023 •
edited
Loading

Implements one of the suggestions in #6554
And probably solves #6410

This adds an option to add extra time per player on the simul screen.

@drybalka
Copy link
Contributor Author

drybalka commented Mar 26, 2023

I could probably also change SimulClock.hostExtraTime into LimitMinutes, but I did not want to touch more code than necessary in my first issue =)

@ornicar
Copy link
Collaborator

ornicar commented Mar 27, 2023

Thanks! This looks pretty good. But could you please change it to seconds instead of minutes? I feel like we're going to need that soon, so better get the DB format right from the get go.

@drybalka
Copy link
Contributor Author

drybalka commented Mar 27, 2023

I don't feel there's a sensible difference between LimitSeconds and IncrementSeconds (or any other seconds for that matter). Imho it would have been better to have simply Clock.Seconds and Clock.Minutes types with conversions between them. Or even better some kind of universal Time class with to/from methods to both Seconds and Minutes.

@ornicar
Copy link
Collaborator

ornicar commented Mar 28, 2023

I don't feel there's a sensible difference between LimitSeconds and IncrementSeconds (or any other seconds for that matter).

But there is. One should only be used for initial time, and the other only for increment. Having the distinction increases type safety and prevents bugs, now and after future refactorings.

ornicar added 4 commits March 28, 2023 17:15
@ornicar
Merge branch 'master' into simul-extra-time-per-player
05ed804 
* master:
  Update TR Dictionary
  /api/player/top/1/standard for mobile app
  add /api/player/top/1/all for mobile app
  tweak leaderboard list API code
  fix storm result posting
  POST /storm with oauth
  GET /api/storm anon or oauth
  Update ru Dictionary
  refer to SECURITY.md on github (lichess-org#12581)
  New Crowdin updates (lichess-org#12567)
  fix path to font file on oops page
  make grid areas more readable
  remove scalatags.me
  Verify website on Mastodon
  Update joda-time to 2.12.4
  add POST /api/streak/:score API endpoint
  add GET /api/streak endpoint
@ornicar
typing tweaks
e49a694
@ornicar
tweak simul default wording
999bb14
@ornicar
set and display simul extra time per player in seconds
276f138
@ornicar ornicar merged commit 679f7bb into lichess-org:master Mar 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@DanielFrantes DanielFrantes DanielFrantes approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@drybalka @ornicar @DanielFrantes