Validate saved key action map before loading it

[EvoLandEco]

It would be safer if we check the saved file before loading it. I suppose the code should look like this:

bool is_kam(const key_action_map& kam) 
{
    bool flag;
    code_to_check_kam;
    return flag;
}

Then

key_action_map load_kam(const std::string& filename)
{
  std::ifstream f(filename);
  key_action_map m;
  f >> m;

  if (is_kam(m))
  {
      return m;
  } else
  {
      show_error_message?;
  }

}

[richelbilderbeek]

@EvoLandEco I suggest to write the tests first on that, instead of the implementation: it is the tests that shape the implementation. Or: we usually don't care about how a function does its thing, as long as it does so correctly. This also allows team members from every level to implement a function (e.g. using raw for loops instead of algorithms).

When writing the tests, you'd observe that the first test will be close to irrelevant for us (as the strong type checking checks if a class is ... well, its class). I encourage you to try and see how silly the test will look :-)

The second test would expect an exception (e.g. a std::runtime_exception) from reading a nonsense std::istream (yes, istream, not ifstream) using operator>>. We have no function yet similar to the R function testthat::expect_error, but it could easily be written.

[EvoLandEco]

Thanks for your advice, once again I forgot to do it in a test-driven way.

I like to write silly tests, let's get it done tomorrow

[EvoLandEco]

@EvoLandEco I suggest to write the tests first on that, instead of the implementation: it is the tests that shape the implementation. Or: we usually don't care about how a function does its thing, as long as it does so correctly. This also allows team members from every level to implement a function (e.g. using raw for loops instead of algorithms).

When writing the tests, you'd observe that the first test will be close to irrelevant for us (as the strong type checking checks if a class is ... well, its class). I encourage you to try and see how silly the test will look :-)

The second test would expect an exception (e.g. a std::runtime_exception) from reading a nonsense std::istream (yes, istream, not ifstream) using operator>>. We have no function yet similar to the R function testthat::expect_error, but it could easily be written.

I feel like we could include key action map in player settings, as this mapping should also be player specific. And besides player settings, perhaps we should store player attributes independent of the settings? And make different sets of functions to read/write them.