Simulator CLI seems to ignore first player code in list and also fails with 5 or longer (thus preventing a 4 player workaround)

[ptudan]

If I try to run catanatron-play -players=VP,AB --num=2 -o sims --csv I get the following errors, which I believe are due to the cli only picking up one player code:

(venv) paul@DESKTOP-65EVJAG:~/environments/catanatron$ catanatron-play -players=VP,AB --num=2 -o sims --csv
Playing 2 games...                                            ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━   0% -:--:--
AlphaBetaPlayer:BLUE(depth=2,value_fn=base_fn,prunning=False) ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━   0%
Traceback (most recent call last):
  File "/home/paul/environments/catanatron/venv/bin/catanatron-play", line 11, in <module>
    load_entry_point('catanatron-experimental', 'console_scripts', 'catanatron-play')()
  File "/home/paul/environments/catanatron/venv/lib/python3.8/site-packages/click/core.py", line 1128, in __call__
    return self.main(*args, **kwargs)
  File "/home/paul/environments/catanatron/venv/lib/python3.8/site-packages/click/core.py", line 1053, in main
    rv = self.invoke(ctx)
  File "/home/paul/environments/catanatron/venv/lib/python3.8/site-packages/click/core.py", line 1395, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/home/paul/environments/catanatron/venv/lib/python3.8/site-packages/click/core.py", line 754, in invoke
    return __callback(*args, **kwargs)
  File "/home/paul/environments/catanatron/catanatron_experimental/catanatron_experimental/play.py", line 184, in simulate
    play_batch(
  File "/home/paul/environments/catanatron/catanatron_experimental/catanatron_experimental/play.py", line 314, in play_batch
    for i, game in enumerate(
  File "/home/paul/environments/catanatron/catanatron_experimental/catanatron_experimental/play.py", line 245, in play_batch_core
    game.play(accumulators)
  File "/home/paul/environments/catanatron/catanatron_core/catanatron/game.py", line 103, in play
    self.play_tick(decide_fn=decide_fn, accumulators=accumulators)
  File "/home/paul/environments/catanatron/catanatron_core/catanatron/game.py", line 124, in play_tick
    else player.decide(self, actions)
  File "/home/paul/environments/catanatron/catanatron_experimental/catanatron_experimental/machine_learning/players/minimax.py", line 70, in decide
    result = self.alphabeta(
  File "/home/paul/environments/catanatron/catanatron_experimental/catanatron_experimental/machine_learning/players/minimax.py", line 121, in alphabeta
    result = self.alphabeta(
  File "/home/paul/environments/catanatron/catanatron_experimental/catanatron_experimental/machine_learning/players/minimax.py", line 121, in alphabeta
    result = self.alphabeta(
  File "/home/paul/environments/catanatron/catanatron_experimental/catanatron_experimental/machine_learning/players/minimax.py", line 100, in alphabeta
    value = value_fn(game, self.color)
  File "/home/paul/environments/catanatron/catanatron_experimental/catanatron_experimental/machine_learning/players/value.py", line 63, in fn
    enemy_production = value_production(enemy_production_sample, "P1", False)
  File "/home/paul/environments/catanatron/catanatron_experimental/catanatron_experimental/machine_learning/players/value.py", line 133, in value_production
    prod_sum = sum([sample[f] for f in features])
  File "/home/paul/environments/catanatron/catanatron_experimental/catanatron_experimental/machine_learning/players/value.py", line 133, in <listcomp>
    prod_sum = sum([sample[f] for f in features])
KeyError: 'EFFECTIVE_P1_WHEAT_PRODUCTION'

If I try to run with 4 players, I only get 3 (and with 3, I get 2).

If I try to run with 5 players, I also get an out of bounds error which I assume is expected.

[ptudan]

I have a fix for this, but I can't seem to push a branch to open a pull request. Would be great if I could get that permission

It seems like the cli arg parser is shortening the players flag to -p when I didn't use two dashes, but then passing in the rest of the string as an arg (despite there not being a space). I get that this is sort of a user error, but I feel like its worth fixing anyway. I'd say either merge this fix in, or change the arg parser so that -players fails rather than fallsback to -p

I just added this in play.py simulate method

    if len(player_keys) > 0:
        player_keys[0] = player_keys[0].replace("layers=", "")

[pachewise]

Hi @ptudan, are you able to fork the repo and open a PR against this repo from your fork?

On the specific issue, I believe the click library is parsing that command as "-p layers=VB, AB", hence that "first" player is not being parsed correctly and is dropped. I think the right approach to fixing this is: (1) remove the -p alias in favor of always using --players (as opposed to trying to handle when people use the wrong number of dashes); (2) change the for loop in simulate() (see below)

https://github.com/bcollazo/catanatron/blob/4b05b1cb7053d6eee5c8ed6e7b7504e166301833/catanatron_experimental/catanatron_experimental/play.py#L172-L180

It is better for the person running the CLI to know that they mistyped a player key, rather than have that key skipped altogether.

[bcollazo]

Hey @ptudan, thanks for opening up the issue and taking a stab at this!

I like @pachewise's (1) solution. Let's go with that. That gives a user error the one attached, and I like --players=VB,AB being more explicit / clear than something like -pVB,AB anyways. So dropping support for -p! 👍

Cooking a PR now.

[bcollazo]

Addressed with #209.