option to disallow surrender [fixes #32] (#36)

* attempting GoogleTest framework  #33

* added cmath include

* another approach to setting protected members

* i think i have the tests configured into CMake/CTest now

* start to look at GH actions workflow

* update notes on building

* update notes on building

* update notes on building

* updates to gh actions workflow

* add bmai_tests target

* notes

* fine tuning some config

* few more notes

* trying to work cout cmake3.8 limitations

* attempt at disallowing the AI to surrender

* updated changelog

* test a build

* apparently the filesystem namespace is a bit bonkers

* trying to get ubuntu filesystem working

* better filesystem support i hope

* typo

* create build dir

* trying to recall cmake3.11 syntax

* FetchContent_Populate

* this old version of cmake is missing some helpful things

* get rid of targets

* msbuild?

* typo

* create build dir

* trying to recall cmake3.11 syntax

* FetchContent_Populate

* this old version of cmake is missing some helpful things

* get rid of targets

* msbuild?

* better filesystem support i hope

* notes, comments, typos, clarifications, changelogs

* typos, comments, notes, changelogs

* add regular auto releases to GHActions workflow

.github/workflows/cmake.yml

@@ -1,9 +1,10 @@
 name: CMake
 
-# if you are doing development on another branch and want GHActions to auto build the branch
-# then add the branch name to the `push` section below
+# when developing on another branch 
+# if you want GHActions to auto build the branch
+# add the branch name to the `push` section below
 # you only need to make that change ON THE BRANCH to be built
-# I like to remove it when merging into `main`
+# please remove it when merging into `main`
 on:
   push:
     branches: [ main ]
@@ -22,13 +23,17 @@ on:
         - Release
         - Debug
 
+  schedule:
+    - cron: '30 2 * * SUN' # run at 02:30 UTC Sundays
+
 env:
   # if the build is not manually dispatched we will default to 'Release' build
   BUILD_TYPE: ${{ inputs.buildType || 'Release' }}
 
 jobs:
   build:
     strategy:
+      fail-fast: false
       matrix:
         os: [ubuntu-latest, windows-latest, macos-latest]
     # The CMake configure and build commands are platform-agnostic
@@ -48,38 +53,72 @@ jobs:
     - name: Add msbuild to PATH
       uses: microsoft/setup-msbuild@v1.3.1
       if: matrix.os == 'windows-latest'
+
+    # for some reason the git describe done inside of CMake
+    # isnt producing the desired results anymore
+    # so i am executing it out here and passing it on the command line
+    - name: git-describe for version string
+      shell: bash
+      run: |
+        echo GIT_DESCRIBE="$(git describe --always --tags --match='v[0-9]*' ${{'pull_request'==github.event_name && 'HEAD^2)-PR' || '--dirty)'}}" >> $GITHUB_ENV
 
 ## on later versions of cmake we can use the -S -B syntax
 #    - name: CMake Configure
 #      shell: bash
-#      run: cmake -S . -B build -DCMAKE_BUILD_TYPE=$BUILD_TYPE
-## later versions of cmake auto-create the build folder
+#      run: cmake -S . -B build -DGIT_DESCRIBE="$GIT_DESCRIBE" -DCMAKE_BUILD_TYPE=$BUILD_TYPE -G"Unix Makefiles"
 
     - name: CMake Configure
       shell: bash
-      run: mkdir -p build && cd build && cmake .. -DCMAKE_BUILD_TYPE=$BUILD_TYPE
+      run: mkdir -p build && cd build && cmake .. -DGIT_DESCRIBE="$GIT_DESCRIBE" -DCMAKE_BUILD_TYPE=$BUILD_TYPE -G"Unix Makefiles"
+      # Unix Makefiles allow the build to behave similarly cross-platform
+      # if we want to ensure that the project can build under a certain build type (like VS2017)
+      # we should consider a very specific GHAction Workflow for that case
+## later versions of cmake auto-create the build folder
 
     - name: CMake Build
       shell: bash
-      ## In later versions of cmake we can specify targets with "--target <NAMEA> <NAMEB>"
+ ## In later versions of cmake we can specify targets with "--target NAMEa NAMEb"
       run: cmake --build build --config $BUILD_TYPE
 
     - name: CTest
       working-directory: build
       shell: bash
       run: ctest -C $BUILD_TYPE --no-tests=error --output-junit ctest-results.xml
   # someday I want to try again to display the test results
-
-    - name: Upload *NIX Binary
-      uses: actions/upload-artifact@v3
-      if: matrix.os != 'windows-latest'
+
+    - uses: actions/upload-artifact@v3
       with:
-          name: bmai-${{ runner.os }}-${{ env.BUILD_TYPE }}
-          path: ./build/bmai
+        name: bmai-${{ runner.os }}-${{ env.BUILD_TYPE }}
+        if-no-files-found: error
+        path: |
+          ./build/bmai*
 
-    - name: Upload WINDOWS Binary
-      uses: actions/upload-artifact@v3
-      if: matrix.os == 'windows-latest'
+    - name: get data for release body
+      shell: bash
+      if: ${{ 'schedule' == github.event_name || 'workflow_dispatch' == github.event_name }}
+      run: |
+        SINCE_TAG="$(git describe --tags --abbrev=0 --match='v[0-9]*')"
+        TEMP_COMMIT_LOG="$(git log $SINCE_TAG..HEAD --oneline)"
+        echo SINCE_TAG="$SINCE_TAG" >> $GITHUB_ENV
+        echo COMMIT_LOG="${TEMP_COMMIT_LOG//$'\n'/<br />}" >> $GITHUB_ENV
+        echo DATE="$(date -u)" >> $GITHUB_ENV
+
+    - name: auto-release tag and upload
+      uses: andelf/nightly-release@main
+      if: ${{ 'schedule' == github.event_name || 'workflow_dispatch' == github.event_name }}
+      env:
+        GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
       with:
-          name: bmai-${{ runner.os }}-${{ env.BUILD_TYPE }}
-          path: ${{ github.workspace }}\build\${{ env.BUILD_TYPE }}\bmai.exe
+        tag_name: auto-release-${{ runner.os }}
+        name: '[auto] BMAI-${{ runner.os }}'
+        prerelease: true
+        body: |
+          Branch: ${{ github.head_ref || github.ref_name || github.ref }} 
+          Build Type: ${{ env.BUILD_TYPE }}
+          Run At: ${{ env.DATE }}
+          Trigger: ${{ github.event_name }}
+          Binary Version: ${{ env.GIT_DESCRIBE }} 
+          <details><summary>Changes since ${{ env.SINCE_TAG }}</summary>${{ env.COMMIT_LOG }}</details>
+        files: |
+          ./build/bmai*
+      # TODO: add other files maybe like test results

CMakeLists.txt

@@ -12,12 +12,14 @@ set(CMAKE_CXX_EXTENSIONS OFF)
 project(${NAME})
 
 # Get the latest git tag and short commit hash
-execute_process(
-    COMMAND git describe --dirty --always --tags
-    WORKING_DIRECTORY ${CMAKE_CURRENT_LIST_DIR}
-    OUTPUT_VARIABLE GIT_DESCRIBE
-    OUTPUT_STRIP_TRAILING_WHITESPACE
-)
+if(NOT DEFINED GIT_DESCRIBE)
+    execute_process(
+        COMMAND git describe --dirty --always --tags --match="v[0-9]*"
+        WORKING_DIRECTORY ${CMAKE_CURRENT_LIST_DIR}
+        OUTPUT_VARIABLE GIT_DESCRIBE
+        OUTPUT_STRIP_TRAILING_WHITESPACE
+    )
+endif ()
 add_definitions("-DGIT_DESCRIBE=\"${GIT_DESCRIBE}\"")
 
 enable_testing()

src/bmai.cpp

@@ -96,6 +96,7 @@
 //				  46.7% with Yoyodyne, 
 // dbl051823	- added P-Swing, Q-Swing support
 // dbl053123	- moved TestRNG and SetupTestGame to a `./test/` dir and testing framework
+// dbl053123	- added ability to disallow surrenders
 ///////////////////////////////////////////////////////////////////////////////////////////
 
 // TOP TODO
@@ -436,7 +437,7 @@ void BMC_Logger::Log(BME_DEBUG _cat, char *_fmt, ... )
 
 bool BMC_Logger::SetLogging(const char *_catname, bool _log)
 {
-	// case insensitive compares are not easy to come by in a cross-platform way
+	// case-insensitive compares are not easy to come by in a cross-platform way
 	// so lets uppercase the input and do a normal std::strcmp below
 	std::string _cn(_catname);
 	std::transform(_cn.begin(), _cn.end(), _cn.begin(), ::toupper);
@@ -531,7 +532,7 @@ void BMC_DieIndexStack::SetBits(BMC_BitArray<BMD_MAX_DICE> & _bits)
 
 // DESC: add the next die to the stack.  If we can't, then remove the top die in the stack 
 //       and replace it with the next available die 
-// PARAM: _add_die: if specificed, then force cycling the top die 
+// PARAM: _add_die: if specified, then force cycling the top die
 // RETURNS: if finished (couldn't cycle)
 bool BMC_DieIndexStack::Cycle(bool _add_die)
 {
@@ -1279,6 +1280,7 @@ BMC_Game::BMC_Game(bool _simulation)
 	m_target_wins = BMD_DEFAULT_WINS;
 	m_simulation = _simulation;
 	m_last_action = BME_ACTION_MAX;
+	m_surrender_allowed = true;
 }
 
 BMC_Game::BMC_Game(const BMC_Game & _game)
@@ -3862,6 +3864,7 @@ maxbranch %1		maximum number of total simulations to run at a ply (valid moves *
 debug %1 %2			adjust logging settings (e.g. "debug SIMULATION 0")
 debugply %1
 ai %1 %2			set player %1 (0-1) to AI type %2 (0 = BMAI, 1 = QAI, 2 = BMAI v2)
+surrender %1        set if AI is allowed to surrender. If off then AI will continue to play loosing positions. [default is on]
 
 ACTIONS
 playgame %1			play %1 games and output results 
@@ -3995,6 +3998,10 @@ void BMC_Parser::Parse()
 			g_rng.SRand(param);
 			printf("Seeding with %d\n", param);
 		}
+        else if (sscanf(line, "surrender %32s", &sparam)==1)
+        {
+            g_game.SetSurrenderAllowed(std::string(sparam)=="on");
+        }
 		else if (!std::strcmp(line, "quit"))
 		{
 			return;

src/bmai_ai.cpp

@@ -1094,7 +1094,7 @@ void BMC_BMAI3::GetAttackAction(BMC_Game *_game, BMC_Move &_move)
 	}
 
 	// SURRENDER: if best move is 0% win, then surrender
-	if (t.best_score==0)
+	if (t.best_score==0 && _game->IsSurrenderAllowed())
 		t.best_move->m_action = BME_ACTION_SURRENDER;
 
 	if (enter_level < sm_debug_level)

src/bmai_ai.h

@@ -10,6 +10,8 @@
 // drp030321 - partial split out to individual headers
 ///////////////////////////////////////////////////////////////////////////////////////////
 
+#include "game.h"
+
 #ifndef __BMAI_AI_H__
 #define __BMAI_AI_H__
 

src/game.h

@@ -79,6 +79,9 @@ class BMC_Game		// 420b
 	float		PlayFight_EvaluateMove(INT _pov_player, BMC_Move &_move);
 	float		PlayRound_EvaluateMove(INT _pov_player);
 
+	bool		IsSurrenderAllowed() {return m_surrender_allowed;};
+	void		SetSurrenderAllowed(bool _surrender_allowed) {m_surrender_allowed=_surrender_allowed;};
+
 protected:
 	// game simulation - level 1
 	void		Setup(BMC_Man *_man1 = NULL, BMC_Man *_man2 = NULL);
@@ -115,4 +118,6 @@ class BMC_Game		// 420b
 
 	// is this a simulation run by the AI?
 	bool		m_simulation;
+
+	bool		m_surrender_allowed;
 };

src/parser.h

@@ -30,7 +30,9 @@ class BMC_Parser
 	bool			Read(bool _fatal = true);
 
 	// output
-	void			Send(char *_fmt, ...);
+
+    // virtual can be overridden in subclass. makes testing a little easier
+	virtual void	Send(char *_fmt, ...);
 	void			SendStats();
 	void			SendSetSwing(BMC_Move &_move);
 	void			SendUseReserve(BMC_Move &_move);

test/BMAI3Tests.cpp

@@ -0,0 +1,94 @@
+#include <gtest/gtest.h>
+#include "../src/bmai_ai.h"
+#include "../src/parser.h"
+#include <cstdarg>  // for va_start() va_end()
+
+// filesystem was weird in c++14 with various ways the operating systems juggled the experimental namespace
+// this util i found gives us c++17 filesystem patterns down in older c++
+#include <ghc/filesystem.hpp>
+namespace fs = ghc::filesystem;
+
+// a path util, if useful consider creaking out into generic utility
+inline std::string resolvePath(const std::string &relPath)
+{
+    auto baseDir = fs::current_path();
+    while (baseDir.has_parent_path())
+    {
+        auto combinePath = baseDir / relPath;
+        if (fs::exists(combinePath))
+        {
+            return combinePath.string();
+        }
+        if(baseDir==baseDir.parent_path()) {
+            break;
+        } else {
+            baseDir = baseDir.parent_path();
+        }
+
+    }
+    throw std::runtime_error("File not found!");
+}
+
+// lets us collect what is being sent to Send for inspection during tests
+class TEST_Parser : public BMC_Parser {
+public:
+    void Send(char *_fmt, ...) {
+        tm_next_to_last_fmt = tm_last_fmt;
+
+        char buff[BMD_MAX_STRING];
+        va_list ap;
+        va_start (ap, _fmt);
+        vsnprintf(buff, BMD_MAX_STRING, _fmt, ap);
+        va_end (ap);
+
+        tm_last_fmt = std::string(buff);
+    }
+    std::string tm_next_to_last_fmt;
+    std::string tm_last_fmt;
+};
+
+class BMAISurrenderTests :public ::testing::TestWithParam<std::tuple<std::string, std::string>> {
+protected:
+    TEST_Parser parser;
+};
+
+// I wanted to test the smaller objects and methods. 
+// For example i could build an AI object and just execute the method GetAttackAction()
+// to inspect the behavior arround surrender
+// however the code is not currently built in a way that is friendly to unit testing
+// so the current pattern is to run full simulations and to monitor the output
+
+INSTANTIATE_TEST_SUITE_P(
+        BMAI3Tests,
+        BMAISurrenderTests,
+        ::testing::Values(
+            std::make_tuple("test/SurrenderDefault-Pass-in.txt", "action\nsurrender\n"),
+            std::make_tuple("test/SurrenderOff-Attack-in.txt", "0\n0\n"),
+            std::make_tuple("test/SurrenderOff-Pass-in.txt", "action\npass\n"),
+            std::make_tuple("test/SurrenderOn-Attack-in.txt", "action\nsurrender\n"),
+            std::make_tuple("test/SurrenderOn-Pass-in.txt", "action\nsurrender\n")
+        ));
+
+TEST_P(BMAISurrenderTests, CheckSurrenderAction){
+    // Arrange Act Assert
+    // is a parallel to the pattern
+    // Given When Then
+
+    // Arrange
+    // Given params that describe out inputs and expected outputs
+    std::string test_file = std::get<0>(GetParam());
+    std::string expected_last_actions= std::get<1>(GetParam());
+
+    // Arrange
+    // Given a game with one of many surrender combinations
+    std::string inpath = resolvePath(test_file);
+    FILE *in = fopen(inpath.c_str(),"r");
+
+    // Act
+    // When calculating the best move
+    parser.Parse(in);
+
+    // Assert
+    // Then the move selected should match our expectations
+    EXPECT_EQ(parser.tm_next_to_last_fmt+parser.tm_last_fmt, expected_last_actions);
+}

test/CMakeLists.txt

@@ -22,10 +22,18 @@ set(gtest_force_shared_crt ON CACHE BOOL "" FORCE)
 FetchContent_Populate(googletest)
 add_subdirectory(${googletest_SOURCE_DIR} ${googletest_BINARY_DIR})
 
+# i wanted to do some file system work in the tests
+# std::filesystem doesnt exist until c++17 & Boost is HUGE
+# found this nice little utility with brings c++17 filesystem apis to older versions
+FetchContent_Declare(ghc GIT_REPOSITORY https://github.com/gulrak/filesystem.git GIT_TAG v1.5.14)
+FetchContent_Populate(ghc)
+add_subdirectory(${ghc_SOURCE_DIR} ${ghc_BINARY_DIR})
 
+
+# TODO: get rid of glob, bad practice apparently
 file(GLOB TEST_SOURCES *.cpp *.h)
 add_executable(${NAME}_tests ${TEST_SOURCES})
-target_link_libraries(${NAME}_tests ${NAME}_lib gtest_main)
+target_link_libraries(${NAME}_tests ${NAME}_lib gtest_main ghc_filesystem)
 
 
 ##

test/SurrenderDefault-Pass-in.txt

@@ -0,0 +1,9 @@
+game
+fight
+player 0 1 1
+1:1
+player 1 1 30
+(30,30):60
+
+
+getaction

test/SurrenderOff-Attack-in.txt

@@ -0,0 +1,9 @@
+game
+fight
+player 0 1 1
+1:1
+player 1 2 30
+1:1
+(30,30):60
+surrender off
+getaction

test/SurrenderOff-Pass-in.txt

@@ -0,0 +1,9 @@
+game
+fight
+player 0 1 1
+1:1
+player 1 1 30
+(30,30):60
+
+surrender off
+getaction

test/SurrenderOn-Attack-in.txt

@@ -0,0 +1,9 @@
+game
+fight
+player 0 1 1
+1:1
+player 1 2 30
+1:1
+(30,30):60
+surrender on
+getaction