Skip to content

show correct ratings in simul #13909

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

Merged
merged 6 commits into from Nov 6, 2023
Merged

show correct ratings in simul #13909

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
89c648c
show rating of simul clock config
Nov 2, 2023
8a7ac01
show host proper rating with provisional
Nov 6, 2023
1b7fd80
refactor
Nov 6, 2023
3c40e60
scalafmtAll
Nov 6, 2023
a76330a
Merge branch 'master' into simul-ratings
ornicar Nov 6, 2023
241a9fc
simul host.provisional is optional
ornicar Nov 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions modules/simul/src/main/JsonView.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ final class JsonView(
"name" -> host.name,
"rating" -> simul.hostRating
)
.add("provisional" -> simul.hostProvisional)
.add("gameId" -> simul.hostGameId.ifTrue(simul.isRunning))
.add("title" -> host.title)
.add("patron" -> host.isPatron)
Expand Down
52 changes: 26 additions & 26 deletions modules/simul/src/main/Simul.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ case class Simul(
estimatedStartAt: Option[Instant] = None,
hostId: UserId,
hostRating: IntRating,
hostProvisional: Option[RatingProvisional],
hostGameId: Option[String], // game the host is focusing on
startedAt: Option[Instant],
finishedAt: Option[Instant],
Expand Down Expand Up @@ -152,29 +153,28 @@ object Simul:
estimatedStartAt: Option[Instant],
featurable: Option[Boolean],
conditions: SimulCondition.All
): Simul = Simul(
_id = SimulId(ThreadLocalRandom nextString 8),
name = name,
status = SimulStatus.Created,
clock = clock,
hostId = host.id,
hostRating = host.perfs.bestRatingIn:
variants.map {
PerfType(_, Speed(clock.config.some))
} ::: List(PerfType.Blitz, PerfType.Rapid, PerfType.Classical)
,
hostGameId = none,
createdAt = nowInstant,
estimatedStartAt = estimatedStartAt,
variants = if position.isDefined then List(chess.variant.Standard) else variants,
position = position,
applicants = Nil,
pairings = Nil,
startedAt = none,
finishedAt = none,
hostSeenAt = nowInstant.some,
color = color.some,
text = text,
featurable = featurable,
conditions = conditions
)
): Simul =
val hostPerf = host.perfs.bestPerf(variants.map { PerfType(_, Speed(clock.config.some)) })
Simul(
_id = SimulId(ThreadLocalRandom nextString 8),
name = name,
status = SimulStatus.Created,
clock = clock,
hostId = host.id,
hostRating = hostPerf.intRating,
hostProvisional = hostPerf.provisional.some,
hostGameId = none,
createdAt = nowInstant,
estimatedStartAt = estimatedStartAt,
variants = if position.isDefined then List(chess.variant.Standard) else variants,
position = position,
applicants = Nil,
pairings = Nil,
startedAt = none,
finishedAt = none,
hostSeenAt = nowInstant.some,
color = color.some,
text = text,
featurable = featurable,
conditions = conditions
)
2 changes: 1 addition & 1 deletion modules/simul/src/main/SimulApi.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ final class SimulApi(
.filter(simul.variants.contains)
.ifTrue(simul.nbAccepted < Game.maxPlayingRealtime)
.so: variant =>
val perfType = PerfType(variant, chess.Speed.Rapid)
val perfType = PerfType(variant, chess.Speed(simul.clock.config.some))
Copy link
Collaborator

@ornicar ornicar Nov 3, 2023

Choose a reason for hiding this comment

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

2 issues with this.

  • it doesn't take into account the simul variant (neither does the current code, so the problem is already here)
  • most simuls use classical clock, and most players don't have a classical rating. I reckon we should try using the simul speed, and if the user has no such rating, fallback to the next faster speed (so classical->rapid)

Copy link
Author

@ghost ghost Nov 3, 2023 •
edited by ghost
Loading

Choose a reason for hiding this comment

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

I did notice the first issue when looking at this and had a short discussion about it in the Discord.

I'll make more changes after tomorrow trying to fix these issues

Copy link
Author

@ghost ghost Nov 6, 2023

Choose a reason for hiding this comment

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

Thinking more about this now. It actually does take into account the simul variant! So no problems there.

Regarding second issue, I'm not sure your suggested behaviour would be best.

  1. I think it would be strange to have some players showing their classical rating while some showing their blitz rating. If a player has a provisional classical rating, then this provisional rating should be shown. Blitz ratings and classical ratings are usually very different, so I do not think they are interchangeable.

  2. If a simul host wants to ensure all players do have a classical rating, they can already do this. There is an entry condition to specify the minimum rating allowed. Perhaps another entry condition option to ensure the player's rating is not provisional would be good though. image

  3. There would be a mismatch between the rating showed on the simul page and the rating shown when you click into each game. I think this would create more confusion than it saves by just showing that the player only has a provisional classical rating.

  4. If a player has no classical rating and you want to see what rating they are in rapid or blitz, you can already hover over their name to see this. For the above reasons, I do not think their rapid rating should be pretended to be their classical rating.

perfsRepo
.withPerf(me.value, perfType)
.flatMap: user =>
Expand Down
8 changes: 8 additions & 0 deletions modules/user/src/main/UserPerfs.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,14 @@ case class UserPerfs(
case (ro, _) => ro
.getOrElse(Perf.default.intRating)

def bestPerf(types: List[PerfType]): Perf =
types
.map(apply)
.foldLeft(none[Perf]):
case (ro, p) if ro.forall(_.intRating < p.intRating) => p.some
case (ro, _) => ro
.getOrElse(Perf.default)

def bestRatingInWithMinGames(types: List[PerfType], nbGames: Int): Option[IntRating] =
types
.map(apply)
Expand Down
1 change: 1 addition & 0 deletions ui/simul/src/interfaces.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ export interface Player extends LightUserOnline {

export interface Host extends LightUserOnline {
rating: number;
provisional?: boolean;
gameId?: string;
}

Expand Down