Request for Feedback: CLI output format

[robbles]

Although the rules CLI allows running games locally, the output isn't easy to parse and is mostly designed for humans. A standard way to run a local game and retrieve the results programmatically has been a common feature request.

@simonamdev has a PR that adds an option to output the game state and results to a file in JSON lines (BattlesnakeOfficial/rules#56). We'd like to get some feedback from the community about the format and implementation details, to ensure we're covering most people's needs.

A couple of questions as a starting point:

1. Should the CLI output the JSON to stdout by default, instead of to a file?
2. Should using this option mute other human-friendly output, or should it go to stderr? Or have a separate quiet mode option?
3. Do the JSON objects emitted have enough context to be useful? Do they need a wrapper with a type field?
4. Should the output fields match the Battlesnake API as much as possible?
   • (vs. matching the internals of BattlesnakeOfficial/rules instead as they do now)
5. Given the detection of winners is a simplification of what the production Battlesnake engine does, should we remove this and rely on devs to apply their own win/lose logic based on the state?

[BoldBigflank]

** Note this is all my opinion, I'm editing out all my subjective qualifiers to be more concise **
It would be important if you're exporting a game state to be able to import that same game state back into the cli. I'm imagining being able to start a game using the exported game's ruleset, plus choosing a turn to start on as well.
I see the advantage of making the output look like the API, but I'm not sure you can actually build a game from just that information. It doesn't include snake urls for example. Perhaps if the output included an array of objects that have the game state sent to POST /move for each turn (generally, minus the you link), that would be the best of both worlds.

[simonamdev]

Thanks to everyone for considering this 😄 here's my 2c:

1. I thought a file made more sense than stdout because by doing so it would effectively have to replace the human friendly output. By going to a file, a human can still watch it or tail those logs knowing the game is still running without having to redirect it to a file themselves. I also think knowledge around handling stdout/err is less common in users on Windows, so a file lowers the barrier to entry.
2. I think the JSON objects emitted are sufficient because its all you really need to know to examine a game. One could always provide inline schema details, but I'm not sure its worth doing so because well formatted documentation can cover this (the same way the API calls don't include schema details)
3. On matching the API instead of the internals, I'm kind of convinced that from a UX perspective it would be better (and can potentially lower the barrier to using that generated output for other means, such as replaying games by re-reading the file and re-sending the API calls). Having said that, I myself have already written conversion functions from one format to the other, so I'm not fussed about it. Consistency is good, and simplicity is too.
4. I think not providing details about win/lose would be a net negative, since devs would have to maintain their own interpretation of the results, especially if the internals had to change.

[xtagon]

My own opinions:

Should the CLI output the JSON to stdout by default, instead of to a file?

I think stdout should be the default. More UNIX-friendly, and you can easily append to a file in bash for example [cli command] > file.jsonl using the > operator if a file is preferred.

Should using this option mute other human-friendly output, or should it go to stderr? Or have a separate quiet mode option?

IMO it would make sense to implement this as separate formatters. Have one formatter option for the legacy Go struct output, have a new formatter option for human-readable logs, have another formatter for JSONL streams, etc.

Better to give people the option than to try to fit every shape into one implementation. This is how popular test suite packages do it too.

Do the JSON objects emitted have enough context to be useful? Do they need a wrapper with a type field?

No opinion, but any way it's done, please prefer backwards compatibility.

Should the output fields match the Battlesnake API as much as possible?

I would say no, trying to keep that in sync will be a chore, and probably not worth it because the engine already has different internals than the public-facing Battlesnake API.

Given the detection of winners is a simplification of what the production Battlesnake engine does, should we remove this and rely on devs to apply their own win/lose logic based on the state?

No, I would say don't remove existing behavior. You could make it an option?

[coreyja]

My turn!

Should the CLI output the JSON to stdout by default, instead of to a file?

I like stdout, as xtagon said is more UNIXy which I like in my clis. It's also more convenient for scripting where I might want to pipe the results to another process. I've done this with the current CLI output to manually grab the winner name from the text, for example.

Should using this option mute other human-friendly output, or should it go to stderr? Or have a separate quiet mode option?

Agree with xtagon again! Having different formatters would be my preference.

For bonus points it would be really cool to be able to specify a 'channel' for a formatter. Imagining a CLI maybe its something like:

--formatter json --to stdout --formatter human --to stderr --formatter internal --to file://some-file.txt

Do the JSON objects emitted have enough context to be useful?

Yup!

Do they need a wrapper with a type field?

Not neccesary

Should the output fields match the Battlesnake API as much as possible? (vs. matching the internals of BattlesnakeOfficial/rules instead as they do now)

Would prefer matching the output as much as possible but as others have said its not challenging to convert between formats.

Given the detection of winners is a simplification of what the production Battlesnake engine does, should we remove this and rely on devs to apply their own win/lose logic based on the state?

Would like to keep it! I think in a perfect world the prod win/lose detection would be part of this repo, or available for this repo to use in some way. But understand that may not be possible/reasonable neccesarily

[robbles]

Thanks for the feedback everyone - this is great 😄

Summarizing what's been said so far:

CLI output to stdout vs. file

• Good points made here for both options
• Maybe we can stick with the filename option, but support interpreting a value of - as stdout, which seems to be a common convention

Output format

• Different output formats (JSONL, human readable, go structs) should be implemented as formatters
  • I think this could be part of a later refactor, as the current implementation doesn't seem to block any obvious use cases
• JSON objects are sufficient without a type field
• Enough info exported to re-run a game
  • I see two small things missing for this: ruleset name and game width/height in the initial JSON object - this first line should probably look like the game object instead of just the ruleset settings

Matching the Battlesnake API

• No consensus
• I think this is probably worth doing, because we need to keep the code for making requests to Battlesnakes compatible with the production engine regardless
• At a minimum, fields like the ruleset settings and other multi-word fields should be camel case - as snake case they don't match anything and are likely to cause friction for no obvious benefit

Removing winner detection

• No, keep it!
• I think this is fine for now - the only case I'm aware of where it could be wrong is for challenges, which aren't available for the CLI. We'll just have to keep an eye on it as more rulesets and options are introduced.

I've highlighted the points above that I think require changes in the PR before merging. @simonamdev, do you have time and interest to keep working on this? I'm happy to either keep reviewing or take it over - let me know!

[simonamdev]

I'm good to continue working on it! I have time off from now till the end of the year so I'll be working on bits and pieces from now till then. If life gets in the way I'll reach out to hand it over.

Some thoughts I had after reviewing the feedback before I go ahead:

1. Does it make sense to have the output be in camelCase whilst also matching the battlesnake API? Having it in camelCase (rather than snake_case which is already the case with output from the API). The data will be in the awkward state of matching the API payload in terms of structure but not in terms of key convention
2. In the interest of minimising time to release, I was thinking I would go for JSONL and filename options only. Once this is in place then it would enable parallelisation of working on different file formats & the option to send to stdout.

WDYT?

[robbles]

2. Makes sense - I don't see adding stdout as an option to be a breaking change either, so there's no reason it can't happen afterwards.

3. I'm not sure I'm following you here. Where's the snake case in the API that you're seeing? The Battlesnake API mostly avoids this by using single word fields, but uses camel case where needed such as in the ruleset settings. The Go fieldnames are also camel case by convention.

[simonamdev]

For the formatter: 👍

For the API Response... call me stupid but I could have SWORN It was in snake_case. Turns out you're completely right. This is after I've competed in two leagues already 🤦‍♂️. Then camel case makes 100% sense.

[robbles]

I think it looks a lot like snake case because 90% of it is single lowercase words 😄