Broadcast preview: add live clocks #12802
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
easier to see what's going on
Preliminary refactor to fetching clock stake. Extremely precautious here due to lack of type checking so each step is tested then commited
Probably more helpers to generate mini-game can go there
It's slightly inaccurate since not only used for mini-game but best effort
Keep the clock state as a sliding window of the last two nodes. since `Object.keys(root)` does not list moves chronologically, we need to store the two paths. `clockTicking` -> the ticking clock, belonging to player whose turn it is to move `node` -> the node of the last move. Contain the clock of the player that just played We cannot use `node.ply` to know the side to move, because when starting from position black can start first
made from
`resultStr: Option[String] = outcome.map(o => Outcome.showResult(o).replace("1/2", "½"))`
with
```
case class Outcome(winner: Option[Color]):
override def toString = winner match
case Some(White) => "1-0"
case Some(Black) => "0-1"
case None => "1/2-1/2"
object Outcome:
def showResult(outcome: Option[Outcome]): String =
outcome.fold("*")(_.toString)
```
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
close #12106
The change to the aggregation query has been isolated to this commit: 24936cd. The change in size is mostly due to reformatting.
Globally I think the integration is quite clean and doesn't add that much complexity, beside the query being slightly more expensive.
Testing on the other end is hard because of complete lack of type checking and errors only happening at runtime. I've tested to the best of my abilities:
Worst case scenario the query fail and the overview cannot be shown, but I don't know if it'd harm the servers (since I guess the results wouldn't be cached anymore, every view on a broadcast would be expensive).
Maybe stage testing with the broadcast would be appropriate.