This repository has been archived by the owner on Jul 7, 2022. It is now read-only.
Refactor : Use state in AR Chess board #251
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Create `ClassicChessBoardState` that correspond to the old one; This state is used to display a classic 2D board - The new `ChessBoardState` is more generic which means that this class only contains general information that need to be display (Pieces position, King in check position)
…rfact with the board
Delete useless files Modifications: - The contents descriptions for `Rank` and `Color` are moved outside, Now they are defined in `ChessBoard` - Piece icon are now outside of the state -> In `ChessBoard`
- The default implmentation implements `pieces` and `checkPosition` that are commun for AR and 2D
board
…sBoardState` The defautl chess board state was removed because it's difficule to deal with generic type. For the moement, we have duplicate on Ar and classic chess board state, We need to fix this later.
This class implment values that are commun in classic chess board and AR chess board
- Add `ArGameScreenState` that describe what is needed to display a AR board - Add converstion between engine rank and AR model path - Same for the colours - Modify the `StatefulArScreen` parameter, now the screen need a match id
The test is broken and doesn't use the new implementation
mobile/src/androidTest/java/ch/epfl/sdp/mobile/test/state/Utils.kt
Outdated
Show resolved
Hide resolved
alexandrepiveteau
previously approved these changes
Apr 22, 2022
mobile/src/androidTest/java/ch/epfl/sdp/mobile/test/state/Utils.kt
Outdated
Show resolved
Hide resolved
Co-authored-by: Alexandre Piveteau <[email protected]>
alexandrepiveteau
previously approved these changes
Apr 22, 2022
matt989253
suggested changes
Apr 25, 2022
mobile/src/androidTest/java/ch/epfl/sdp/mobile/test/ui/game/ar/ChessSceneTest.kt
Outdated
Show resolved
Hide resolved
mobile/src/androidTest/java/ch/epfl/sdp/mobile/test/ui/game/ar/ChessSceneTest.kt
Outdated
Show resolved
Hide resolved
mobile/src/androidTest/java/ch/epfl/sdp/mobile/test/ui/game/ar/ChessSceneTest.kt
Outdated
Show resolved
Hide resolved
mobile/src/androidTest/java/ch/epfl/sdp/mobile/test/ui/game/ar/ChessSceneTest.kt
Outdated
Show resolved
Hide resolved
mobile/src/androidTest/java/ch/epfl/sdp/mobile/test/ui/game/ar/ChessSceneTest.kt
Outdated
Show resolved
Hide resolved
mobile/src/androidTest/java/ch/epfl/sdp/mobile/test/ui/game/ar/ChessSceneTest.kt
Outdated
Show resolved
Hide resolved
mobile/src/androidTest/java/ch/epfl/sdp/mobile/test/ui/game/ar/ChessSceneTest.kt
Outdated
Show resolved
Hide resolved
Co-authored-by: Matthieu Burguburu <[email protected]>
|
Code Climate has analyzed commit 34d371c and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 94.0% (80% is the threshold). This pull request will bring the total coverage in the repository to 97.7% (0.7% change). View more on Code Climate. |
alexandrepiveteau
approved these changes
Apr 25, 2022
matt989253
approved these changes
Apr 26, 2022
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR refactors the AR chess board.
Now the chess board takes a state that represent the state of a specific game, which means that the tab in the bottom bar is now removed.
For now, we use
SnapshotChessBoardStatewhich is not perfect, some methods aren't required for the AR screen (e.g.onArClick). But it's the faster way to have something that works with state. We can fix with a cleaner code later.The test mentioned in #215 is up again, updated with the new implementation. I tested locally on devices with and without AR Core. It seems that the bug disappears.
Fixed #239