Repository: cuttle-cards/cuttle (js)

Issue:
[Feature]: Implement Scuttling in the GameState API
### Feature Summary

We need to implement Scuttling in the GameState API in order to support the full game logic in this API. See the [spec](https://docs.google.com/document/d/1vYe4xoVULIbbFU2yhErok-Qm1cg8ujMGNqlUOgNXLwE/edit#heading=h.wpeuh0qc6ps2) for a general overview of the API, its underlying motivations, and the development approach.

Scuttling means moving the `playedCard` from the player's `points` to the `scrap` and the `targetCard` from the opponent's `points` to the `scrap`.

### Detailed Description

Implementation should take the general approach shown in the [Backend Control Flow section of the GameState API Spec](https://docs.google.com/document/d/1vYe4xoVULIbbFU2yhErok-Qm1cg8ujMGNqlUOgNXLwE/edit#heading=h.vgf103gono5r) and follow the example of #1025 .

Making this move, like all others, will be triggered via the endpoint:
```
POST /api/game/:gameId/move
```
Which move gets made is determined by the request body, which should specify a `moveType` field with a value from the `MoveType` enum. Scuttling a card will be triggered by sending a request body like: 
```
{
  moveType: MoveType.SCUTTLE,
  playedCardId: '7H',
  targetCardId: '5D',
};
```

The endpoint will already exist. When a `moveType: MoveType.SCUTTLE` is sent in the req body, the endpoint will attempt to utilize the `scuttle` helpers: `sails.helpers.gamestate.moves.scuttle.validate()` and `sails.helpers.gamestate.moves.scuttle.execute()`. Each of these should follow the paradigm outlined in the [docs](https://docs.google.com/document/d/1vYe4xoVULIbbFU2yhErok-Qm1cg8ujMGNqlUOgNXLwE/edit#heading=h.vgf103gono5r):

There are four pieces to implement:
1. update the `api/policies/hasValidMoveBody` policy to allow and validate requests with a moveType of `MoveType.SCUTTLE`
2. Implement the `api/helpers/gamestate/moves/scuttle/validate` helper to verify if the move is legal
3. Implement the `api/helpers/gamestate/moves/scuttle/execute` helper to create a new GameState that scuttles the `targetCard` with the `playedCard`
4. Unskip the relevant tests by removing the `cy.skipOnOnGameStateApi()` commands from tests which can now pass in the GameState API

### Updating the `api/policies/hasValidMoveBody` policy
We must also update the policy `api/policies/hasValidMoveBody.js` to accommodate the `SCUTTLE` moveType. This policy is already applied to the endpoint for making a move and it uses a `switch()` statement on `req.body.moveType` to validate that a given req body has all the data necessary for the given `moveType`. We should add a `MoveType.SCUTTLE` case to this policy.

A `moveType`  of `MoveType.SCUTTLE` requires a `playedCardId` and a `targetCardId` which are both valid card ids. If both are present in req.body and both values can be found in `utils/DeckIds.json`, then we can `return res.next()` in this switch case. Otherwise we should return `res.badRequest()` with an error that explains which `req.body` attribute is missing or invalid.

### Implementing `validate()`
Create a new `api/helpers/gamestate/moves/scuttle/validate.js` helper file that matches the general setup of the existing `api/helpers/gamestate/moves/points/validate.js` helper. It should throw errors if the requested move is illegal. It should have three inputs: 
1. `currentState` - the unpacked `GameState` from before the move is made
2. `requestedMove` -req body describing the move being made3.
3. `playedBy` - 0 or 1 for whether p0 or p1 is making the move.
`validate()` should throw if:
* It is not the requesting player's turn
* The GameState's `phase !== GamePhase.MAIN`
* The `playedCard` is not in the player's hand (`"game.snackbar.global.playFromHand"`)
* The `playedCard` is frozen (`game.snackbar.global.cardFrozen`)
* The `targetCard` is not in the opponent's `points` (`game.snackbar.scuttle.mustTargetPointCard`)
* The `targetCard` is bigger than the `playedCard` (compare first by rank then by suit to break tie, so 10 of spades is the biggest number card and it can scuttle any other point card, while the 5 of Hearts can scuttle the 5 of Clubs, 5 of Diamonds, and any Ace - Four, but not the 5 of Spades or any 6+). - (`game.snackbar.scuttle.rankTooLow`)

### Implementing `execute()`
Create a new `api/helpers/gamestate/moves/scuttle/execute.js` helper file that matches the general approach of the existing execute helper `api/helpers/gamestate/moves/points/execute.js`. It should create and return a new `GameState` that is the result of making the `scuttle` move for the requested player i.e. **it should remove `playedCard` from the player's hand, remove the `targetCard` from the opponent's `points`, and move both cards to the scrap. It should also increment the turn and set the phase to `GamePhase.MAIN`. The `playedBy`, `playedCard`, `targetCard` attributes should be included in the returned `GameState`.**

`execute()`  should have 3 inputs:
1. `currentState` - the unpacked `GameState` from before the move is made
2. `requestedMove` -req body describing the move being made
3. `playedBy` - 0 or 1 for whether p0 or p1 is making the move.

### Unskipping Tests
Because we are approaching the `GameState API project with a feature-flag approach, all changes are merged into the `main` branch and the application and tests are set to dynamically run in either the GameState API mode, or the legacy mode, based on the `VITE_USE_GAMESTATE_API` env variable. In CI, we run the full suite of tests against the legacy version of the application and then run a subset of the tests against the GameState API version of the application.

Which tests are run in the GameState API version of CI is controlled via the test command `cy.skipOnGameStateApi()` which programmatically skips a test if the env var `VITE_USE_GAMESTATE_API` is enabled. As we develop new features in the GameState API, we should remove the `cy.skipOnGameStateApi()` command from more and more of the tests.

**Essential tests**
For this PR, at minimum, we should unskip the following tests, which should be able to pass once it is working correctly. All the following tests are in the `tests/e2e/specs/in-game` folder:
* `basicMoves.spec.js` - `it('Scuttles as p0')`

You can run the tests against the GameState API locally by:
 booting the server in gamestate mode with
`npm run start:server:gamestate`

Then booting the client in gamestate mode with
`npm run start:client:gamestate`

running the tests in gamestate mode with
`npm run e2e:client:gamestate`.

Pull request description:
 scuttling in gamestate
<!-- Thanks for contributing to Cuttle! 🎉 -->

## Issue number #947 

Relevant [issue number](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue)
- Resolves #947 

## Please check the following

- [X] Do the tests still pass? (see [Run the Tests](https://github.com/cuttle-cards/cuttle/blob/main/docs/setup-development.md#run-the-tests))
- [X] Is the code formatted properly? (see [Linting (Formatting)](https://github.com/cuttle-cards/cuttle/blob/main/docs/setup-development.md#linting-formatting))
- For New Features:
  - [ ] Have tests been added to cover any new features or fixes?
  - [ ] Has the documentation been updated accordingly?

## Please describe additional details for testing this change


Patch:
diff --git a/api/helpers/gamestate/moves/scuttle/execute.js b/api/helpers/gamestate/moves/scuttle/execute.js
new file mode 100644
index 00000000..0fe808a2
--- /dev/null
+++ b/api/helpers/gamestate/moves/scuttle/execute.js
@@ -0,0 +1,61 @@
+const GamePhase = require('../../../../../utils/GamePhase.json');
+
+module.exports = {
+  friendlyName: 'Scuttle',
+
+  description: 'Returns new GameState resulting from requested scuttle move',
+
+  inputs: {
+    currentState: {
+      type: 'ref',
+      description: 'The latest GameState before the requesting player scuttles',
+      required: true,
+    },
+    /**
+     * @param { Object } requestedMove - Object describing the request to play scuttle
+     * @param { String } requestMove.cardId - Card Played 
+     * @param { String } requestMove.targetId - Card Targeted
+     * @param { MoveType.SCUTTLE } requestedMove.moveType - Specifies that this a scuttle
+     */
+    requestedMove: {
+      type: 'ref',
+      description: 'The move being requested. Specifies moveType, card played and targeted',
+    },
+    playedBy: {
+      type: 'number',
+      description: 'Player number of player requesting move.',
+    },
+  },
+  sync: true, // synchronous helper
+  fn: ({ currentState, requestedMove, playedBy }, exits) => {
+    let result = _.cloneDeep(currentState);
+
+    const { cardId, targetId } = requestedMove;
+
+    const player = playedBy ? result.p1 : result.p0;
+    const opponent = playedBy ? result.p0 : result.p1;
+
+    // remove playedCard from players hand
+    const cardPlayedIndex = player.hand.findIndex(({ id }) => id === cardId);
+    const [ playedCard  ] = player.hand.splice(cardPlayedIndex, 1);
+    
+    // remove target card from oppponent points
+    const targetPlayedIndex = opponent.points.findIndex(({ id }) => id === targetId);
+    const [ targetCard ]= opponent.points.splice(targetPlayedIndex, 1);
+    
+    // move both cards into scrap
+    result.scrap.push(targetCard, playedCard );
+
+    result.turn++;
+
+    result = {
+      ...result,
+      ...requestedMove,
+      phase: GamePhase.MAIN,
+      playedBy,
+      playedCard,
+      targetCard
+    };
+    return exits.success(result);
+  },
+};
diff --git a/api/helpers/gamestate/moves/scuttle/validate.js b/api/helpers/gamestate/moves/scuttle/validate.js
new file mode 100644
index 00000000..91ba0811
--- /dev/null
+++ b/api/helpers/gamestate/moves/scuttle/validate.js
@@ -0,0 +1,76 @@
+const GamePhase = require('../../../../../utils/GamePhase.json');
+
+module.exports = {
+  friendlyName: 'Validate request to scuttle',
+
+  description: 'Verifies whether a request to make scuttle is legal, throwing explanatory error if not.',
+
+  inputs: {
+    currentState: {
+      type: 'ref',
+      descriptions: 'Object containing the current game state',
+      required: true,
+    },
+    /**
+     * @param { Object } requestedMove - Object describing the request to scuttle
+     * @param { String } requestedMove.cardId - Card Played for scuttle
+     * @param { String } requestedMove.targetId - Card Targeted for scuttle
+     * @param { MoveType.SCUTTLE } requestedMove.moveType - Specifies that this a scuttle
+     */
+    requestedMove: {
+      type: 'ref',
+      description: 'Object containing data needed for current move',
+      required: true,
+    },
+    playedBy: {
+      type: 'number',
+      description: 'Player number of player requesting move',
+      required: true,
+    },
+  },
+  sync: true,
+  fn: ({ requestedMove, currentState, playedBy }, exits) => {
+    try {
+      const player = playedBy ? currentState.p1 : currentState.p0;
+      const opponent = playedBy ? currentState.p0 : currentState.p1;
+
+      const playedCard = player.hand.find(({ id }) => id === requestedMove.cardId);
+      const targetCard = opponent.points.find(({ id }) => id === requestedMove.targetId);
+
+      if (currentState.turn % 2 !== playedBy) {
+        throw new Error('game.snackbar.global.notYourTurn');
+      }
+      
+      if (currentState.phase !== GamePhase.MAIN) {
+        throw new Error(`Can only play points in main phase, not ${currentState.phase}`);
+      }
+
+      if (!playedCard) {
+        throw new Error('game.snackbar.global.playFromHand');
+      }
+
+      if (playedCard.isFrozen) {
+        throw new Error('game.snackbar.global.cardFrozen');
+      }
+
+      if (playedCard.rank > 10) {
+        throw new Error('game.snackbar.points.numberOnlyForPoints');
+      }
+
+      if (!targetCard) {
+        throw new Error('game.snackbar.scuttle.mustTargetPointCard');
+      }
+
+      const lowerRank = playedCard.rank < targetCard.rank;
+      const sameRankLowerSuit = playedCard.rank === targetCard.rank && playedCard.suit < targetCard.suit;
+      
+      if (lowerRank || sameRankLowerSuit) {
+        throw new Error('game.snackbar.scuttle.rankTooLow');
+      }
+
+      return exits.success();
+    } catch (err) {
+      return exits.error(err);
+    }
+  },
+};
diff --git a/api/helpers/gamestate/publish-game-state.js b/api/helpers/gamestate/publish-game-state.js
index aae688db..bb73c04e 100644
--- a/api/helpers/gamestate/publish-game-state.js
+++ b/api/helpers/gamestate/publish-game-state.js
@@ -101,7 +101,9 @@ module.exports = {
         playedBy,
         pNum,
         // Conditionally included properties if truthy
-        ...(gameState.resolved && {oneOff: gameState.resolved}),
+        ...(gameState.playedCard && { playedCardId: gameState.playedCard.id }),
+        ...(gameState.targetCard && { targetCardId: gameState.targetCard.id }),
+        ...(gameState.resolved && { oneOff: gameState.resolved }),
         ...(chosenCard && { chosenCard }),
         ...(discardedCards && { discardedCards }),
       };
diff --git a/api/policies/hasValidMoveBody.js b/api/policies/hasValidMoveBody.js
index 33e06444..1ee61ada 100644
--- a/api/policies/hasValidMoveBody.js
+++ b/api/policies/hasValidMoveBody.js
@@ -31,6 +31,20 @@ module.exports = function (req, res, next) {
     }
       return next();
 
+    case MoveType.SCUTTLE: {
+      if (!cardId || !targetId) {
+        return res.badRequest({ message: 'Must specify a card' });
+      }
+
+      const invalidCardId = [cardId, targetId].find(id => !DeckIds.includes(id));
+
+      if (invalidCardId) {
+        return res.badRequest({ message: `${invalidCardId} is not a valid cardId` });
+      }
+      
+    }
+      return next();
+    
     case MoveType.ONE_OFF: {
       if (!cardId) {
         return res.badRequest({ message: 'Must specify a card' });


Test patch:
diff --git a/tests/e2e/specs/in-game/basicMoves.spec.js b/tests/e2e/specs/in-game/basicMoves.spec.js
index 5b020e77..3114a425 100644
--- a/tests/e2e/specs/in-game/basicMoves.spec.js
+++ b/tests/e2e/specs/in-game/basicMoves.spec.js
@@ -52,7 +52,6 @@ describe('Game Basic Moves - P0 Perspective', () => {
   });
 
   it('Scuttles as P0', () => {
-    cy.skipOnGameStateApi();
     // Set Up
     cy.loadGameFixture(0, {
       p0Hand: [Card.ACE_OF_CLUBS, Card.ACE_OF_SPADES, Card.SEVEN_OF_CLUBS],
diff --git a/tests/unit/fixtures/gameStates/points.js b/tests/unit/fixtures/gameStates/points.js
index d4b1cada..06f3f89f 100644
--- a/tests/unit/fixtures/gameStates/points.js
+++ b/tests/unit/fixtures/gameStates/points.js
@@ -75,6 +75,7 @@ export const points = {
     change: MoveType.POINTS,
     happened: true,
     playedBy: 0,
+    playedCardId: 'TS',
     pNum: 0,
     game: {
       players: [
diff --git a/tests/unit/fixtures/gameStates/resolveNine.js b/tests/unit/fixtures/gameStates/resolveNine.js
index d046e2d2..bdda9d3e 100644
--- a/tests/unit/fixtures/gameStates/resolveNine.js
+++ b/tests/unit/fixtures/gameStates/resolveNine.js
@@ -66,6 +66,7 @@ export const resolveNine = {
     oneOff: Card.NINE_OF_CLUBS,
     pNum: 0,
     playedBy: 0,
+    targetCardId: 'JC',
     game: {
       chat: [],
       deck: [],
diff --git a/tests/unit/fixtures/gameStates/resolveThree.js b/tests/unit/fixtures/gameStates/resolveThree.js
index 8a02d921..06518333 100644
--- a/tests/unit/fixtures/gameStates/resolveThree.js
+++ b/tests/unit/fixtures/gameStates/resolveThree.js
@@ -70,6 +70,7 @@ export const resolveThree = {
     oneOff: Card.THREE_OF_SPADES,
     chosenCard: Card.TEN_OF_HEARTS,
     playedBy: 0,
+    targetCardId: 'TH',
     pNum: 0,
     game: {
       players: [