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

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)

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

from catanatron.

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=", "")

from catanatron.

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.

from catanatron.

Addressed with #209.

from catanatron.