Skip to content

Conversation

@andy-ife
Copy link
Contributor

This pr adds three new features to over the board games: resign, offer draw, and a notification on threefold repetition. No new widget or class was added, and these features were implemented much like they are in regular online games.

Fixes #1029

Changes made:

  • OverTheBoardGame model class: Added three new getters: abortable, resignable and drawable. Calculated the same way as in the Game model class.
  • OverTheBoardGameController: Added two new functions: resign() and draw(). Also added a check for a threefold repetition on every move.
  • OverTheBoardScreen: Added a hamburger menu to the UI, added listeners for the new controller functions, and used YesNoDialog to request user confirmations.

Tests performed:

  • Ran flutter test. Some tests failed, but these were not due to any changes I made
  • Ran flutter analyze. No issues
  • Manually tested the new features - debug build on an Android device, API Level 31
draw.mp4
resign.mp4
threefold.mp4

Copy link
Collaborator

@tom-anders tom-anders left a comment

Choose a reason for hiding this comment

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

Nice work! :)

),
BottomSheetAction(
makeLabel: (context) => Text(context.l10n.flipBoard),
onPressed: onFlipBoard,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Isn't there already a bottom bar button for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, there is. I'll remove the option from the menu. This will leave the hamburger menu with only one option (New game) at the start of a game, making the menu pointless for that state. However, when the game starts and the Resign and Draw options come into play, the hamburger menu is needed. So if it's fine by you, I'll make the bottom bar button switch between the plus icon and the menu icon as required.

Alternatively, we could just leave it as is (regular online games have a resign button in the bottom bar AND in the menu. Also I'm lazy haha)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah that makes sense, got it! I think switching between hamburger/plus icon sounds good, but I'm also ok with leaving it like it is is right now.

@veloce has the final word on that as the maintainer, so let's wait for his opinion

Copy link
Contributor

Choose a reason for hiding this comment

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

Is the flip button that important so it needs to be in the bottom bar? Because we can also remove it from the bottom bar.

If not I prefer leaving the hamburger menu all the time and not switching between icons.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@andy-ife andy-ife force-pushed the over-the-board-feats branch from 1fac83c to 9543b9b Compare March 22, 2025 00:29
Copy link
Contributor

@veloce veloce left a comment

Choose a reason for hiding this comment

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

Nice work! only a couple of small comments.


if (previous?.game.isThreefoldRepetition == false &&
newGameState.game.isThreefoldRepetition == true) {
ref.read(overTheBoardClockProvider.notifier).pause();
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't the clock be paused at the same time the dialog is displayed? It is weird to pause it before.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was done the same way in the previous if check for gameState.finished. Would you like me to change both?

Copy link
Contributor

Choose a reason for hiding this comment

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

Well no, because if the game is finished it makes sense to stop the clock immediately.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's true

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

bool get abortable =>
playable &&
lastPosition.fullmoves <= 1 &&
(meta.rules == null || !meta.rules!.contains(GameRule.noAbort));
Copy link
Contributor

Choose a reason for hiding this comment

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

You can remove that. meta is only available in online games.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

(meta.rules == null || !meta.rules!.contains(GameRule.noAbort));

bool get resignable => playable && !abortable;
bool get drawable => playable && lastPosition.fullmoves >= 2 && !(me?.offeringDraw == true);
Copy link
Contributor

Choose a reason for hiding this comment

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

You can remove the me?.offeringDraw, it is only available in online games.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@veloce veloce merged commit 30fd0c3 into lichess-org:main Apr 4, 2025
1 check 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.

[Feature] Over the board resign and draw function

3 participants