Skip to content
This repository has been archived by the owner on Jul 31, 2023. It is now read-only.

Fix recently found bugs #1

Merged
merged 4 commits into from
Oct 20, 2020
Merged

Fix recently found bugs #1

merged 4 commits into from
Oct 20, 2020

Conversation

alkurbatov
Copy link
Member

@alkurbatov alkurbatov commented Oct 8, 2020

  • Document how to launch old replays (OS X + Win).
  • Split the input params with '--'. The observer's params go before '--', the game params go after '--'.
  • Rebase on the newer API: Abort the game launch if we found an unexpected option.
  • Integrate Google tests.

Brings the following:
* Don't allow to start the game with invalid input args.
* Fixes project compilation on the latest OS X.

Signed-off-by: Alexander Kurbatov <Alexander.Kurbatov@acronis.com>
@alkurbatov alkurbatov added the bug Something isn't working label Oct 8, 2020
Copy link
Contributor

@lladdy lladdy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.
Does this need any further testing?

@alkurbatov
Copy link
Member Author

alkurbatov commented Oct 9, 2020

Small refactoring needed for better testing later and there is missing delimiter in the windows example.

Alexander Kurbatov added 2 commits October 9, 2020 12:46
Observer has several options which conflict with SC2 (e.g. -p, --Path,
vs -p, --port) and we can't explicitly specify which options should be
used e.g. only in Observer and which only in SC2. As a result, such
design causes confusion and weird behavior.

Adopt the approach used by various command line tools and request division
of cmdline options with the -- delimiter.

Signed-off-by: Alexander Kurbatov <Alexander.Kurbatov@acronis.com>
Signed-off-by: Alexander Kurbatov <Alexander.Kurbatov@acronis.com>
@alkurbatov
Copy link
Member Author

Changes:

  • Moved the input args split function to separate files in order to reuse for testing later.
  • Add missing delimiter to the example.

Not checked:

  • Windows compilation.

* Integrate Google testing framework into the project.
* Add simple unit tests for ArgParser::splitInputOptions.

Signed-off-by: Alexander Kurbatov <Alexander.Kurbatov@acronis.com>
@alkurbatov
Copy link
Member Author

Now with unit tests :)

@alkurbatov alkurbatov requested a review from lladdy October 10, 2020 18:05
@@ -1,4 +1,4 @@
cmake_minimum_required(VERSION 3.1)
cmake_minimum_required(VERSION 3.10)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Required to use gtest_discover_tests

@danielvschoor danielvschoor merged commit 545221c into aiarena:master Oct 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants