feat: add tournament screen
With this PR, we'd have very basic support for tournaments. However, the game screen is still missing some features, like berserking, displaying rank, etc. That will be a follow-up PR. Same goes for the "podium" screen for finished tournaments.
I did not add widget tests yet, wanted to get feedback on the layout/design first.
The tournament screen is great! Love it. I think it would be useful to also have the players' recent results (at least the last one) to know if one needs to win one or two games to get the win-streak buff. In an end tournament rush, it can help to make strategic decisions.
Nice work on the UI! Will review asap.
Thanks for the review! I added two widgets tests now, let me know if you think there's anything else that should be tested
Just need to change the mocking of data in tests and it's good!
Maybe I overlooked your comment about this, or you forgot to submit - What needs to be changed?
It would be nice to add some other tests to verify we cannot join the tournament if conditions (tournament conditions or not logged in) are not met.
Will do 👍
Once the logic is implemented, we'd want to test the redirect to a game. But that's for another PR.
The redirect is already implemented actually, but not via the socket message, as we could miss the message while we're still in the game. Instead, we're watching the gameId here: https://github.com/tom-anders/lichess-mobile/blob/tournamentScreen/lib/src/view/tournament/tournament_screen.dart#L47
So we could (and should) already test it I think
Maybe I overlooked your comment about this, or you forgot to submit - What needs to be changed?
Oops. Looks like this comment got lost somehow... made another one: https://github.com/lichess-org/mobile/pull/1583#discussion_r2022706754
Once the logic is implemented, we'd want to test the redirect to a game. But that's for another PR.
The redirect is already implemented actually, but not via the socket message, as we could miss the message while we're still in the game. Instead, we're watching the
gameIdhere: https://github.com/tom-anders/lichess-mobile/blob/tournamentScreen/lib/src/view/tournament/tournament_screen.dart#L47So we could (and should) already test it I think
Hmm, I tried, but I keep getting
══╡ EXCEPTION CAUGHT BY FLUTTER TEST FRAMEWORK ╞════════════════════════════════════════════════════
The following assertion was thrown running a test:
A Timer is still pending even after the widget tree was disposed.
'package:flutter_test/src/binding.dart':
Failed assertion: line 1606 pos 12: '!timersPending'
in the test when the GameScreen is opened. The stack trace looks related to the websocket client, but I'm not familiar enough with the game screen to figure out why that happens
Once the logic is implemented, we'd want to test the redirect to a game. But that's for another PR.
The redirect is already implemented actually, but not via the socket message, as we could miss the message while we're still in the game. Instead, we're watching the
gameIdhere: https://github.com/tom-anders/lichess-mobile/blob/tournamentScreen/lib/src/view/tournament/tournament_screen.dart#L47 So we could (and should) already test it I thinkHmm, I tried, but I keep getting
══╡ EXCEPTION CAUGHT BY FLUTTER TEST FRAMEWORK ╞════════════════════════════════════════════════════ The following assertion was thrown running a test: A Timer is still pending even after the widget tree was disposed. 'package:flutter_test/src/binding.dart': Failed assertion: line 1606 pos 12: '!timersPending'in the test when the
GameScreenis opened. The stack trace looks related to the websocket client, but I'm not familiar enough with the game screen to figure out why that happens
I pushed a commit with that test if you want to try to fix it. Otherwise, we can also skip that test for now until we figure out how to do it
I pushed a commit with that test if you want to try to fix it. Otherwise, we can also skip that test for now until we figure out how to do it
The reason why timers were still pending is that the game controller onDispose callback was not called. I am not sure exactly what is happening on riverpod side, but that happens if the tests don't give the controller the time to properly load.
Here this future was never resolved, so the test failed.
So we cannot just expect GameScreen, we need to load the game.
https://github.com/user-attachments/assets/6a361f49-867a-4df3-8c7a-a1b988536d33
Problem with scrolling on iOS. This is due to the featured game clocks.
Thanks, will take another look!
While I'm at it, I'll also refactor the testing code to be end to-end as discussed
Couple of notes:
* the whole screen should probably be refreshable (pull-to-refresh)
The server already sends a reload event when something notable changes, so I don't think manual refreshing would have any benefit
* the featured game doesn't seem to move to the next one
Ah, it's because currently we only allow one socket at a time: https://github.com/lichess-org/mobile/blob/05d57ced0faf9bf09ac591b64ad4d1a68db89f9a/lib/src/network/socket.dart#L535
But here, we need the tournament socket to stay active. If I delete that linked code, moving to the next game works (but it's probably not the solution we want...?)
The server already sends a reload event when something notable changes, so I don't think manual refreshing would have any benefit
Then it needs to register app lifecycle onPause/onResume callbacks to force a reload on resume
But here, we need the tournament socket to stay active. If I delete that linked code, moving to the next game works (but it's probably not the solution we want...?)
indeed it is by design we don't want more than one socket connection at a time; how does it work on the website? I don't think the tournament page opens 2 sockets. IIRC the old mobile app also do not open 2 sockets at the same time and still has the featured game.
The server already sends a reload event when something notable changes, so I don't think manual refreshing would have any benefit
Then it needs to register app lifecycle onPause/onResume callbacks to force a reload on resume
Done :+1:
But here, we need the tournament socket to stay active. If I delete that linked code, moving to the next game works (but it's probably not the solution we want...?)
indeed it is by design we don't want more than one socket connection at a time; how does it work on the website? I don't think the tournament page opens 2 sockets. IIRC the old mobile app also do not open 2 sockets at the same time and still has the featured game.
Ah, I checked with Firefox devtools, the website sends a startWatching to the server, so we need to do the same. I guess that means we can't reuse the TvController anymore, but that's fine
Alright, we're using the tournament websocket now for displaying the featured game, now everything works even when there's a new featured game.
I also made the board look more like in broadcasts:
Sorry it messed the PR commit history...
Will merge soon I think, still some testing and a couple of UI changes.