GH-7-implement-thief #31

Merged
zumbrseb merged 23 commits from GH-7-implement-thief into dev 2021-12-09 22:39:13 +00:00
zumbrseb commented 2021-12-05 14:56:55 +00:00 (Migrated from github.zhaw.ch)

Implements the thief functionality.
The changes aren't tested yet

Implements the thief functionality. The changes aren't tested yet
costajon (Migrated from github.zhaw.ch) requested changes 2021-12-09 22:08:19 +00:00
costajon (Migrated from github.zhaw.ch) left a comment

The logic seems pretty sound. I couldn't find any issues with it. There a few typos, though. I also added some suggestions for the test documentation.

The logic seems pretty sound. I couldn't find any issues with it. There a few typos, though. I also added some suggestions for the test documentation.
costajon (Migrated from github.zhaw.ch) commented 2021-12-09 21:31:48 +00:00
     * Steals a random resource from a random player that has a settlement (or

```suggestion * Steals a random resource from a random player that has a settlement (or ```
costajon (Migrated from github.zhaw.ch) commented 2021-12-09 21:33:23 +00:00
    private void stealRandomResource(Point field) {

Note: Remember to update all references to this function.

```suggestion private void stealRandomResource(Point field) { ``` Note: Remember to update all references to this function.
costajon (Migrated from github.zhaw.ch) commented 2021-12-09 21:34:15 +00:00
        List<Player> potentialStealingTargets = board.getPlayersAroundField(field);

Note: Remember to update all references to this variable.

```suggestion List<Player> potentialStealingTargets = board.getPlayersAroundField(field); ``` Note: Remember to update all references to this variable.
costajon (Migrated from github.zhaw.ch) commented 2021-12-09 21:35:25 +00:00

playerResources would be more concise in my opinion.

`playerResources` would be more concise in my opinion.
costajon (Migrated from github.zhaw.ch) commented 2021-12-09 21:44:56 +00:00
        return "A seven was rolled. Choose carefully where the mighty thief should move next!";
```suggestion return "A seven was rolled. Choose carefully where the mighty thief should move next!"; ```
@ -21,0 +90,4 @@
// switch players so red doesn't steel from himself
model.switchToNextPlayer();
// do the actuall stealing
costajon (Migrated from github.zhaw.ch) commented 2021-12-09 21:50:21 +00:00
        // make sure, that red only has bricks, so that the thief has to steal bricks

```suggestion // make sure, that red only has bricks, so that the thief has to steal bricks ```
@ -21,0 +94,4 @@
assertTrue(model.placeThiefAndStealCard(new Point(5, 5)));
// the thief should steal one brick as this is
checkAllInventoriesEmptyExcept(model, null);
}
costajon (Migrated from github.zhaw.ch) commented 2021-12-09 21:51:49 +00:00
        // switch players so red doesn't steal from himself

```suggestion // switch players so red doesn't steal from himself ```
@ -21,0 +96,4 @@
checkAllInventoriesEmptyExcept(model, null);
}
/**
costajon (Migrated from github.zhaw.ch) commented 2021-12-09 21:52:00 +00:00
        // do the actual stealing

```suggestion // do the actual stealing ```
@ -21,0 +135,4 @@
*
* @param model the siedler game
* @param nonEmptyPlayer the player with a non empty inventory or null if no
* player should have an empty inventory
costajon (Migrated from github.zhaw.ch) commented 2021-12-09 21:52:42 +00:00
        // All inventories should still be empty, as the thief prevents the payout

```suggestion // All inventories should still be empty, as the thief prevents the payout ```
costajon (Migrated from github.zhaw.ch) commented 2021-12-09 21:56:44 +00:00
  3. For fields that are water, false should be returned and the thief shouldn't be moved
```suggestion 3. For fields that are water, false should be returned and the thief shouldn't be moved ```
costajon (Migrated from github.zhaw.ch) commented 2021-12-09 21:58:41 +00:00
  4. The thief can only be moved if the target field is valid and not water
```suggestion 4. The thief can only be moved if the target field is valid and not water ```
costajon (Migrated from github.zhaw.ch) commented 2021-12-09 22:01:49 +00:00
Tests if a random card from a player around the field where the thief was placed is stolen. To test this, only the red player has one brick in its inventory. All other players have their inventory cleared. This ensures that the thief has to steal from the red player.
```suggestion Tests if a random card from a player around the field where the thief was placed is stolen. To test this, only the red player has one brick in its inventory. All other players have their inventory cleared. This ensures that the thief has to steal from the red player. ```
costajon (Migrated from github.zhaw.ch) commented 2021-12-09 22:03:37 +00:00
Tests if the thief steals nothing when all nearby players have empty inventories. For this, all inventories are cleared beforehand.
```suggestion Tests if the thief steals nothing when all nearby players have empty inventories. For this, all inventories are cleared beforehand. ```
costajon (Migrated from github.zhaw.ch) commented 2021-12-09 22:07:27 +00:00
Tests that fields don't trigger a payout when a thief is on them. To check this, all inventories cleared first. If all inventories are empty after the test, the test was successful.```
```suggestion Tests that fields don't trigger a payout when a thief is on them. To check this, all inventories cleared first. If all inventories are empty after the test, the test was successful.```
@ -0,0 +1,50 @@
# Test Cases
This document provides a brief explanation of the testcases.
costajon (Migrated from github.zhaw.ch) commented 2021-12-09 21:53:49 +00:00
This document provides a brief explanation of the test cases.  
```suggestion This document provides a brief explanation of the test cases. ```
@ -0,0 +1,50 @@
# Test Cases
This document provides a brief explanation of the testcases.
All tests are supposed to run through with the expected result successfully.
costajon (Migrated from github.zhaw.ch) commented 2021-12-09 21:54:31 +00:00
All tests are supposed to run successfully.
```suggestion All tests are supposed to run successfully. ```
@ -0,0 +5,4 @@
## `SiedlerGameTest`
### `testPlacingThief`
This test tests if the thief can be moved. This testcase doesn't test if a card is stolen, see testStealRandomCardWhenPlacingThief() for this.
costajon (Migrated from github.zhaw.ch) commented 2021-12-09 21:55:22 +00:00
This test tests if the thief can be moved. It doesn't test if a card is stolen, see `testStealRandomCardWhenPlacingThief()` for this.
```suggestion This test tests if the thief can be moved. It doesn't test if a card is stolen, see `testStealRandomCardWhenPlacingThief()` for this. ```
@ -0,0 +8,4 @@
This test tests if the thief can be moved. This testcase doesn't test if a card is stolen, see testStealRandomCardWhenPlacingThief() for this.
#### Equivalence Classes
1. For out of bounds coordinate, false should be returned and the thief shouldn't be moved
costajon (Migrated from github.zhaw.ch) commented 2021-12-09 21:55:43 +00:00
  1. For out of bounds coordinates, false should be returned and the thief shouldn't be moved
```suggestion 1. For out of bounds coordinates, false should be returned and the thief shouldn't be moved ```
@ -0,0 +9,4 @@
#### Equivalence Classes
1. For out of bounds coordinate, false should be returned and the thief shouldn't be moved
2. For position, which aren't fields, false should be returned and the thief shouldn't be moved
costajon (Migrated from github.zhaw.ch) commented 2021-12-09 21:56:04 +00:00
  2. For positions, that aren't fields, false should be returned and the thief shouldn't be moved
```suggestion 2. For positions, that aren't fields, false should be returned and the thief shouldn't be moved ```
costajon (Migrated from github.zhaw.ch) approved these changes 2021-12-09 22:38:30 +00:00
thalmma5 (Migrated from github.zhaw.ch) requested changes 2021-12-09 22:49:46 +00:00
thalmma5 (Migrated from github.zhaw.ch) left a comment

Awesome job!
However - I have a few changes to ask for

Awesome job! However - I have a few changes to ask for
thalmma5 (Migrated from github.zhaw.ch) commented 2021-12-09 22:39:32 +00:00

Fix typo

Fix typo
@ -149,4 +149,5 @@ public class SiedlerBoardTextView extends HexBoardTextView<SiedlerField, Siedler
}
return label;
}
thalmma5 (Migrated from github.zhaw.ch) commented 2021-12-09 22:41:01 +00:00

Unnecessary blank line

Unnecessary blank line
thalmma5 (Migrated from github.zhaw.ch) commented 2021-12-09 22:42:04 +00:00

Unnecessary blank line

Unnecessary blank line
thalmma5 (Migrated from github.zhaw.ch) commented 2021-12-09 22:44:29 +00:00

In my understanding, you should get one of the potential target candidates rather than any random player (I might be wrong, tho)

In my understanding, you should get one of the potential target candidates rather than any random player (I might be wrong, tho)
thalmma5 (Migrated from github.zhaw.ch) commented 2021-12-09 22:45:01 +00:00

(If I'm correct) you should accept a parameter where you can pass the potential target candidates

(If I'm correct) you should accept a parameter where you can pass the potential target candidates
@ -56,0 +60,4 @@
public void executeUntilSuccessful() {
while (!execute()) {
}
}
thalmma5 (Migrated from github.zhaw.ch) commented 2021-12-09 22:45:18 +00:00

Awesome job, I like this solution!

Awesome job, I like this solution!✨
@ -21,0 +104,4 @@
void testPaymentOfFieldWithThief() {
SiedlerGame model = ThreePlayerStandard.getAfterSetupPhase(4);
clearAllInventories(model);
// place thief on 5,5
thalmma5 (Migrated from github.zhaw.ch) commented 2021-12-09 22:48:31 +00:00
     * Tests whether no error would occur if non of the player had anything in

```suggestion * Tests whether no error would occur if non of the player had anything in ```
Sign in to join this conversation.
No description provided.