Skip to content
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.

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

Support emt tag in study #15062

Merged
merged 12 commits into from
Apr 15, 2024

Conversation

lenguyenthanh
Copy link
Member

@lenguyenthanh lenguyenthanh commented Apr 10, 2024

I quickly added emt field to study to fix #15037 and other related issues.

It requires new field in database, but I guess it's fine.

Is this something We should support?

cc @kraktus @ornicar

TODO

  • fix tests
  • tests
  • json
  • calculate clock via emt if necessary
  • update api

@lenguyenthanh lenguyenthanh force-pushed the support-emt-tag-in-study branch from 764d9a5 to d5b0930 Compare April 10, 2024 13:31
@lenguyenthanh lenguyenthanh changed the title Add emt field to Study & parse it Support emt tag in study Apr 10, 2024
@kraktus
Copy link
Member

kraktus commented Apr 10, 2024

Looking good 👍 At least to support it in scalachess, that way Lila has the liberty to use them as needed, even though they're not a great source of information.

@kraktus
Copy link
Member

kraktus commented Apr 10, 2024

NVM this is Lila, not scalachess. Do you think we need to store EMT separately, instead of converting it directly to clock time?

@lenguyenthanh
Copy link
Member Author

Do you think we need to store EMT separately, instead of converting it directly to clock time?

Oh yeah, this is a great idea saving database's field is a (probably) always good idea!
The only issue is We need to accumulate the clocks somehow. Let me see how to do that.

@lenguyenthanh lenguyenthanh force-pushed the support-emt-tag-in-study branch 2 times, most recently from b7400ac to f9111ac Compare April 12, 2024 01:43
@lenguyenthanh lenguyenthanh marked this pull request as ready for review April 12, 2024 02:36
@lenguyenthanh lenguyenthanh force-pushed the support-emt-tag-in-study branch from cc80dd6 to a0d1cc4 Compare April 12, 2024 02:38
@lenguyenthanh lenguyenthanh force-pushed the support-emt-tag-in-study branch from 92b4cca to 20fcdcc Compare April 12, 2024 02:50
@lenguyenthanh lenguyenthanh force-pushed the support-emt-tag-in-study branch from 6872de5 to 26929a2 Compare April 12, 2024 03:08
@lenguyenthanh lenguyenthanh requested review from ornicar and kraktus and removed request for ornicar April 15, 2024 03:43
gamebook = None,
glyphs = data.metas.glyphs,
opening = None,
clock = clock.orElse((context.previousClock, emt).mapN(_ - _)),
Copy link
Collaborator

Choose a reason for hiding this comment

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

What about clock increments tho? This only works for clocks like x+0.

Or do we think it's better to estimate assuming zero increment, rather than having no clock at all?

Copy link
Collaborator

Choose a reason for hiding this comment

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

If PGN sources specified the clock format in the PGN tags, then we could use that, but they rarely do.

ornicar added 2 commits April 15, 2024 10:53
* master: (59 commits)
  remove deps to game
  remove pool dep to game
  remove deps to game
  remove simul dep to game
  scalafmt
  remove opening dep to game
  remove deps to game
  lila.core.game.Game compiles
  lila.core.game.Game WIP
  lila.core.game.Game WIP
  scalafmt
  Fix HttpFilter env
  Disable coep:credentialless on PayPal patron page
  Add setting to disable site-wide coep:credentialless
  Cleanup
  Only enable site-wide coep:credentialless in Chrome 113+
  Allow coep:credentialless on broadcasts with embeds
  eager vals, move tryDailyPuzzle to lila.puzzle.Env
  fix master merge
  fix scala syntax
  ...
@ornicar
Copy link
Collaborator

ornicar commented Apr 15, 2024

better like this, than storing emt in DB 👍

@ornicar ornicar merged commit a2cab41 into lichess-org:master Apr 15, 2024
3 checks passed
@lenguyenthanh lenguyenthanh deleted the support-emt-tag-in-study branch April 15, 2024 08:58
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.

Improving clock time visibility in broadcasts
3 participants