Proposal: Enable multiple configuration files and overriding from CLI

[AdaptCharm]

The command line and the parameters is very essential for linux users.

647bac8
I'd rather prefer using the command line. A lot of people are confused of the "new way". Please add the old way back and make it default, also add an option inside config.json to enable the release_config.json for those who rather want to use that.

[elicwhite]

As stated in Slack:

It seems like as this bot grows, it is going to be more and more configurable. At a certain point, it doesn't make sense to configure everything through cli args and will have to move to something like json configuration.

If the problem is that for multiple bots, the only real way is via command line args, we can probably find a way to fix that.

I could imagine a cli flag that passed a path to a config file. Would that solve the problem at hand?

Perhaps an approach would be to pass either a cli arg that tells it what config file to use, or there is a command line arg that take a string of the json it should use as config

[AdaptCharm]

@TheSavior Sure, but if people wants to use the parameters, make the parameters (i.e. -c -iv) overwrite the config file.

[curtis2447]

I think having both would just add more functionality, if someone wants to run 3-4 accounts at once using .bat files and parameters it is easier than having 4 separate bots

[elicwhite]

@TheGoldenXY I think that is reasonable to have cli flags overwrite settings in the config file, but how would you handle overwriting deeply nested configuration? For example, if we merged release_config into config and you wanted to overwrite a field like this: https://github.com/TheSavior/PokemonGo-Bot/blob/0d44885fcd8d710e54b718d3468094965801d56d/config.json.example#L18

Perhaps

--release_config.any.release_under_cp 300

Something you might not have context into, is what the later stages of this bot will likely look like. @douglascamata @solderzzc and I have been talking about being more flexible to support the diverse needs of people leading to a proposal like this: #142

At scale, it doesn't make sense to have 100s of cli args and instead needs something more supportive of deeply nested configuration. Perhaps the cli approach that satisfies the needs people have when running multiple bots is to take a command line argument like this:

--config '{"username": "foo"...}'

[AdaptCharm]

@TheSavior Great statement.

How about --release_config.any.cp 200 and --release_config.any.iv 0.4?

[elicwhite]

One of the downsides of that approach is that it may not make sense to support the shorthand cli args anymore so that the configuration can be defined in only one place config.json and it becomes clear in code how they are being overridden. Right now they are all duplicated here: https://github.com/PokemonGoF/PokemonGo-Bot/blob/dev/pokecli.py#L60-L133 What are your thoughts on that?

The result would be that it would be very clear what the command line flags are, because they match the config.json perfectly, but it would mean that the length of the command to run would be longer.

[AdaptCharm]

@TheSavior What do you recommend then?

[elicwhite]

Here is what I propose:

We check in a config.json file by default. It contains the current defaults that are set in pokecli.py

We remove all of the custom command line arguments that currently exist.

We create a new command line flag. --config_file. If this is set, we will read that file for default configuration. If it is not set, we read config.json. If there is no config.json, we throw.

We then dynamically read the rest of the command line flags. We take the format like --release_config.any.cp 200 and use that to overwrite this key in the loaded configuration:

{
  "release_config": {
    "any": {
      "cp": 300
    }
  }
}

If there is no key, or no path to that key, for example --release_config.any.doesnt_exist 'foo' or --release_config.doesnt_exist, then we throw.

Down the road, if this gets unmanageable and people want something different, we add an additional command line argument called --config. This argument takes JSON and is merged on top of the loaded configuration file.

[AdaptCharm]

@TheSavior I like this way. I would rather do it in this approach.

[elicwhite]

@solderzzc and @douglascamata Can you chime in with your concerns or support for this proposal?

Also, can you rename the ticket to something like "Proposal: Enable multiple configuration files and overriding from CLI"

[solderzzc]

Sure, let me do it.