fix: Fix badges disappearing randomly (#1671)

* fix: Get badges after code updated * Merge branch 'master' into badges_disappearing * Merge branch 'master' into badges_disappearing * Make initial code update not trigger badge check * fix test * Try index 0 in log list in test * Filter badges in frontend * Fix tests * Run prettier * Make sure gamestate is defined * Update comment * Simplify filtering
ocadotechnology · Jun 16, 2022 · 10cabe7 · 10cabe7
1 parent 4c6af7b
commit 10cabe7
Show file tree

Hide file tree

Showing 10 changed files with 183 additions and 264 deletions.
diff --git a/Pipfile.lock b/Pipfile.lock
diff --git a/game_frontend/src/components/Badge/index.js b/game_frontend/src/components/Badge/index.js
@@ -29,16 +29,16 @@ const BadgeModalImg = styled(Box)`
 export default class BadgeModal extends Component {
   static propTypes = {
     modalOpen: PropTypes.bool,
-    taskId: PropTypes.string,
+    badgeId: PropTypes.string,
   }
 
   render() {
     if (!this.props.modalOpen) {
       return null
     }
 
-    const taskId = this.props.taskId
-    const info = badgeInfo[taskId]
+    const badgeId = this.props.badgeId
+    const info = badgeInfo[badgeId]
     if (info === undefined) {
       return null
     }
@@ -62,15 +62,15 @@ export default class BadgeModal extends Component {
   }
 }
 
-export function getBadges(tasks) {
-  return tasks.map((task) => (
+export function getBadges(badges) {
+  return badges.map((badge) => (
     <Box
       component="img"
       style={{ height: 45, marginRight: 15 }}
-      alt={badgeInfo[task].name}
-      title={badgeInfo[task].name}
-      src={badgeInfo[task].img}
-      key={task}
+      alt={badgeInfo[badge].name}
+      title={badgeInfo[badge].name}
+      src={badgeInfo[badge].img}
+      key={badge}
     />
   ))
 }
diff --git a/game_frontend/src/components/NavigationBar/index.js b/game_frontend/src/components/NavigationBar/index.js
@@ -38,11 +38,12 @@ export class NavigationBar extends Component {
   static propTypes = {
     // the props received from redux state or reducers
     modalOpen: PropTypes.bool,
-    completedTasks: PropTypes.string,
+    completedBadges: PropTypes.string,
     badgesInit: PropTypes.func,
+    gameState: PropTypes.any,
   }
 
-  state = { modalOpen: false, completedTasks: [], lastTask: '' }
+  state = { modalOpen: false, completedBadges: [], lastBadge: '' }
 
   componentDidMount() {
     this.props.badgesInit()
@@ -56,27 +57,33 @@ export class NavigationBar extends Component {
 
   static getDerivedStateFromProps(props, state) {
     // Any time completedTasks change, pass the new info as state
-    if (props.completedTasks !== undefined) {
-      // convert to string for comparison
-      const stateTasksString = state.completedTasks.join() + ','
+    if (props.completedBadges !== undefined && props.gameState !== undefined) {
+      const worksheetID = props.gameState.worksheetID
+      let badges = props.completedBadges.split(',')
+      badges = badges.filter((s) => s) // remove empty element
+      // remove any badge that's not relevant to the current worksheet
+      badges = badges.filter((b) => {
+        return b.startsWith(worksheetID + ':')
+      })
 
-      if (props.completedTasks !== stateTasksString) {
-        let newTasks = props.completedTasks.split(',')
-        newTasks = newTasks.filter((s) => s) // remove empty element
-        const lastTask = newTasks[newTasks.length - 1] // assume the last element is the last task
+      // convert to string for comparison
+      const stateBadgesString = state.completedBadges.join() + ','
 
+      if (props.completedBadges !== stateBadgesString) {
+        const lastBadge = badges[badges.length - 1] // assume the last element is the last badge
+        // return badge info with popup if there is a new badge earned
         return {
           modalOpen: props.modalOpen,
-          completedTasks: newTasks,
-          lastTask: lastTask,
+          completedBadges: badges,
+          lastBadge: lastBadge,
         }
       }
     }
     return null // no change
   }
 
   renderLogoToolbar = () => {
-    const badges = getBadges(this.state.completedTasks)
+    const badges = getBadges(this.state.completedBadges)
     return (
       <LogoToolbar>
         <IconButton href={urlForAimmoDashboard} aria-label="Kurono dashboard" color="inherit">
@@ -109,15 +116,16 @@ export class NavigationBar extends Component {
           {this.renderLogoToolbar()}
           {this.renderButtonToolbar()}
         </KuronoAppBar>
-        <BadgeModal taskId={this.state.lastTask} modalOpen={this.state.modalOpen} />
+        <BadgeModal badgeId={this.state.lastBadge} modalOpen={this.state.modalOpen} />
       </NavigationBarLayout>
     )
   }
 }
 
 const mapStateToProps = (state) => ({
-  completedTasks: state.avatarWorker.completedTasks,
+  completedBadges: state.avatarWorker.completedBadges,
   modalOpen: state.avatarWorker.modalOpen,
+  gameState: state.game.gameState,
 })
 
 const mapDispatchToProps = {

diff --git a/game_frontend/src/pyodide/badges.test.ts b/game_frontend/src/pyodide/badges.test.ts
@@ -1,27 +1,27 @@
 /* eslint-env jest */
-import { checkIfBadgeEarned, filterByWorksheet } from './badges'
+import { checkIfBadgeEarned } from './badges'
 jest.mock('threads/worker')
 
 describe('Badges check', () => {
   it('awards badge 1 if the right conditions are met', () => {
     const badges = ''
     const turnResult = {
       action: {
-        action_type: "move",
+        action_type: 'move',
         options: {
-          direction: {x: 0, y: -1}
-        }
+          direction: { x: 0, y: -1 },
+        },
       },
-      log: "",
+      log: '',
       turnCount: 1,
     }
     const userCode = ''
-    const gameState = {worksheetID: 1}
+    const gameState = { worksheetID: 1 }
     const playerAvatarId = 1
 
     const result = checkIfBadgeEarned(badges, turnResult, userCode, gameState, playerAvatarId)
 
-    const expected = "1:1,"
+    const expected = '1:1,'
 
     expect(result).toBe(expected)
   })
@@ -30,12 +30,12 @@ describe('Badges check', () => {
     const badges = ''
     const turnResult = {
       action: {
-        action_type: "move",
+        action_type: 'move',
         options: {
-          direction: {x: 0, y: -1}
-        }
+          direction: { x: 0, y: -1 },
+        },
       },
-      log: "",
+      log: '',
       turnCount: 1,
     }
     const userCode = `
@@ -56,12 +56,12 @@ def next_turn(world_state, avatar_state):
     
     return action
 `
-    const gameState = {worksheetID: 1}
+    const gameState = { worksheetID: 1 }
     const playerAvatarId = 1
 
     const result = checkIfBadgeEarned(badges, turnResult, userCode, gameState, playerAvatarId)
 
-    const expected = "1:1,1:2,"
+    const expected = '1:1,1:2,'
 
     expect(result).toBe(expected)
   })
@@ -70,12 +70,12 @@ def next_turn(world_state, avatar_state):
     const badges = ''
     const turnResult = {
       action: {
-        action_type: "move",
+        action_type: 'move',
         options: {
-          direction: {x: 0, y: -1}
-        }
+          direction: { x: 0, y: -1 },
+        },
       },
-      log: "",
+      log: '',
       turnCount: 1,
     }
     const userCode = `
@@ -108,44 +108,16 @@ def next_turn(world_state, avatar_state):
       players: [
         {
           id: 1,
-          location: {x: 10, y: 10}
-        }
+          location: { x: 10, y: 10 },
+        },
       ],
       obstacles: [],
     }
     const playerAvatarId = 1
 
     const result = checkIfBadgeEarned(badges, turnResult, userCode, gameState, playerAvatarId)
 
-    const expected = "1:1,1:2,1:3,"
-
-    expect(result).toBe(expected)
-  })
-
-  it('filters badges', () => {
-    const badges = '1:1,1:2,1:3,2:1,2:2,3:1'
-
-    let gameState = {worksheetID: 1}
-
-    let result = filterByWorksheet(badges, gameState)
-
-    let expected = "1:1,1:2,1:3"
-
-    expect(result).toBe(expected)
-
-    gameState = {worksheetID: 2}
-
-    result = filterByWorksheet(badges, gameState)
-
-    expected = "2:1,2:2"
-
-    expect(result).toBe(expected)
-
-    gameState = {worksheetID: 3}
-
-    result = filterByWorksheet(badges, gameState)
-
-    expected = "3:1"
+    const expected = '1:1,1:2,1:3,'
 
     expect(result).toBe(expected)
   })

diff --git a/game_frontend/src/pyodide/badges.ts b/game_frontend/src/pyodide/badges.ts
@@ -1,25 +1,11 @@
 /* eslint-env worker */
 import ComputedTurnResult from './computedTurnResult'
 
-export function filterByWorksheet(badges: any, gameState: any): string {
-  const worksheetID = gameState.worksheetID
-  let badgesArr = badges.split(',')
-
-  badgesArr = badgesArr.filter((s) => s) // remove empty element
-  // remove any badge that's not relevant to the current worksheet
-  badgesArr = badgesArr.filter((b) => {
-    return b.startsWith(worksheetID + ':')
-  })
-
-  return badgesArr.join(',')
-}
-
 export function checkIfBadgeEarned(
   badges: string,
   result: ComputedTurnResult,
   userCode: string,
-  gameState: any,
-  playerAvatarId: number
+  gameState: any
 ): string {
   const userPythonCode = userCode.replace(/\s*#.*/gm, '') // Remove all comment lines from the user's code
   const badgesPerWorksheet = [
@@ -28,7 +14,7 @@ export function checkIfBadgeEarned(
     {
       id: 3,
       worksheetID: 1,
-      trigger: badge3Trigger(result, userPythonCode, gameState, playerAvatarId),
+      trigger: badge3Trigger(result, userPythonCode),
     },
   ]
 
@@ -40,7 +26,7 @@ export function checkIfBadgeEarned(
       badge.trigger
     ) {
       // Here is when a new badge is earned
-      // TODO on worksshet 2: This might have to order the badges, in case user does not do the worksheet in order
+      // TODO on worksheet 2: This might have to order the badges, in case user does not do the worksheet in order
       badges += `${badgeWorksheetPair},`
     }
   }
@@ -71,21 +57,11 @@ function badge2Trigger(userPythonCode: string): boolean {
   return substrings.every((substring) => userPythonCode.includes(substring))
 }
 
-function badge3Trigger(
-  result: any,
-  userPythonCode: string,
-  gameState: any,
-  playerAvatarId: number
-): boolean {
+function badge3Trigger(result: any, userPythonCode: string): boolean {
   // Check the code contains certain keywords about moving to a cell
   const substrings = ['world_state.can_move_to(', 'print(', 'if ']
   const codeContainsKeywords = substrings.every((substring) => userPythonCode.includes(substring))
 
-  if (!codeContainsKeywords) return false
-
-  // Check action is move action
-  const isMoveAction = result.action.action_type === 'move'
-  if (!isMoveAction) return false
-
-  return true
+  // And check it returns a move action
+  return codeContainsKeywords && result.action.action_type === 'move'
 }
diff --git a/game_frontend/src/pyodide/pyodideRunner.ts b/game_frontend/src/pyodide/pyodideRunner.ts
@@ -16,18 +16,13 @@ async function initializePyodideWorker() {
   await worker.initializePyodide()
 }
 
-export async function filterByWorksheet(badges: string, gameState: any): Promise<string> {
-  return worker.filterByWorksheet(badges, gameState)
-}
-
 export async function checkIfBadgeEarned(
   badges: string,
   result: ComputedTurnResult,
   userCode: string,
-  gameState: any,
-  playerAvatarId: number
+  gameState: any
 ): Promise<string> {
-  return worker.checkIfBadgeEarned(badges, result, userCode, gameState, playerAvatarId)
+  return worker.checkIfBadgeEarned(badges, result, userCode, gameState)
 }
 
 export async function updateAvatarCode(

diff --git a/game_frontend/src/redux/features/AvatarWorker/actions.js b/game_frontend/src/redux/features/AvatarWorker/actions.js
@@ -26,11 +26,6 @@ const getBadgesRequest = () => ({
   type: types.GET_BADGES_REQUEST,
 })
 
-const filterBadges = (badges) => ({
-  type: types.FILTER_BADGES,
-  payload: badges,
-})
-
 const getBadgesReceived = (badges) => ({
   type: types.GET_BADGES_SUCCESS,
   payload: badges,
@@ -61,7 +56,6 @@ export default {
   avatarCodeUpdated,
   avatarsNextActionComputed,
   badgesEarned,
-  filterBadges,
   getBadgesRequest,
   getBadgesReceived,
   checkBadgesReceived,