Skip to content

Commit f4e8798

Browse files
veloceveloce
authored
Merge pull request #1650 from lichess-org/takeback_clock_bug
Fix takeback clock bug
2 parents 2fdad1f + bdfe6c5 commit f4e8798

File tree

3 files changed

+149
-60
lines changed

3 files changed

+149
-60
lines changed

‎lib/src/model/game/game_controller.dart

+8-8
Original file line numberDiff line numberDiff line change
@@ -508,14 +508,6 @@ class GameController extends _$GameController {
508508
final fullEvent = GameFullEvent.fromJson(event.data as Map<String, dynamic>);
509509
_socketClient.version = fullEvent.socketEventVersion;
510510

511-
if (fullEvent.game.clock != null) {
512-
_updateClock(
513-
white: fullEvent.game.clock!.white,
514-
black: fullEvent.game.clock!.black,
515-
activeSide: state.requireValue.activeClockSide,
516-
);
517-
}
518-
519511
final isOpponentOnGame =
520512
fullEvent.game.playerOf(fullEvent.game.youAre?.opposite ?? Side.white).onGame ?? false;
521513

@@ -535,6 +527,14 @@ class GameController extends _$GameController {
535527
),
536528
);
537529

530+
if (fullEvent.game.clock != null) {
531+
_updateClock(
532+
white: fullEvent.game.clock!.white,
533+
black: fullEvent.game.clock!.black,
534+
activeSide: state.requireValue.activeClockSide,
535+
);
536+
}
537+
538538
// Server asking for a resync
539539
case 'resync':
540540
_onFullReload?.call();

‎lib/src/view/game/game_body.dart

+2
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,7 @@ class GameBody extends ConsumerWidget {
151151
valueListenable: gameState.liveClock!.black,
152152
builder: (context, value, _) {
153153
return Clock(
154+
key: const ValueKey('black-clock'),
154155
timeLeft: value,
155156
active: gameState.activeClockSide == Side.black,
156157
emergencyThreshold:
@@ -201,6 +202,7 @@ class GameBody extends ConsumerWidget {
201202
valueListenable: gameState.liveClock!.white,
202203
builder: (context, value, _) {
203204
return Clock(
205+
key: const ValueKey('white-clock'),
204206
timeLeft: value,
205207
active: gameState.activeClockSide == Side.white,
206208
emergencyThreshold:

‎test/view/game/game_screen_test.dart

+139-52
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import 'dart:convert';
22

33
import 'package:chessground/chessground.dart';
44
import 'package:dartchess/dartchess.dart';
5+
import 'package:flutter/cupertino.dart';
56
import 'package:flutter/material.dart';
67
import 'package:flutter_riverpod/flutter_riverpod.dart';
78
import 'package:flutter_test/flutter_test.dart';
@@ -132,6 +133,95 @@ void main() {
132133
});
133134
});
134135

136+
group('Game action negotiation', () {
137+
testWidgets('takeback', (WidgetTester tester) async {
138+
await createTestGame(
139+
tester,
140+
pgn: 'e4 e5 Nf3',
141+
clock: const (
142+
running: true,
143+
initial: Duration(minutes: 1),
144+
increment: Duration.zero,
145+
white: Duration(seconds: 58),
146+
black: Duration(seconds: 54),
147+
emerg: Duration(seconds: 10),
148+
),
149+
);
150+
expect(find.byType(Chessboard), findsOneWidget);
151+
expect(find.byType(PieceWidget), findsNWidgets(32));
152+
153+
// black plays
154+
sendServerSocketMessages(testGameSocketUri, [
155+
'{"t": "move", "v": 1, "d": {"ply": 4, "uci": "b8c6", "san": "Nc6", "clock": {"white": 58, "black": 52}}}',
156+
]);
157+
await tester.pump(const Duration(milliseconds: 500));
158+
expect(find.byKey(const Key('c6-blackknight')), findsOneWidget);
159+
expect(
160+
tester.widgetList<Clock>(find.byType(Clock)).last.active,
161+
true,
162+
reason: 'white clock is active',
163+
);
164+
// white clock ticking
165+
await tester.pump(const Duration(seconds: 1));
166+
expect(findClockWithTime(Side.white, '0:56'), findsOneWidget);
167+
await tester.pump(const Duration(seconds: 1));
168+
expect(findClockWithTime(Side.white, '0:55'), findsOneWidget);
169+
170+
// black asks for takeback
171+
sendServerSocketMessages(testGameSocketUri, [
172+
'{"t":"takebackOffers","v":2,"d":{"black":true}}',
173+
]);
174+
await tester.pump(const Duration(milliseconds: 1));
175+
176+
// see takeback button
177+
expect(find.byIcon(CupertinoIcons.arrowshape_turn_up_left), findsOneWidget);
178+
await tester.tap(find.byIcon(CupertinoIcons.arrowshape_turn_up_left));
179+
// wait for the popup to show (cannot use pumpAndSettle because of clocks)
180+
await tester.pump(const Duration(milliseconds: 100));
181+
await tester.tap(find.text('Accept'));
182+
await tester.pump(const Duration(milliseconds: 10));
183+
// server acknowledges the takeback and ask client to reload
184+
sendServerSocketMessages(testGameSocketUri, ['{"v": 3}', '{"t":"reload","v":4,"d":null}']);
185+
// wait for client to reconnect
186+
await tester.pump(const Duration(milliseconds: 1));
187+
// socket will reconnect, wait for connection
188+
await tester.pump(kFakeWebSocketConnectionLag);
189+
// server sends 'full' event immediately after reconnect
190+
sendServerSocketMessages(testGameSocketUri, [
191+
makeFullEvent(
192+
const GameId('qVChCOTc'),
193+
'e4 e5 Nf3',
194+
whiteUserName: 'Peter',
195+
blackUserName: 'Steven',
196+
youAre: Side.white,
197+
socketVersion: 5,
198+
clock: (
199+
running: true,
200+
initial: const Duration(minutes: 1),
201+
increment: Duration.zero,
202+
white: const Duration(seconds: 55),
203+
black: const Duration(seconds: 53),
204+
emerg: const Duration(seconds: 10),
205+
),
206+
),
207+
]);
208+
await tester.pump(const Duration(milliseconds: 1));
209+
210+
// black move is cancelled
211+
expect(find.byKey(const Key('c6-blackknight')), findsNothing);
212+
expect(find.byKey(const Key('b8-blackknight')), findsOneWidget);
213+
expect(tester.widget<Clock>(findClock(Side.black)).active, true);
214+
expect(tester.widget<Clock>(findClock(Side.white)).active, false);
215+
// black clock is ticking
216+
await tester.pump(const Duration(seconds: 1));
217+
expect(findClockWithTime(Side.black, '0:52'), findsOneWidget);
218+
expect(findClockWithTime(Side.white, '0:55'), findsOneWidget);
219+
await tester.pump(const Duration(seconds: 1));
220+
expect(findClockWithTime(Side.black, '0:51'), findsOneWidget);
221+
expect(findClockWithTime(Side.white, '0:55'), findsOneWidget);
222+
});
223+
});
224+
135225
group('Castling', () {
136226
const String castlingSetupPgn = 'e4 e5 Nf3 Nf6 Bc4 Bc5 d3 d6 Bd2 Bd7 Nc3 Nc6 Qe2 Qe7';
137227

@@ -287,7 +377,8 @@ void main() {
287377
group('Clock', () {
288378
testWidgets('loads on game start', (WidgetTester tester) async {
289379
await createTestGame(tester);
290-
expect(findClockWithTime('3:00'), findsNWidgets(2));
380+
expect(findClockWithTime(Side.white, '3:00'), findsOneWidget);
381+
expect(findClockWithTime(Side.black, '3:00'), findsOneWidget);
291382
expect(
292383
tester
293384
.widgetList<Clock>(find.byType(Clock))
@@ -300,7 +391,8 @@ void main() {
300391

301392
testWidgets('ticks after the first full move', (WidgetTester tester) async {
302393
await createTestGame(tester);
303-
expect(findClockWithTime('3:00'), findsNWidgets(2));
394+
expect(findClockWithTime(Side.white, '3:00'), findsOneWidget);
395+
expect(findClockWithTime(Side.black, '3:00'), findsOneWidget);
304396
await playMove(tester, 'e2', 'e4');
305397
// at that point clock is not yet started
306398
expect(
@@ -316,15 +408,11 @@ void main() {
316408
'{"t": "move", "v": 2, "d": {"ply": 2, "uci": "e7e5", "san": "e5", "clock": {"white": 180, "black": 180}}}',
317409
]);
318410
await tester.pump(const Duration(milliseconds: 10));
319-
expect(
320-
tester.widgetList<Clock>(find.byType(Clock)).last.active,
321-
true,
322-
reason: 'my clock is now active',
323-
);
411+
expect(tester.widget<Clock>(findClock(Side.white)).active, true);
324412
await tester.pump(const Duration(seconds: 1));
325-
expect(findClockWithTime('2:59'), findsOneWidget);
413+
expect(findClockWithTime(Side.white, '2:59'), findsOneWidget);
326414
await tester.pump(const Duration(seconds: 1));
327-
expect(findClockWithTime('2:58'), findsOneWidget);
415+
expect(findClockWithTime(Side.white, '2:58'), findsOneWidget);
328416
});
329417

330418
testWidgets('ticks immediately when resuming game', (WidgetTester tester) async {
@@ -340,17 +428,13 @@ void main() {
340428
emerg: Duration(seconds: 30),
341429
),
342430
);
343-
expect(
344-
tester.widgetList<Clock>(find.byType(Clock)).first.active,
345-
true,
346-
reason: 'black clock is already active',
347-
);
348-
expect(findClockWithTime('2:58'), findsOneWidget);
349-
expect(findClockWithTime('2:54'), findsOneWidget);
431+
expect(tester.widget<Clock>(findClock(Side.black)).active, true);
432+
expect(findClockWithTime(Side.white, '2:58'), findsOneWidget);
433+
expect(findClockWithTime(Side.black, '2:54'), findsOneWidget);
350434
await tester.pump(const Duration(seconds: 1));
351-
expect(findClockWithTime('2:53'), findsOneWidget);
435+
expect(findClockWithTime(Side.black, '2:53'), findsOneWidget);
352436
await tester.pump(const Duration(seconds: 1));
353-
expect(findClockWithTime('2:52'), findsOneWidget);
437+
expect(findClockWithTime(Side.black, '2:52'), findsOneWidget);
354438
});
355439

356440
testWidgets('switch timer side after a move', (WidgetTester tester) async {
@@ -366,43 +450,44 @@ void main() {
366450
emerg: Duration(seconds: 30),
367451
),
368452
);
369-
expect(tester.widgetList<Clock>(find.byType(Clock)).last.active, true);
453+
expect(tester.widget<Clock>(findClock(Side.white)).active, true);
370454
// simulates think time of 3s
371455
await tester.pump(const Duration(seconds: 3));
372456
await playMove(tester, 'g1', 'f3');
373-
expect(findClockWithTime('2:55'), findsOneWidget);
457+
expect(findClockWithTime(Side.white, '2:55'), findsOneWidget);
374458
expect(
375-
tester.widgetList<Clock>(find.byType(Clock)).last.active,
459+
tester.widget<Clock>(findClock(Side.white)).active,
376460
false,
377461
reason: 'white clock is stopped while waiting for server ack',
378462
);
379463
expect(
380-
tester.widgetList<Clock>(find.byType(Clock)).first.active,
464+
tester.widget<Clock>(findClock(Side.black)).active,
381465
true,
382466
reason: 'black clock is now active but not yet ticking',
383467
);
384-
expect(findClockWithTime('3:00'), findsOneWidget);
468+
expect(findClockWithTime(Side.black, '3:00'), findsOneWidget);
385469
// simulates a long lag just to show the clock is not running yet
386470
await tester.pump(const Duration(milliseconds: 200));
387-
expect(findClockWithTime('3:00'), findsOneWidget);
471+
expect(findClockWithTime(Side.black, '3:00'), findsOneWidget);
388472
// server ack having the white clock updated with the increment
389473
sendServerSocketMessages(testGameSocketUri, [
390474
'{"t": "move", "v": 1, "d": {"ply": 3, "uci": "g1f3", "san": "Nf3", "clock": {"white": 177, "black": 180}}}',
391475
]);
392476
await tester.pump(const Duration(milliseconds: 10));
393477
// we see now the white clock has got its increment
394-
expect(findClockWithTime('2:57'), findsOneWidget);
478+
expect(findClockWithTime(Side.white, '2:57'), findsOneWidget);
395479
await tester.pump(const Duration(milliseconds: 100));
396480
// black clock is ticking
397-
expect(findClockWithTime('2:59'), findsOneWidget);
481+
expect(findClockWithTime(Side.black, '2:59'), findsOneWidget);
398482
await tester.pump(const Duration(seconds: 1));
399-
expect(findClockWithTime('2:57'), findsOneWidget);
400-
expect(findClockWithTime('2:58'), findsOneWidget);
483+
expect(findClockWithTime(Side.white, '2:57'), findsOneWidget);
484+
expect(findClockWithTime(Side.black, '2:58'), findsOneWidget);
401485
await tester.pump(const Duration(seconds: 1));
402-
expect(findClockWithTime('2:57'), findsNWidgets(2));
486+
expect(findClockWithTime(Side.white, '2:57'), findsOneWidget);
487+
expect(findClockWithTime(Side.black, '2:57'), findsOneWidget);
403488
await tester.pump(const Duration(seconds: 1));
404-
expect(findClockWithTime('2:57'), findsOneWidget);
405-
expect(findClockWithTime('2:56'), findsOneWidget);
489+
expect(findClockWithTime(Side.white, '2:57'), findsOneWidget);
490+
expect(findClockWithTime(Side.black, '2:56'), findsOneWidget);
406491
});
407492

408493
testWidgets('compensates opponent lag', (WidgetTester tester) async {
@@ -436,15 +521,15 @@ void main() {
436521
socketVersion: ++socketVersion,
437522
);
438523
// black clock is active
439-
expect(tester.widgetList<Clock>(find.byType(Clock)).first.active, true);
440-
expect(findClockWithTime('0:54'), findsOneWidget);
524+
expect(tester.widget<Clock>(findClock(Side.black)).active, true);
525+
expect(findClockWithTime(Side.black, '0:54'), findsOneWidget);
441526
await tester.pump(const Duration(milliseconds: 250));
442527
// lag is 250ms, so clock will only start after that delay
443-
expect(findClockWithTime('0:54'), findsOneWidget);
528+
expect(findClockWithTime(Side.black, '0:54'), findsOneWidget);
444529
await tester.pump(const Duration(milliseconds: 100));
445-
expect(findClockWithTime('0:53'), findsOneWidget);
530+
expect(findClockWithTime(Side.black, '0:53'), findsOneWidget);
446531
await tester.pump(const Duration(seconds: 1));
447-
expect(findClockWithTime('0:52'), findsOneWidget);
532+
expect(findClockWithTime(Side.black, '0:52'), findsOneWidget);
448533
});
449534

450535
testWidgets('onEmergency', (WidgetTester tester) async {
@@ -464,11 +549,11 @@ void main() {
464549
overrides: [soundServiceProvider.overrideWith((_) => mockSoundService)],
465550
);
466551
expect(
467-
tester.widget<Clock>(findClockWithTime('0:40')).emergencyThreshold,
552+
tester.widget<Clock>(findClockWithTime(Side.white, '0:40')).emergencyThreshold,
468553
const Duration(seconds: 30),
469554
);
470555
await tester.pump(const Duration(seconds: 10));
471-
expect(findClockWithTime('0:30'), findsOneWidget);
556+
expect(findClockWithTime(Side.white, '0:30'), findsOneWidget);
472557
verify(() => mockSoundService.play(Sound.lowTime)).called(1);
473558
});
474559

@@ -489,19 +574,15 @@ void main() {
489574
emerg: Duration(seconds: 30),
490575
),
491576
);
492-
expect(
493-
tester.widgetList<Clock>(find.byType(Clock)).first.active,
494-
true,
495-
reason: 'black clock is active',
496-
);
577+
expect(tester.widget<Clock>(findClock(Side.black)).active, true);
497578

498-
expect(findClockWithTime('2:58'), findsOneWidget);
499-
expect(findClockWithTime('2:54'), findsOneWidget);
579+
expect(findClockWithTime(Side.white, '2:58'), findsOneWidget);
580+
expect(findClockWithTime(Side.black, '2:54'), findsOneWidget);
500581
await tester.pump(const Duration(seconds: 1));
501-
expect(findClockWithTime('2:53'), findsOneWidget);
582+
expect(findClockWithTime(Side.black, '2:53'), findsOneWidget);
502583
await tester.pump(const Duration(minutes: 2, seconds: 53));
503-
expect(findClockWithTime('2:58'), findsOneWidget);
504-
expect(findClockWithTime('0:00.0'), findsOneWidget);
584+
expect(findClockWithTime(Side.white, '2:58'), findsOneWidget);
585+
expect(findClockWithTime(Side.black, '0:00.0'), findsOneWidget);
505586

506587
expect(
507588
tester.widgetList<Clock>(find.byType(Clock)).first.active,
@@ -531,8 +612,8 @@ void main() {
531612
2,
532613
reason: 'both clocks are now inactive',
533614
);
534-
expect(findClockWithTime('2:58'), findsOneWidget);
535-
expect(findClockWithTime('0:00.00'), findsOneWidget);
615+
expect(findClockWithTime(Side.white, '2:58'), findsOneWidget);
616+
expect(findClockWithTime(Side.black, '0:00.00'), findsOneWidget);
536617

537618
// wait for the dong
538619
await tester.pump(const Duration(seconds: 500));
@@ -654,10 +735,16 @@ void main() {
654735
});
655736
}
656737

657-
Finder findClockWithTime(String text, {bool skipOffstage = true}) {
738+
/// Finds the clock on the specified [side].
739+
Finder findClock(Side side, {bool skipOffstage = true}) {
740+
return find.byKey(ValueKey('${side.name}-clock'), skipOffstage: skipOffstage);
741+
}
742+
743+
/// Finds the clock with the given [text] on the specified [side].
744+
Finder findClockWithTime(Side side, String text, {bool skipOffstage = true}) {
658745
return find.ancestor(
659746
of: find.text(text, findRichText: true, skipOffstage: skipOffstage),
660-
matching: find.byType(Clock, skipOffstage: skipOffstage),
747+
matching: find.byKey(ValueKey('${side.name}-clock'), skipOffstage: skipOffstage),
661748
);
662749
}
663750

0 commit comments

Comments
 (0)