-
Notifications
You must be signed in to change notification settings - Fork 71
fix: Fix badges disappearing randomly #1671
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 6 of 6 files at r1, 2 of 2 files at r2, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @faucomte97)
game_frontend/cypress/integration/avatarWorker/codeExecution.spec.js
line 68 at r2 (raw file):
.its('consoleLog.logs') .then(logs => { const log = logs.entries().next().value[0]
Why has this changed?
game_frontend/src/pyodide/badges.ts
line 21 at r2 (raw file):
result: ComputedTurnResult, userCode: string, gameState: any
This needs a comma at the end of it I think. (So you don't use prettier do you?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 2 files at r2, 11 of 11 files at r3, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @faucomte97)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 10 of 12 files reviewed, 2 unresolved discussions (waiting on @dionizh and @razvan-pro)
game_frontend/src/pyodide/badges.ts
line 21 at r2 (raw file):
Previously, dionizh (Dioni Zhong) wrote…
This needs a comma at the end of it I think. (So you don't use prettier do you?)
Done.
Actually, Prettier removed it 😬
game_frontend/cypress/integration/avatarWorker/codeExecution.spec.js
line 68 at r2 (raw file):
Previously, dionizh (Dioni Zhong) wrote…
Why has this changed?
Reverted it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 7 of 11 files at r3, 2 of 2 files at r5, 1 of 1 files at r6, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @faucomte97)
game_frontend/src/components/NavigationBar/index.js
line 81 at r6 (raw file):
} } // return current badges
Can you add a comment about this is for the initialisation when game loads? So this is called when gameState changes from undefined to not undefined, right?
game_frontend/src/pyodide/badges.ts
line 21 at r2 (raw file):
Previously, faucomte97 (Florian Aucomte) wrote…
Done.
Actually, Prettier removed it 😬
Oh sorry this is a function parameter. I think I was thinking of object/dict.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @dionizh)
game_frontend/src/components/NavigationBar/index.js
line 81 at r6 (raw file):
Previously, dionizh (Dioni Zhong) wrote…
Can you add a comment about this is for the initialisation when game loads? So this is called when gameState changes from undefined to not undefined, right?
I've update the comment, it's not for when the gameState changes but for when the props and the state is the same (aka no new badge)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 1 files at r8, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @faucomte97)
Codecov Report
@@ Coverage Diff @@
## master #1671 +/- ##
===========================================
+ Coverage 67.32% 87.95% +20.62%
===========================================
Files 171 38 -133
Lines 3532 1046 -2486
Branches 298 159 -139
===========================================
- Hits 2378 920 -1458
+ Misses 1122 103 -1019
+ Partials 32 23 -9 |
Description
Fixes a bug whereby the last badge earned disappears when submitting the code again, under certain conditions that haven't been fully determined.
How Has This Been Tested?
Manually
This change is