added gamestate_observer class,

...used it to simplify candidate actions interface and implementation,
by making is_gamestate_changed flag managed externally, by rca stage

projectfiles/CodeBlocks-SCons/wesnoth.cbp

@@ -99,6 +99,8 @@
 		<Unit filename="..\..\src\ai\formula\stage_unit_formulas.hpp" />
 		<Unit filename="..\..\src\ai\game_info.cpp" />
 		<Unit filename="..\..\src\ai\game_info.hpp" />
+		<Unit filename="..\..\src\ai\gamestate_observer.cpp" />
+		<Unit filename="..\..\src\ai\gamestate_observer.hpp" />
 		<Unit filename="..\..\src\ai\interface.cpp" />
 		<Unit filename="..\..\src\ai\interface.hpp" />
 		<Unit filename="..\..\src\ai\manager.cpp" />

projectfiles/CodeBlocks/wesnoth.cbp

@@ -128,6 +128,8 @@
 		<Unit filename="..\..\src\ai\formula\stage_unit_formulas.hpp" />
 		<Unit filename="..\..\src\ai\game_info.cpp" />
 		<Unit filename="..\..\src\ai\game_info.hpp" />
+		<Unit filename="..\..\src\ai\gamestate_observer.cpp" />
+		<Unit filename="..\..\src\ai\gamestate_observer.hpp" />
 		<Unit filename="..\..\src\ai\interface.cpp" />
 		<Unit filename="..\..\src\ai\interface.hpp" />
 		<Unit filename="..\..\src\ai\manager.cpp" />

projectfiles/VC9/wesnoth.vcproj

@@ -4148,6 +4148,34 @@
 						/>
 					</FileConfiguration>
 				</File>
+				<File
+					RelativePath="..\..\src\ai\gamestate_observer.cpp"
+					>
+					<FileConfiguration
+						Name="Debug|Win32"
+						>
+						<Tool
+							Name="VCCLCompilerTool"
+							ObjectFile="$(IntDir)\ai\"
+						/>
+					</FileConfiguration>
+					<FileConfiguration
+						Name="Release|Win32"
+						>
+						<Tool
+							Name="VCCLCompilerTool"
+							ObjectFile="$(IntDir)\ai\"
+						/>
+					</FileConfiguration>
+					<FileConfiguration
+						Name="Debug (fast)|Win32"
+						>
+						<Tool
+							Name="VCCLCompilerTool"
+							ObjectFile="$(IntDir)\ai\"
+						/>
+					</FileConfiguration>
+				</File>
 				<File
 					RelativePath="..\..\src\ai\interface.cpp"
 					>
@@ -6219,6 +6247,10 @@
 					RelativePath="..\..\src\ai\game_info.hpp"
 					>
 				</File>
+				<File
+					RelativePath="..\..\src\ai\gamestate_observer.hpp"
+					>
+				</File>
 				<File
 					RelativePath="..\..\src\ai\interface.hpp"
 					>

src/CMakeLists.txt

@@ -189,6 +189,7 @@ set(wesnoth-main_SRC
     ai/formula/stage_side_formulas.cpp
     ai/formula/stage_unit_formulas.cpp
     ai/game_info.cpp
+    ai/gamestate_observer.cpp
     ai/interface.cpp
     ai/manager.cpp
     ai/registry.cpp

src/Makefile.am

@@ -69,6 +69,7 @@ wesnoth_source = \
 	ai/formula/stage_side_formulas.cpp \
 	ai/formula/stage_unit_formulas.cpp \
 	ai/game_info.cpp \
+	ai/gamestate_observer.cpp \
 	ai/interface.cpp \
 	ai/manager.cpp \
 	ai/registry.cpp \

src/SConscript

@@ -174,6 +174,7 @@ wesnoth_sources = Split("""
     ai/formula/stage_side_formulas.cpp
     ai/formula/stage_unit_formulas.cpp
     ai/game_info.cpp
+    ai/gamestate_observer.cpp
     ai/interface.cpp
     ai/manager.cpp
     ai/registry.cpp

src/ai/composite/engine_fai.cpp

@@ -57,9 +57,9 @@ public:
 	}
 
 
-	virtual bool execute()
+	virtual void execute()
 	{
-		return formula_ai_.execute_candidate_action(fai_ca_);
+		formula_ai_.execute_candidate_action(fai_ca_);
 	}
 
 	virtual config to_config() const

src/ai/composite/engine_lua.cpp

@@ -54,10 +54,9 @@ public:
 	}
 
 
-	virtual bool execute()
+	virtual void execute()
 	{
-		//@todo: lua must do the actions. the return value is supposed to be 'true if gamestate has changed' but it'll be refactored away soon.
-		return false;
+		//@todo: lua must do the actions.
 	}
 
 	virtual config to_config() const

src/ai/composite/rca.hpp

@@ -73,7 +73,7 @@ public:
 	/**
 	 * Execute the candidate action
 	 */
-	virtual bool execute() = 0;
+	virtual void execute() = 0;
 
 	/**
 	 * Is this candidate action enabled ?

src/ai/gamestate_observer.cpp

@@ -0,0 +1,58 @@
+/* $Id$ */
+/*
+   Copyright (C) 2010 by Yurii Chernyi <terraninfo@terraninfo.net>
+   Part of the Battle for Wesnoth Project http://www.wesnoth.org/
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License version 2
+   or at your option any later version.
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY.
+
+   See the COPYING file for more details.
+*/
+
+/**
+ * Base gamestate observer - useful to see if the gamestate has changed
+ * between two points of time
+ * @file ai/gamestate_observer.cpp
+ */
+
+#include "manager.hpp"
+#include "gamestate_observer.hpp"
+
+namespace ai {
+// =======================================================================
+
+gamestate_observer::gamestate_observer()
+	: gamestate_change_counter_(0)
+{
+	ai::manager::add_gamestate_observer(this);
+}
+
+
+gamestate_observer::~gamestate_observer()
+{
+	ai::manager::remove_gamestate_observer(this);
+}
+
+
+void gamestate_observer::handle_generic_event(const std::string &event_name)
+{
+	++gamestate_change_counter_;
+}
+
+
+bool gamestate_observer::is_gamestate_changed()
+{
+	return (gamestate_change_counter_>0);
+}
+
+
+void gamestate_observer::reset()
+{
+	gamestate_change_counter_=0;
+}
+
+// =======================================================================
+} //end of namespace ai

src/ai/gamestate_observer.hpp

@@ -0,0 +1,59 @@
+/* $Id$ */
+/*
+   Copyright (C) 2010 by Yurii Chernyi <terraninfo@terraninfo.net>
+   Part of the Battle for Wesnoth Project http://www.wesnoth.org/
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License version 2
+   or at your option any later version.
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY.
+
+   See the COPYING file for more details.
+*/
+
+/**
+ * @file ai/gamestate_observer.hpp
+ * A helper class to observe the game state
+ */
+
+#ifndef AI_GAMESTATE_OBSERVER_HPP_INCLUDED
+#define AI_GAMESTATE_OBSERVER_HPP_INCLUDED
+
+#include "../generic_event.hpp"
+
+namespace ai {
+
+class gamestate_observer : public events::observer {
+public:
+
+
+	gamestate_observer();
+
+
+	virtual ~gamestate_observer();
+
+
+	void handle_generic_event(const std::string &event_name);
+
+
+	/**
+	 * Check if the gamestate has changed since last reset
+	 * reset is done once on construction, and can be done
+	 * by hand via reset() method.
+	 * @return has gamestate changed since last reset?
+	 */
+	bool is_gamestate_changed();
+
+	/**
+	 * Reset the counter of gamestate changes.
+	 */
+	void reset();
+
+private:
+	int gamestate_change_counter_;
+};
+
+} //of namespace ai
+
+#endif

src/ai/testing/ca.cpp

@@ -92,19 +92,16 @@ double goto_phase::evaluate()
 	return BAD_SCORE;
 }
 
-bool goto_phase::execute()
+void goto_phase::execute()
 {
-	bool gamestate_changed = false;
 	if (!move_) {
-		return gamestate_changed;
+		return;
 	}
 
 	move_->execute();
 	if (!move_->is_ok()){
 		LOG_AI_TESTING_AI_DEFAULT << get_name() << "::execute not ok" << std::endl;
 	}
-	gamestate_changed |= move_->is_gamestate_changed();
-	return gamestate_changed;
 }
 
 //==============================================================
@@ -140,15 +137,15 @@ double aspect_recruitment_phase::evaluate()
 	return get_score();
 }
 
-bool aspect_recruitment_phase::execute()
+void aspect_recruitment_phase::execute()
 {
 	raise_user_interact();
 	stage_ptr r = get_recruitment(*this);
 	if (r) {
-		return r->play_stage();
+		r->play_stage();
+	} else {
+		ERR_AI_TESTING_AI_DEFAULT << "no recruitment aspect - skipping recruitment" << std::endl;
 	}
-	ERR_AI_TESTING_AI_DEFAULT << "no recruitment aspect - skipping recruitment" << std::endl;
-	return false;
 }
 
 //==============================================================
@@ -188,9 +185,8 @@ double recruitment_phase::evaluate()
 	return get_score();
 }
 
-bool recruitment_phase::execute()
+void recruitment_phase::execute()
 {
-	bool gamestate_changed = false;
 	not_recommended_units_.clear();
 	unit_combat_scores_.clear();
 	unit_movement_scores_.clear();
@@ -258,7 +254,7 @@ bool recruitment_phase::execute()
 			<< scouts_wanted << " in total\n";
 
 		while(unit_types["scout"] < scouts_wanted) {
-			if (!recruit_usage("scout",gamestate_changed)){
+			if (!recruit_usage("scout")){
 				break;
 			}
 			++unit_types["scout"];
@@ -271,7 +267,7 @@ bool recruitment_phase::execute()
 		options.push_back("");
 	}
 	// Buy units as long as we have room and can afford it.
-	while (recruit_usage(options[rand()%options.size()],gamestate_changed)) {
+	while (recruit_usage(options[rand()%options.size()])) {
 		//refresh the recruitment pattern - it can be changed by recruit_usage
 		options = get_recruitment_pattern();
 		if (options.empty()) {
@@ -279,10 +275,9 @@ bool recruitment_phase::execute()
 		}
 	}
 
-	return gamestate_changed;
 }
 
-bool recruitment_phase::recruit_usage(const std::string& usage, bool &gamestate_changed)
+bool recruitment_phase::recruit_usage(const std::string& usage)
 {
 	raise_user_interact();
 
@@ -327,7 +322,6 @@ bool recruitment_phase::recruit_usage(const std::string& usage, bool &gamestate_
 	if(options.empty() == false) {
 		const int option = rand()%options.size();
 		recruit_result_ptr recruit_result = execute_recruit_action(options[option]);
-		gamestate_changed |= recruit_result->is_gamestate_changed();
 		return recruit_result->is_ok();
 	}
 	if (found) {
@@ -581,29 +575,26 @@ double combat_phase::evaluate()
 	}
 }
 
-bool combat_phase::execute()
+void combat_phase::execute()
 {
 	assert(choice_rating_ > 0.0);
-	bool gamestate_changed = false;
 	map_location from   = best_analysis_.movements[0].first;
 	map_location to     = best_analysis_.movements[0].second;
 	map_location target_loc = best_analysis_.target;
 
 	if (from!=to) {
 		move_result_ptr move_res = execute_move_action(from,to,false);
-		gamestate_changed |= move_res->is_gamestate_changed();
 		if (!move_res->is_ok()) {
-			return gamestate_changed;
+			LOG_AI_TESTING_AI_DEFAULT << get_name() << "::execute not ok, move failed" << std::endl;
+			return;
 		}
 	}
 
 	attack_result_ptr attack_res = execute_attack_action(to, target_loc, -1);
-	gamestate_changed |= attack_res->is_gamestate_changed();
 	if (!attack_res->is_ok()) {
-		return gamestate_changed;
+		LOG_AI_TESTING_AI_DEFAULT << get_name() << "::execute not ok, attack failed" << std::endl;
 	}
 
-	return gamestate_changed;
 }
 
 //==============================================================
@@ -699,22 +690,18 @@ double move_leader_to_goals_phase::evaluate()
 
 }
 
-bool move_leader_to_goals_phase::execute()
+void move_leader_to_goals_phase::execute()
 {
-	bool gamestate_changed = false;
 	move_->execute();
 	if (!move_->is_ok()){
 		LOG_AI_TESTING_AI_DEFAULT << get_name() << "::execute not ok" << std::endl;
 	}
-	gamestate_changed |= move_->is_gamestate_changed();
 	if (move_->get_unit_location()==dst_) {
 		//goal already reached
 		if (auto_remove_ && !id_.empty()) {
 			remove_goal(id_);
 		}
 	}
-
-	return gamestate_changed;
 }
 
 void move_leader_to_goals_phase::remove_goal(const std::string &id)
@@ -802,15 +789,12 @@ double move_leader_to_keep_phase::evaluate()
 	return BAD_SCORE;
 }
 
-bool move_leader_to_keep_phase::execute()
+void move_leader_to_keep_phase::execute()
 {
-	bool gamestate_changed = false;
 	move_->execute();
 	if (!move_->is_ok()){
 		LOG_AI_TESTING_AI_DEFAULT <<  get_name() <<"::execute not ok" << std::endl;
 	}
-	gamestate_changed |= move_->is_gamestate_changed();
-	return gamestate_changed;
 }
 
 //==============================================================
@@ -840,9 +824,9 @@ double get_villages_phase::evaluate()
 	return BAD_SCORE;
 }
 
-bool get_villages_phase::execute()
+
+void get_villages_phase::execute()
 {
-	bool gamestate_changed = false;
 	unit_map &units_ = get_info().units;
 	unit_map::const_iterator leader = units_.find_leader(get_side());
 	// Move all the units to get villages, however move the leader last,
@@ -856,9 +840,8 @@ bool get_villages_phase::execute()
 		} else {
 			if(units_.count(i->first) == 0) {
 				move_result_ptr move_res = execute_move_action(i->second,i->first,true);
-				gamestate_changed |= move_res->is_gamestate_changed();
 				if (!move_res->is_ok()) {
-					return gamestate_changed;
+					return;
 				}
 
 				const map_location loc = move_res->get_unit_location();
@@ -882,14 +865,13 @@ bool get_villages_phase::execute()
 	if(leader_move.second.valid()) {
 		if(units_.count(leader_move.first) == 0 && get_info().map.is_village(leader_move.first)) {
 			move_result_ptr move_res = execute_move_action(leader_move.second,leader_move.first,true);
-			gamestate_changed |= move_res->is_gamestate_changed();
 			if (!move_res->is_ok()) {
-				return gamestate_changed;
+				return;
 			}
 		}
 	}
 
-	return gamestate_changed;
+	return;
 }
 
 void get_villages_phase::get_villages(const moves_map& possible_moves,
@@ -1693,16 +1675,13 @@ double get_healing_phase::evaluate()
 	return BAD_SCORE;
 }
 
-bool get_healing_phase::execute()
+void get_healing_phase::execute()
 {
 	LOG_AI_TESTING_AI_DEFAULT << "moving unit to village for healing...\n";
-	bool gamestate_changed = false;
 	move_->execute();
 	if (!move_->is_ok()){
 		LOG_AI_TESTING_AI_DEFAULT << get_name() << "::execute not ok" << std::endl;
 	}
-	gamestate_changed |= move_->is_gamestate_changed();
-	return gamestate_changed;
 }
 
 //==============================================================
@@ -1816,15 +1795,12 @@ double retreat_phase::evaluate()
 	return BAD_SCORE;
 }
 
-bool retreat_phase::execute()
+void retreat_phase::execute()
 {
-	bool gamestate_changed = false;
 	move_->execute();
 	if (!move_->is_ok()){
 		LOG_AI_TESTING_AI_DEFAULT << get_name() << "::execute not ok" << std::endl;
 	}
-	gamestate_changed |= move_->is_gamestate_changed();
-	return gamestate_changed;
 }
 
 
@@ -1908,15 +1884,12 @@ double simple_move_and_targeting_phase::evaluate()
 	return BAD_SCORE;
 }
 
-bool simple_move_and_targeting_phase::execute()
+void simple_move_and_targeting_phase::execute()
 {
-	bool gamestate_changed = false;
 	move_->execute();
 	if (!move_->is_ok()){
 		LOG_AI_TESTING_AI_DEFAULT << get_name() << "::execute not ok" << std::endl;
 	}
-	gamestate_changed |= move_->is_gamestate_changed();
-	return gamestate_changed;
 }
 
 
@@ -1940,10 +1913,9 @@ double leader_control_phase::evaluate()
 
 
 
-bool leader_control_phase::execute()
+void leader_control_phase::execute()
 {
 	ERR_AI_TESTING_AI_DEFAULT << get_name() << ": execute - not yet implemented" << std::endl;
-	return false;
 }
 
 //==============================================================

src/ai/testing/ca.hpp

@@ -48,7 +48,7 @@ public:
 
 	virtual double evaluate();
 
-	virtual bool execute();
+	virtual void execute();
 private:
 	move_result_ptr move_;
 };
@@ -64,7 +64,7 @@ public:
 
 	virtual double evaluate();
 
-	virtual bool execute();
+	virtual void execute();
 };
 
 //============================================================================
@@ -78,11 +78,11 @@ public:
 
 	virtual double evaluate();
 
-	virtual bool execute();
+	virtual void execute();
 
 private:
 
-	bool recruit_usage(const std::string& usage, bool &gamestate_changed);
+	bool recruit_usage(const std::string& usage);
 
 	std::map<std::string,int> unit_movement_scores_;
 	std::set<std::string> not_recommended_units_;
@@ -125,7 +125,7 @@ public:
 
 	virtual double evaluate();
 
-	virtual bool execute();
+	virtual void execute();
 private:
 	attack_analysis best_analysis_;
 	double choice_rating_;
@@ -143,7 +143,7 @@ public:
 
 	virtual double evaluate();
 
-	virtual bool execute();
+	virtual void execute();
 private:
 
 	void remove_goal(const std::string &id);
@@ -165,7 +165,7 @@ public:
 
 	virtual double evaluate();
 
-	virtual bool execute();
+	virtual void execute();
 
 private:
 	move_result_ptr move_;
@@ -182,7 +182,7 @@ public:
 
 	virtual double evaluate();
 
-	virtual bool execute();
+	virtual void execute();
 private:
 	/** Location of the keep the closest to our leader. */
 	map_location keep_loc_;
@@ -274,7 +274,7 @@ public:
 
 	virtual double evaluate();
 
-	virtual bool execute();
+	virtual void execute();
 private:
 
 	move_result_ptr move_;
@@ -291,7 +291,7 @@ public:
 
 	virtual double evaluate();
 
-	virtual bool execute();
+	virtual void execute();
 private:
 
 	bool should_retreat(const map_location& loc, const unit_map::const_iterator& un, const move_map &srcdst, const move_map &dstsrc, double caution);
@@ -311,7 +311,7 @@ public:
 
 	virtual double evaluate();
 
-	virtual bool execute();
+	virtual void execute();
 
 private:
 
@@ -330,7 +330,7 @@ public:
 
 	virtual double evaluate();
 
-	virtual bool execute();
+	virtual void execute();
 };
 
 

src/ai/testing/ca_default_move_to_targets.cpp

@@ -126,10 +126,8 @@ double default_move_to_targets_phase::evaluate()
 }
 
 
-bool default_move_to_targets_phase::execute()
+void default_move_to_targets_phase::execute()
 {
-	bool gamestate_changed = false;
-
 	unit_map::const_iterator leader = get_info().units.find_leader(get_side());
 	gamemap &map_ = get_info().map;
 	LOG_AI << "finding targets...\n";
@@ -171,7 +169,6 @@ bool default_move_to_targets_phase::execute()
 		LOG_AI << "move: " << move.first << " -> " << move.second << '\n';
 
 		move_result_ptr move_ptr = execute_move_action(move.first,move.second,true);
-		gamestate_changed |= move_ptr->is_gamestate_changed();
 
 		if(!move_ptr->is_ok()) {
 			WRN_AI << "unexpected outcome of move"<<std::endl;
@@ -179,7 +176,6 @@ bool default_move_to_targets_phase::execute()
 		}
 	}
 
-	return gamestate_changed;;
 }
 
 

src/ai/testing/ca_default_move_to_targets.hpp

@@ -49,7 +49,7 @@ public:
 
 	virtual double evaluate();
 
-	virtual bool execute();
+	virtual void execute();
 
 protected:
 	void access_points(const move_map& srcdst, const map_location& u, const map_location& dst, std::vector<map_location>& out);

src/ai/testing/ca_testing_move_to_targets.cpp

@@ -126,10 +126,8 @@ double testing_move_to_targets_phase::evaluate()
 }
 
 
-bool testing_move_to_targets_phase::execute()
+void testing_move_to_targets_phase::execute()
 {
-	bool gamestate_changed = false;
-
 	unit_map::const_iterator leader = get_info().units.find_leader(get_side());
 	gamemap &map_ = get_info().map;
 	LOG_AI << "finding targets...\n";
@@ -171,15 +169,11 @@ bool testing_move_to_targets_phase::execute()
 		LOG_AI << "move: " << move.first << " -> " << move.second << '\n';
 
 		move_result_ptr move_ptr = execute_move_action(move.first,move.second,true);
-		gamestate_changed |= move_ptr->is_gamestate_changed();
-
 		if(!move_ptr->is_ok()) {
 			WRN_AI << "unexpected outcome of move"<<std::endl;
 			break;
 		}
 	}
-
-	return gamestate_changed;;
 }
 
 

src/ai/testing/ca_testing_move_to_targets.hpp

@@ -49,7 +49,7 @@ public:
 
 	virtual double evaluate();
 
-	virtual bool execute();
+	virtual void execute();
 
 protected:
 	void access_points(const move_map& srcdst, const map_location& u, const map_location& dst, std::vector<map_location>& out);

src/ai/testing/stage_rca.cpp

@@ -20,6 +20,7 @@
 #include "stage_rca.hpp"
 
 #include "../composite/ai.hpp"
+#include "../gamestate_observer.hpp"
 #include "../../foreach.hpp"
 #include "../../log.hpp"
 
@@ -122,13 +123,14 @@ bool candidate_action_evaluation_loop::do_play_stage()
 		//Execution
 		if (best_score>candidate_action::BAD_SCORE) {
 			DBG_AI_TESTING_RCA_DEFAULT << "Best candidate action: "<< *best_ptr << std::endl;
-			executed = best_ptr->execute();
-			if (!executed) {
+			gamestate_observer gs_o;
+			best_ptr->execute();
+			executed = true;
+			if (!gs_o.is_gamestate_changed()) {
 				//this means that this CA has lied to us in evaluate()
 				//we punish it by disabling it
 				best_ptr->disable();
-				//since we don't re-enable at this play_stage, if we disable this CA, other may get the change to go.
-				executed = true;
+				//since we don't re-enable at this play_stage, if we disable this CA, other may get the chance to go.
 			} else {
 				gamestate_changed = true;
 			}