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

buxfix: "urgent" game notification handling #2905

Merged
merged 1 commit into from
Jan 1, 2025

Conversation

benjaminpjones
Copy link
Contributor

@benjaminpjones benjaminpjones commented Jan 1, 2025

time_to_move does not exist on the boards in notification manager. Yay TypeScript 🥳


In case you don't believe typescript, one can also verify in the console:

> Object.values(notification_manager.active_boards)[0].time_per_move
88449
> Object.values(notification_manager.active_boards)[0].time_to_move
undefined 

The bug: time_to_move does not exist on the boards in notification manager.
adding types reveals this error.
@benjaminpjones benjaminpjones changed the title buxfix: "urgent" game notifications buxfix: "urgent" game notification handling Jan 1, 2025
@benjaminpjones
Copy link
Contributor Author

Requesting @GreenAsJade review, as I'm not totally clear on how the urgent game logic (#1802) is supposed to work.

Copy link

github-actions bot commented Jan 1, 2025

Uffizzi Preview deployment-59406 was deleted.

Copy link
Contributor

@GreenAsJade GreenAsJade left a comment

Choose a reason for hiding this comment

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

Looks like the right fix to me.

Back when this bug was created (by me), we didn't even have ClientToServer types :)

It would not have been experienced as a "bug" by users, unless they specifically knew what we were trying to do.

The effect of fixing it should be that when you play a move in a live game, and you have more than two lives game going and you have auto-advance on, then you should arrive at the most urgent game rather than the next in the sequence from where you were up to circling through them. Subtle :)

@benjaminpjones
Copy link
Contributor Author

Back when this bug was created (by me), we didn't even have ClientToServer types :)

I figured! It may not have even been a bug at that point in time 🤷‍♂️

The effect of fixing it should be that when you play a move in a live game, and you have more than two lives game going and you have auto-advance on, then you should arrive at the most urgent game rather than the next in the sequence from where you were up to circling through them. Subtle :)

Ah, that makes sense! I can see why no one complained about the bug - very few simul players and urgency may not often be an issue. But this seems like the right behavior for the tight cases 👍

@GreenAsJade
Copy link
Contributor

I had a little dig in the back end - it looks like it was really a bug.

@anoek
Copy link
Member

anoek commented Jan 1, 2025

Thanks for the fix!

@anoek anoek merged commit 482c73d into online-go:devel Jan 1, 2025
4 checks passed
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.

3 participants