buttonmen-dev / buttonmen

Buttonmen - an online dice game
Other
16 stars 24 forks source link

Re-enabled zero option dice for CustomBM #2961

Closed blackshadowshade closed 2 months ago

blackshadowshade commented 3 months ago

Addresses part of #2772.

The aim of this pull request is to simply re-enable zero option dice. Since we currently have no enabled recipes involving zero option dice, this should not affect the prod site at all.

We already have a low-level test that was added as part of #2641 that checks that an option die sets correctly to zero (i.e., value = 0, max = 0, min = 0).

Thus, the direct effect of this change would be to make zero option dice available for use via CustomBM. If we do move towards adding a recipe with a zero option die, we would follow our standard procedure of testing with RandomAI, as well as adding an appropriate responder test to make sure that the zero option die works correctly.

[Dev site at: https://2773-reenable-custombm-zero-option.blackshadowshade.dev.buttonweavers.com/ui/]

cgolubi1 commented 2 months ago

Okay. Let's get this up on a dev site so people can manually try to make it blow up, and on a replay site so i can programmatically try to make it blow up.

cgolubi1 commented 2 months ago

Sigh, #2969 blocks this. There's a fix to be reviewed at #2970, then i can ask you to rebase and we'll try again.

blackshadowshade commented 2 months ago

Rebase done.

cgolubi1 commented 2 months ago

There's a dev site up at https://2773-reenable-custombm-zero-option.blackshadowshade.dev.buttonweavers.com/ui/ --- check it out.

cgolubi1 commented 2 months ago

I stood up a normal replay site, but that's obviously not going to be very interesting. When i get a second i'll hack it a bit to play a bunch of CustomBM games with zero-sided option dice.

cgolubi1 commented 2 months ago

Alright, i setup some replay testing in a loop where one of the buttons is always CustomBM, CustomBM has a higher chance of putting option on a given die, and if it does put option on a die, there's a 50% chance that the second option will be 0. So far it's successfully run at least one game with the recipe F!(13/0) (10) and one game with gHb(6) mMf(8/0) gHb(6) mMf(8/0) gHb(6) mMf(8/0) gHb(6) mMf(8/0) gHb(6) mMf(8/0). I'll let that run, and see if anything unexpected happens.

cgolubi1 commented 2 months ago
About to execute: sudo -u root sh -c "phpunit --bootstrap /usr/local/etc/buttonmen_phpunit.php --group fulltest_deps /buttonmen/test/src/api/ 2>&1" | tee /srv/bmgames/chaos-test/replay_state/replay-blackshadowshade-2773_reenable_CustomBM_zero_option/local.20240728.042733.output
PHP Parse error:  syntax error, unexpected ''),' (T_CONSTANT_ENCAPSED_STRING), expecting ')' in /buttonmen/test/src/api/responder99Test.php on line 35166

I strongly suspect this has nothing to do with zero-sided option dice, and is just some sort of bug around replaying CustomBM games in general. Well, if we want more responderTests with CustomBM, we're going to have to fix it.

cgolubi1 commented 2 months ago

Ah, here's the failing test. Sigh:

    /**
     * @depends responder00Test::test_request_savePlayerInfo
     * @group fulltest_deps
     */
    public function test_interface_game_00073() {

        // responder003 is the POV player, so if you need to fake
        // login as a different player e.g. to submit an attack, always
        // return to responder003 as soon as you've done so
        $this->game_number = 73;
        $_SESSION = $this->mock_test_user_login('responder003');

        // Expect game creation to fail
        $args = array(
            'type' => 'createGame',
            'playerInfoArray' => array(array('responder003', 'CustomBM'),
                                       array('responder004', 'Jocasta')),
            'maxWins' => '3',
            'customRecipeArray' => array(0 => 's'(2/2)'),
        );
        $this->verify_api_failure($args, 'Custom recipe s'(2/2) is not allowed to contain unimplemented skills.');
    }
blackshadowshade commented 2 months ago

That looks like the correct error. A normal quote is not allowed in a custom recipe because it would be an undefined skill.

blackshadowshade commented 2 months ago

Oh, I see what you're saying. The quote is causing a backend error. Okay, I'll fix that.

cgolubi1 commented 2 months ago

Shadowshade, i'm 100% sure the error's on my end --- i'm not escaping quotes properly in the PHP generated by random_ai for replay. It failed as a replay test, not an original game!

Apologies for pasting it on here and confusing you --- it's not a backend bug, there's nothing to do.

cgolubi1 commented 2 months ago

Okay, this works fine:

    /**
     * @depends responder00Test::test_request_savePlayerInfo
     * @group fulltest_deps
     */
    public function test_interface_game_00073() {

        // responder003 is the POV player, so if you need to fake
        // login as a different player e.g. to submit an attack, always
        // return to responder003 as soon as you've done so
        $this->game_number = 73;
        $_SESSION = $this->mock_test_user_login('responder003');

        // Expect game creation to fail
        $args = array(
            'type' => 'createGame',
            'playerInfoArray' => array(array('responder003', 'CustomBM'),
                                       array('responder004', 'Jocasta')),
            'maxWins' => '3',
            'customRecipeArray' => array(0 => 's\'(2/2)'),
        );
        $this->verify_api_failure($args, 'Custom recipe s\'(2/2) is not allowed to contain unimplemented skills.');
    }

So i could make random_ai's PHP-writer escape single-quotes, and if we ever need to allow quotes in recipes, that's the right thing to do.

But instead i'm going to take the easy way out - i have yet to find any backend bugs by putting quotes in recipes, so i think i'll just skip them for now.

cgolubi1 commented 2 months ago

My takeaway from replay testing is, this seems fine.

Here's the diff i used for testing:

$ git diff tools/
diff --git a/tools/api-client/python/lib/random_ai.py b/tools/api-client/python/lib/random_ai.py
index 401b94d4..218bf403 100755
--- a/tools/api-client/python/lib/random_ai.py
+++ b/tools/api-client/python/lib/random_ai.py
@@ -17,7 +17,9 @@ import string
 #   errors or stalls like any other game.

 VALID_SKILLS = '!#%&+?^`BbcDdFfGgHhIJkMmnopqrstvwz'
-VALID_ISH_SKILLS = string.punctuation + string.ascii_letters
+# * If we ever need to include ' or ", update the replay PHP writer to
+#   escape them properly in both recipe strings and API return messages
+VALID_ISH_SKILLS = (string.punctuation + string.ascii_letters).replace("'", "").replace('"', '')

 VALID_SHORT_SIDES_LIST = range(0, 20) + [30, 'P', 'R', 'S', 'T', 'U', 'V', 'W', 'X', 'Y', 'Z']
 VALID_SIDES_LIST = range(0, 100) + ['P', 'R', 'S', 'T', 'U', 'V', 'W', 'X', 'Y', 'Z']
@@ -48,7 +50,7 @@ def get_side_max(die_sides):
   if die_sides in string.ascii_uppercase: return 0
   assert isinstance(die_sides, int), "Expected %s to be an int" % die_sides

-def get_fuzzy_sides():
+def get_fuzzy_sides(prefer_zeroes=False):
   side_max = 0
   valid_ish = []
   if random.random() < 0.7:
@@ -58,6 +60,8 @@ def get_fuzzy_sides():
   else:
     for x in VALID_ISH_SIDES_LIST: valid_ish.append(x)
   random.shuffle(valid_ish)
+  if prefer_zeroes and random.random() < 0.5:
+    valid_ish.insert(0, 0)
   sides = str(valid_ish[0])
   side_max += get_side_max(valid_ish[0])
   if random.random() < 0.01:
@@ -66,8 +70,8 @@ def get_fuzzy_sides():
     new_sides, new_side_max = get_fuzzy_sides()
     sides += ',' + new_sides
     side_max += new_side_max
-  elif random.random() < 0.1:
-    new_sides, new_side_max = get_fuzzy_sides()
+  elif random.random() < 0.25:
+    new_sides, new_side_max = get_fuzzy_sides(prefer_zeroes=True)
     sides += '/' + new_sides
     side_max = max(side_max, new_side_max)
   return sides, side_max