[BLB] Implement Season of Gathering and Pawprints mechanic #12617

jimga150 · 2024-07-29T03:00:58Z

#11853
My current implementation requires pawprint cards to also set a min and max number of modes according to what logically could be chosen (0 and 5, for this and what will be the other four bloomburrow "Season of" cards).
In playtesting, the maxModes being set to 5 isnt required, since i added switches to check against the pawprint count instead of the mode count, but for some reason if i don't set the maxModes to 5 (default is 1), my tests all fail like this:

[ERROR] Failures: 
[ERROR]   PawPrintsTest.test_Choose113:36->CardTestPlayerAPIImpl.execute:296->CardTestPlayerAPIImpl.assertAllCommandsUsed:1599->CardTestPlayerAPIImpl.assertTargetsCount:1585 (Targets of PlayerA) Count are not equal (found [Memnite]). Some inner choose dialogs can be set up only in strict mode. expected:<0> but was:<1>
[ERROR]   PawPrintsTest.test_Choose122:212->CardTestPlayerAPIImpl.execute:296->CardTestPlayerAPIImpl.assertAllCommandsUsed:1598->CardTestPlayerAPIImpl.assertChoicesCount:1576 (Choices of PlayerA) Count are not equal (found [Artifact, Enchantment]). Some inner choose dialogs can be set up only in strict mode. expected:<0> but was:<2>
[ERROR]   PawPrintsTest.test_Choose123:70->CardTestPlayerAPIImpl.execute:296->CardTestPlayerAPIImpl.assertAllCommandsUsed:1598->CardTestPlayerAPIImpl.assertChoicesCount:1576 (Choices of PlayerA) Count are not equal (found [Enchantment]). Some inner choose dialogs can be set up only in strict mode. expected:<0> but was:<1>
[ERROR]   PawPrintsTest.test_Choose1x5:146->CardTestPlayerAPIImpl.execute:296->CardTestPlayerAPIImpl.assertAllCommandsUsed:1599->CardTestPlayerAPIImpl.assertTargetsCount:1585 (Targets of PlayerA) Count are not equal (found [Memnite, Memnite, Memnite, Memnite]). Some inner choose dialogs can be set up only in strict mode. expected:<0> but was:<4>
[ERROR]   PawPrintsTest.test_Choose2111:113->CardTestPlayerAPIImpl.execute:296->CardTestPlayerAPIImpl.assertAllCommandsUsed:1599->CardTestPlayerAPIImpl.assertTargetsCount:1585 (Targets of PlayerA) Count are not equal (found [Memnite, Memnite, Memnite]). Some inner choose dialogs can be set up only in strict mode. expected:<0> but was:<3>
[ERROR]   PawPrintsTest.test_Choose23:176->CardTestPlayerAPIImpl.execute:296->CardTestPlayerAPIImpl.assertAllCommandsUsed:1598->CardTestPlayerAPIImpl.assertChoicesCount:1576 (Choices of PlayerA) Count are not equal (found [Artifact]). Some inner choose dialogs can be set up only in strict mode. expected:<0> but was:<1>
[INFO] 
[ERROR] Tests run: 6, Failures: 6, Errors: 0, Skipped: 0

Which suggests that for some reason, the choice/target ingesting code is getting clogged. I'm not sure why, since i haven't had any luck trying to use breakpoints in maven run configurations.

xenohedron

Overall this looks good to me! I'll let another dev take a look before merging. Thanks for the test cases.

xenohedron · 2024-07-30T03:43:40Z

Mage/src/main/java/mage/abilities/Mode.java

@@ -19,6 +19,7 @@ public class Mode implements Serializable {
    protected final Effects effects;
    protected String flavorWord;
    protected Cost cost = null;
+    protected int pawPrintValue = -1;


please add comment on usage of "magic number"

xenohedron · 2024-07-30T03:45:07Z

Mage/src/main/java/mage/abilities/Mode.java

@@ -119,4 +121,17 @@ public Mode withCost(Cost cost) {
    public Cost getCost() {
        return cost;
    }
+
+    public void setPawPrintValue(int pawPrintValue) {


what's the point of having separate "set" and "with" methods? I would just have the "with" method, the return can be ignored if not needed

xenohedron · 2024-07-30T03:45:37Z

Mage/src/main/java/mage/abilities/Modes.java

@@ -53,6 +55,7 @@ public Modes() {
        this.put(currentMode.getId(), currentMode);
        this.minModes = 1;
        this.maxModes = 1;
+        this.maxPawPrints = -1;


comment here as well

-1 logic smells bed. Must be 0 by default. And other code checks must use > 0.

xenohedron · 2024-07-30T03:47:00Z

Mage/src/main/java/mage/abilities/Modes.java

+        Mode mode = get(modeId);
+
+        if (mode.getPawPrintValue() < 0 && this.getMaxPawPrints() >= 0){
+            throw new RuntimeException("Selected mode has no pawprint value, but this Modes object requires pawprint modes!");


don't throw RuntimeException, use something like IllegalStateException

Looks like it’s verify check and can be added to card constructor time instead runtime. As example: modes.addMode - after new mode added make sure modes amount with paw equal to total modes amount or zero.

For info:

verify check -- can be called from card constructor (by card, ability, effect or other data setup). There are some verify tests that call constructor for all cards and tokens. So such errors will be catches by tests as fast as possible.

runtime check -- can be called from real game only.

To clarify, are you asking for both here? I moved the runtime check pointed to in this thread to a check in addMode() to catch it earlier.

Mage/src/main/java/mage/abilities/Modes.java

xenohedron · 2024-07-30T03:49:02Z

Mage/src/main/java/mage/abilities/dynamicvalue/common/ControlledCreaturesGreatestPower.java

+/**
+ * @author jimga150
+ */
+public enum ControlledCreaturesGreatestPower implements DynamicValue {


GreatestPowerAmongControlledCreaturesValue already exists

(and is a good example of stream, mapToInt, sum as a cleaner style)

Mage.Sets/src/mage/cards/s/SeasonOfGathering.java

JayDi85

Require some code style and check logic changes. See related comments.

JayDi85 · 2024-07-30T05:31:41Z

Mage.Server.Plugins/Mage.Player.AI/src/main/java/mage/player/ai/ComputerPlayer.java

@@ -2217,6 +2217,10 @@ public Mode chooseMode(Modes modes, Ability source, Game game) {
                    continue AvailableMode;
                }
            }
+            if (modes.getMaxPawPrints() >= 0 && modes.getSelectedPawPrints() + mode.getPawPrintValue() > modes.getMaxPawPrints()){


must put paw checks inside getAvailableModes, not outside.

chooseMode is where the max modes check is, so it seemed symmetrical to place the max pawprints check here too. should the max modes check move to getAvailableModes too?

Try to add paw check to getMaxMode. getAvailableModes must use paw check too, not getMaxMode.

JayDi85 · 2024-07-30T05:32:13Z

Mage.Server.Plugins/Mage.Player.Human/src/mage/player/human/HumanPlayer.java

@@ -2572,6 +2572,11 @@ public Mode chooseMode(Modes modes, Ability source, Game game) {
                    }
                }

+                if (modes.getMaxPawPrints() >= 0 && modes.getSelectedPawPrints() + mode.getPawPrintValue() > modes.getMaxPawPrints()){


Linking for my reference #12617 (comment)

Mage.Server.Plugins/Mage.Player.Human/src/mage/player/human/HumanPlayer.java

JayDi85 · 2024-07-30T05:42:22Z

Mage.Tests/src/test/java/org/mage/test/player/TestPlayer.java

@@ -2082,6 +2082,10 @@ public Mode chooseMode(Modes modes, Ability source, Game game) {
            int needMode = Integer.parseInt(modesSet.get(0));
            int i = 1;
            for (Mode mode : modes.getAvailableModes(source, game)) {
+                if (modes.getMaxPawPrints() >= 0 && modes.getSelectedPawPrints() + mode.getPawPrintValue() > modes.getMaxPawPrints()){


Linking for my reference #12617 (comment)

JayDi85 · 2024-07-30T05:43:54Z

Mage/src/main/java/mage/abilities/Modes.java

@@ -32,9 +32,11 @@ public class Modes extends LinkedHashMap<UUID, Mode> implements Copyable<Modes>
    private final List<UUID> selectedModes = new ArrayList<>(); // all selected modes (this + duplicate), use getSelectedModes all the time to keep modes order
    private final Map<UUID, Mode> selectedDuplicateModes = new LinkedHashMap<>(); // for 2x selects: additional selected modes
    private final Map<UUID, UUID> selectedDuplicateToOriginalModeRefs = new LinkedHashMap<>(); // for 2x selects: stores ref from duplicate to original mode
+    private int selectedPawPrints = 0;


It’s dynamic value from selected modes, no need to store it inside.

JayDi85 · 2024-07-30T05:46:11Z

Mage/src/main/java/mage/abilities/Modes.java

@@ -53,6 +55,7 @@ public Modes() {
        this.put(currentMode.getId(), currentMode);
        this.minModes = 1;
        this.maxModes = 1;
+        this.maxPawPrints = -1;


-1 logic smells bed. Must be 0 by default. And other code checks must use > 0.

JayDi85 · 2024-07-30T05:55:31Z

Mage/src/main/java/mage/abilities/Modes.java

+        Mode mode = get(modeId);
+
+        if (mode.getPawPrintValue() < 0 && this.getMaxPawPrints() >= 0){
+            throw new RuntimeException("Selected mode has no pawprint value, but this Modes object requires pawprint modes!");


Looks like it’s verify check and can be added to card constructor time instead runtime. As example: modes.addMode - after new mode added make sure modes amount with paw equal to total modes amount or zero.

…n to addMode

jimga150 · 2024-08-11T14:33:28Z

Is there a way to retrigger travis builds? this one looks like it was a transient error

xenohedron

minor followup correction needed, otherwise I think this is ready to go

xenohedron · 2024-08-13T02:55:42Z

Mage.Server.Plugins/Mage.Player.Human/src/mage/player/human/HumanPlayer.java

-            String message = "Choose mode (selected " + modes.getSelectedModes().size() + " of " + modes.getMaxModes(game, source)
-                    + ", min " + modes.getMinModes() + ")";
+            String message;
+            if (modes.getMaxPawPrints() < 0){


should this be modes.getMaxPawPrints() == 0 now?

yeah, missed this and another check when converting the pattern to 0 -> non-pawprints

xenohedron · 2024-08-13T02:58:08Z

Mage.Server.Plugins/Mage.Player.Human/src/mage/player/human/HumanPlayer.java

                }
            }

            // done button for "for up" choices only
-            boolean canEndChoice = modes.getSelectedModes().size() >= modes.getMinModes() || modes.isMayChooseNone();
+            boolean canEndChoice = (modes.getSelectedModes().size() >= modes.getMinModes() && modes.getMaxPawPrints() < 0) ||
+                    (modes.getSelectedPawPrints() >= modes.getMaxPawPrints() && modes.getMaxPawPrints() >= 0) ||


I think this needs to be updated now that max paw prints = 0 indicates no pawprints

jimga150 added 5 commits July 27, 2024 16:18

Add skeleton

7a6253b

Implement Pawprints modal functionality

771a223

Implement Seasons of Gathering

caa7320

remove unused imports

bb89a47

Add Pawprints test

381b3e7

github-actions bot added server cards engine tests labels Jul 29, 2024

xenohedron reviewed Jul 30, 2024

View reviewed changes

JayDi85 reviewed Jul 30, 2024

View reviewed changes

Mage.Sets/src/mage/cards/s/SeasonOfGathering.java Show resolved Hide resolved

xenohedron mentioned this pull request Jul 30, 2024

BLB - Bloomburrow Set Card Implementation Tracking #11853

Open

JayDi85 reviewed Jul 30, 2024

View reviewed changes

jimga150 added 6 commits July 30, 2024 20:08

use withPawPRintValue() instead of setter

84490db

use 0 for non-pawprint mode and modes classes and move mode validatio…

2025f9f

…n to addMode

Use GreatestPowerAmongControlledCreaturesValue

876aa38

Fix pawprints check

7cce01b

calcualte sleected pawprint count based on selected modes

0f72b5a

move max pawprints check to getAvailableModes

5d9b59d

xenohedron reviewed Aug 13, 2024

View reviewed changes

fix max pawprints checks

f656f46

xenohedron approved these changes Aug 15, 2024

View reviewed changes

xenohedron merged commit e976920 into magefree:master Aug 15, 2024
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BLB] Implement Season of Gathering and Pawprints mechanic #12617

[BLB] Implement Season of Gathering and Pawprints mechanic #12617

jimga150 commented Jul 29, 2024

xenohedron left a comment

xenohedron Jul 30, 2024

xenohedron Jul 30, 2024

xenohedron Jul 30, 2024

JayDi85 Jul 30, 2024

xenohedron Jul 30, 2024

JayDi85 Jul 30, 2024 •

edited

Loading

JayDi85 Jul 31, 2024

jimga150 Jul 31, 2024 •

edited

Loading

xenohedron Jul 30, 2024

JayDi85 left a comment

JayDi85 Jul 30, 2024

jimga150 Jul 31, 2024

JayDi85 Jul 31, 2024

JayDi85 Jul 30, 2024

jimga150 Jul 31, 2024

JayDi85 Jul 30, 2024

jimga150 Jul 31, 2024

JayDi85 Jul 30, 2024

JayDi85 Jul 30, 2024

JayDi85 Jul 30, 2024 •

edited

Loading

jimga150 commented Aug 11, 2024

xenohedron left a comment

xenohedron Aug 13, 2024

jimga150 Aug 14, 2024

xenohedron Aug 13, 2024

[BLB] Implement Season of Gathering and Pawprints mechanic #12617

[BLB] Implement Season of Gathering and Pawprints mechanic #12617

Conversation

jimga150 commented Jul 29, 2024

xenohedron left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JayDi85 Jul 30, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jimga150 Jul 31, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JayDi85 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JayDi85 Jul 30, 2024 • edited Loading

Choose a reason for hiding this comment

jimga150 commented Aug 11, 2024

xenohedron left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JayDi85 Jul 30, 2024 •

edited

Loading

jimga150 Jul 31, 2024 •

edited

Loading

JayDi85 Jul 30, 2024 •

edited

Loading