Skip to content
This repository has been archived by the owner on Aug 7, 2024. It is now read-only.

fix: adding pause #1744

Merged
39 commits merged into from
Mar 20, 2023
Merged

fix: adding pause #1744

39 commits merged into from
Mar 20, 2023

Conversation

KamilPawel
Copy link
Contributor

@KamilPawel KamilPawel commented Feb 28, 2023

Description

How Has This Been Tested?

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have linked this PR to a ZenHub Issue.

This change is Reviewable

Copy link
Contributor

@faucomte97 faucomte97 left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 6 files at r3, 8 of 26 files at r5, 2 of 13 files at r6, 29 of 31 files at r7, 2 of 2 files at r8, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @KamilPawel)


game_frontend/src/containers/IDEEditor/index.js line 148 at r8 (raw file):

            variant="outlined"
            onClick={this.onPauseClicked}
            startIcon={<PauseCircleFilled />}

This icon needs to change when the game is paused, ideally to a play arrow, like a "resume" arrow like on YouTube for example.


game_frontend/src/redux/features/Game/epics.js line 87 at r8 (raw file):

  action$.pipe(
    ofType(types.TOGGLE_PAUSE_GAME),
    filter(() => state$.value.game.gamePaused === true),

do we need === true?


game_frontend/src/redux/features/Game/epics.js line 89 at r8 (raw file):

    filter(() => state$.value.game.gamePaused === true),
    map(() =>
      avatarWorkerActions.avatarsNextActionComputed({ turnCount: state$.value.game.gameState.turnCount + 1, log: "You have paused the game" })

I know it wasn't in the task description but can we have a log similar to this one when the player resumes the game? Something like "You have resumed the game"


game_frontend/src/redux/features/Game/epics.js line 96 at r8 (raw file):

  action$.pipe(
    ofType(types.TOGGLE_PAUSE_GAME),
    filter(() => state$.value.game.gamePaused === true),

Same question here about the === true

Copy link
Contributor

@faucomte97 faucomte97 left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 2 of 2 files at r9, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @KamilPawel)

@codecov
Copy link

codecov bot commented Mar 20, 2023

Codecov Report

Merging #1744 (60c5b4d) into master (a7574d4) will increase coverage by 21.63%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master    #1744       +/-   ##
===========================================
+ Coverage   66.32%   87.95%   +21.63%     
===========================================
  Files         179       38      -141     
  Lines        3661     1046     -2615     
  Branches      255      109      -146     
===========================================
- Hits         2428      920     -1508     
+ Misses       1201      103     -1098     
+ Partials       32       23        -9     
Impacted Files Coverage Δ
aimmo-game-worker/simulation/utils.py 100.00% <100.00%> (ø)

... and 141 files with indirect coverage changes

@ghost ghost merged commit 764ae03 into master Mar 20, 2023
@ghost ghost deleted the pause branch March 20, 2023 15:19
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants