Refactor AR Screen to not destroy all pieces' node #391

KurohanaJuri · 2022-05-17T18:52:51Z

Refactor the AR scene to not destroy the piece's nodes at each update.

In this refactor, we integrate the AR screen with the new delegate introduced in #331

The update function takes the current a list of piece present on the board (from the state) and compare with the local version to update (move, delete, add piece for the promotion) the view.

This refactor produces the same behaviour that what we have in the main branch.

Some line cannot be tested because it required to create a screen, and we still have the bug #213.

This code still have a problem for the future issue #369, for some reason we cannot call smooth which is used to animate the 3D model. (Need more investigation)

Closes #284, Closes #377

Only display the starting baord

Still need to be clean and update tests

…update_pieces

Refactor `loadStartingBoard` - renamed to `loadBoard` - This function load the board then the given pieces - Save the BBox when `loadBoard` called

Remove tests that are now irrevelant

…update_pieces

…oid into refactor/cyk/update_pieces

The default value seems not enough for the CI

alexandrepiveteau

Since ArGameScreenState and ChessBoardState share a lot of functionality and implementation, we should probably use ChessBoardState in the StatefulArScreen.

mobile/src/main/java/ch/epfl/sdp/mobile/ui/game/ar/ChessScene.kt

Move the decleration insize the scope, that allow use to remove the null check

Co-authored-by: Alexandre Piveteau <alexandre@piveteau.email>

…oid into refactor/cyk/update_pieces

alexandrepiveteau

LGTM, thanks !

Fouad-sys

Great work! Just a few documentation typos and suggestions to follow our documentation convention, but overall looks great!

mobile/src/main/java/ch/epfl/sdp/mobile/ui/game/ar/ChessScene.kt

mobile/src/main/java/ch/epfl/sdp/mobile/ui/game/ar/ArChessBoardScreen.kt

mobile/src/androidTest/java/ch/epfl/sdp/mobile/test/ui/game/ar/ChessSceneTest.kt

Co-authored-by: Fouad-sys <61212919+Fouad-sys@users.noreply.github.com>

Fouad-sys

Thanks!

codeclimate · 2022-05-25T18:27:31Z

Code Climate has analyzed commit c28d57f and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 83.3% (80% is the threshold).

This pull request will bring the total coverage in the repository to 96.7% (0.2% change).

View more on Code Climate.

KurohanaJuri and others added 17 commits May 15, 2022 22:32

Integrate the AR with the new delegation state

0334155

Only display the starting baord

Piece move on state changes

c58be93

Change currentPieces map type

d74a32c

Remove old pieces

f2d4b27

Merge branch 'main' into refactor/cyk/update_pieces

4f143d0

Piece update correctly (eat, move, promotion)

17d9bca

Still need to be clean and update tests

Merge branch 'main' of github.com:epfl-SDP/android into refactor/cyk/…

7df005f

…update_pieces

Remove unused functions

472e98f

Extract to have a function that create and place a pieces

f0668bf

re

d275abb

Refactor `loadStartingBoard` - renamed to `loadBoard` - This function load the board then the given pieces - Save the BBox when `loadBoard` called

Remove null check on context

6b54b6a

Clean updateBoard

811181d

Remove private on context

877bf0d

Remove useless check isLoaded

5221135

Add doc on ArGameScrrenState

f1a7528

Add doc on DelegatingArState

94d0bf5

update old tests

538f1ec

Remove tests that are now irrevelant

KurohanaJuri changed the title ~~Refactor/cyk/update pieces~~ Refactor AR Screen to not destroy all pieces' node May 17, 2022

KurohanaJuri added 2 commits May 17, 2022 20:54

Add private modifier on toArPosition

308523c

Merge branch 'main' of github.com:epfl-SDP/android into refactor/cyk/…

ea23fba

…update_pieces

KurohanaJuri self-assigned this May 17, 2022

KurohanaJuri added the AR Issues related to the AR features label May 17, 2022

KurohanaJuri and others added 8 commits May 17, 2022 21:04

Fix formatting

d56f3e8

Add private context and throw error if null

215aa3d

Merge branch 'refactor/cyk/update_pieces' of github.com:epfl-SDP/andr…

341a055

…oid into refactor/cyk/update_pieces

Add and update tests on ChessScene

801de48

Use runTest instead of TestScope

2b61fef

Use truth in ChessSceneTest

3b480e1

remove useless context as parameter of some functions

76bc70e

Update outdated doc

d299d7d

KurohanaJuri added 2 commits May 24, 2022 15:43

Increase the waitUntil timeout

ef36444

The default value seems not enough for the CI

Add test for DelegatingArState

314dae5

KurohanaJuri requested a review from matt989253 May 24, 2022 18:55

alexandrepiveteau reviewed May 25, 2022

View reviewed changes

KurohanaJuri added 3 commits May 25, 2022 11:42

Remove DelegatingArState and use ChessBoardState instead

3a13d59

Remove ArGameScreenState

2782014

Create a const val for the timeout in ChessSceneTest

0517738

alexandrepiveteau reviewed May 25, 2022

View reviewed changes

mobile/src/main/java/ch/epfl/sdp/mobile/ui/game/ar/ChessScene.kt Show resolved Hide resolved

Fix typo

1827d7d

alexandrepiveteau reviewed May 25, 2022

View reviewed changes

mobile/src/main/java/ch/epfl/sdp/mobile/ui/game/ar/ChessScene.kt Outdated Show resolved Hide resolved

alexandrepiveteau reviewed May 25, 2022

View reviewed changes

mobile/src/main/java/ch/epfl/sdp/mobile/ui/game/ar/ChessScene.kt Show resolved Hide resolved

KurohanaJuri and others added 4 commits May 25, 2022 11:50

Move BBox declaration location

a85ea39

Move the decleration insize the scope, that allow use to remove the null check

Remove old documentation

54e7405

Co-authored-by: Alexandre Piveteau <alexandre@piveteau.email>

Merge branch 'refactor/cyk/update_pieces' of github.com:epfl-SDP/andr…

d555265

…oid into refactor/cyk/update_pieces

Add doc for currentPieces inside ChessScene

1bc91f2

KurohanaJuri enabled auto-merge (squash) May 25, 2022 09:57

Use ActivityScenarioRule inside ChessSceneTest

884ec5e

KurohanaJuri requested a review from alexandrepiveteau May 25, 2022 11:40

alexandrepiveteau previously approved these changes May 25, 2022

View reviewed changes

Fouad-sys reviewed May 25, 2022

View reviewed changes

KurohanaJuri dismissed alexandrepiveteau’s stale review via efed896 May 25, 2022 16:18

Fix some typo

efed896

Co-authored-by: Fouad-sys <61212919+Fouad-sys@users.noreply.github.com>

KurohanaJuri requested review from alexandrepiveteau and Fouad-sys May 25, 2022 16:19

alexandrepiveteau approved these changes May 25, 2022

View reviewed changes

Fouad-sys approved these changes May 25, 2022

View reviewed changes

Merge branch 'main' into refactor/cyk/update_pieces

c28d57f

KurohanaJuri merged commit ec8c9fb into main May 25, 2022

KurohanaJuri deleted the refactor/cyk/update_pieces branch May 25, 2022 18:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor AR Screen to not destroy all pieces' node #391

Refactor AR Screen to not destroy all pieces' node #391

KurohanaJuri commented May 17, 2022 •

edited

alexandrepiveteau left a comment

alexandrepiveteau left a comment

Fouad-sys left a comment

Fouad-sys left a comment

codeclimate bot commented May 25, 2022

Refactor AR Screen to not destroy all pieces' node #391

Refactor AR Screen to not destroy all pieces' node #391

Conversation

KurohanaJuri commented May 17, 2022 • edited

alexandrepiveteau left a comment

Choose a reason for hiding this comment

alexandrepiveteau left a comment

Choose a reason for hiding this comment

Fouad-sys left a comment

Choose a reason for hiding this comment

Fouad-sys left a comment

Choose a reason for hiding this comment

codeclimate bot commented May 25, 2022

KurohanaJuri commented May 17, 2022 •

edited