From a23eeebe36d39684f95cc421273ccd5a7251fecf Mon Sep 17 00:00:00 2001
From: Gabriel Morin <gabrielmorin@gmail.com>
Date: Sat, 12 Jun 2010 06:04:35 +0000
Subject: [PATCH] Whiteboard:

- moved to std::auto_ptr for some variables to ensure proper deletion
  on manager "death".

- fixed infamous off-by-one error with the team indexes

- made manager noncopyable

- use more forward declarations and less includes in manager.hpp
---
 src/whiteboard/manager.cpp      | 32 ++++++++++++++++++++++----------
 src/whiteboard/manager.hpp      | 15 +++++++++------
 src/whiteboard/move.cpp         |  5 ++++-
 src/whiteboard/side_actions.cpp |  6 +-----
 4 files changed, 36 insertions(+), 22 deletions(-)

diff --git a/src/whiteboard/manager.cpp b/src/whiteboard/manager.cpp
index 6d1cb1cf33e..666b6c4df80 100644
--- a/src/whiteboard/manager.cpp
+++ b/src/whiteboard/manager.cpp
@@ -19,6 +19,7 @@
 #include "manager.hpp"
 
 #include "action.hpp"
+#include "mapbuilder_visitor.hpp"
 
 #include "arrow.hpp"
 #include "foreach.hpp"
@@ -28,17 +29,28 @@
 
 namespace wb {
 
-manager::manager(): active_(false), move_arrow_(NULL)
+manager::manager():
+		active_(false),
+		mapbuilder_(NULL),
+		move_arrow_(NULL),
+		fake_unit_(NULL)
+{
+}
+
+manager::~manager()
 {
 }
 
 void manager::apply_temp_modifiers()
 {
 	mapbuilder_.reset(new mapbuilder_visitor(*resources::units));
-	team& current_team = (*resources::teams)[resources::controller->current_side()];
-	const action_set& actions = current_team.get_side_actions().actions();
-	foreach (action_ptr action, actions)
+	int current_side = resources::controller->current_side();
+	team& current_team = (*resources::teams)[current_side - 1];
+	side_actions& side_actions = current_team.get_side_actions();
+	const action_set& actions = side_actions.actions();
+	foreach (const action_ptr &action, actions)
 	{
+		assert(action);
 		action->accept(*mapbuilder_);
 	}
 }
@@ -54,10 +66,10 @@ void manager::set_route(const std::vector<map_location> &steps)
 	route_ = steps;
 	if (route_.size() > 1)
 	{
-		if (move_arrow_ == NULL)
+		if (move_arrow_.get() == NULL)
 		{
 			display *screen = (display*) resources::screen;
-			move_arrow_ = new arrow(screen);
+			move_arrow_.reset(new arrow(screen));
 			move_arrow_->set_color("white");
 			move_arrow_->set_alpha(0.5);
 			screen->add_arrow(*move_arrow_);
@@ -70,7 +82,7 @@ void manager::set_route(const std::vector<map_location> &steps)
 void manager::create_move_from_route(unit& subject)
 {
 	int current_side = resources::controller->current_side();
-	team& current_team = (*resources::teams)[current_side];
+	team& current_team = (*resources::teams)[current_side - 1];
 
 	LOG_WB << "Creating move for unit " << subject.name() << " [" << subject.id() << "]"
 			<< " from " << subject.get_location()
@@ -78,9 +90,9 @@ void manager::create_move_from_route(unit& subject)
 
 	move_arrow_->set_color(team::get_side_color_index(current_side));
 
-	current_team.get_side_actions().queue_move(subject, route_.back(), *move_arrow_);
-	//ownership of the arrow transferred to the new move action
-	move_arrow_ = NULL;
+	current_team.get_side_actions().queue_move(subject, route_.back(),
+			*(move_arrow_.release()) /* ownership of the arrow transferred to the new move action */);
+
 }
 
 } // end namespace wb
diff --git a/src/whiteboard/manager.hpp b/src/whiteboard/manager.hpp
index 402954af84b..c935b02c438 100644
--- a/src/whiteboard/manager.hpp
+++ b/src/whiteboard/manager.hpp
@@ -19,12 +19,11 @@
 #ifndef WB_MANAGER_HPP_
 #define WB_MANAGER_HPP_
 
-#include "mapbuilder_visitor.hpp"
-
 #include "map_location.hpp"
 
-#include <boost/scoped_ptr.hpp>
+#include <boost/noncopyable.hpp>
 
+#include <memory>
 #include <vector>
 
 class arrow;
@@ -32,14 +31,17 @@ class unit;
 
 namespace wb {
 
+class mapbuilder_visitor;
+
 /**
  * This class holds and manages all of the whiteboard's planned actions.
  */
-class manager
+class manager : private boost::noncopyable
 {
 public:
 
 	manager();
+	~manager();
 
 	/**
 	 * Determine whether the whiteboard is activated.
@@ -70,11 +72,12 @@ private:
 	 */
 	bool active_;
 
-	boost::scoped_ptr<mapbuilder_visitor> mapbuilder_;
+	std::auto_ptr<mapbuilder_visitor> mapbuilder_;
 
 	std::vector<map_location> route_;
 
-	arrow* move_arrow_;
+	std::auto_ptr<arrow> move_arrow_;
+	std::auto_ptr<unit> fake_unit_;
 };
 
 } // end namespace wb
diff --git a/src/whiteboard/move.cpp b/src/whiteboard/move.cpp
index 7f931db3f86..e8e267b97fa 100644
--- a/src/whiteboard/move.cpp
+++ b/src/whiteboard/move.cpp
@@ -27,7 +27,10 @@
 namespace wb {
 
 move::move(unit& subject, const map_location& target_hex, arrow& arrow)
-: unit_(subject), orig_hex_(subject.get_location()), dest_hex_(target_hex), arrow_(arrow)
+: unit_(subject),
+  orig_hex_(subject.get_location()),
+  dest_hex_(target_hex),
+  arrow_(arrow)
 {
 }
 
diff --git a/src/whiteboard/side_actions.cpp b/src/whiteboard/side_actions.cpp
index 3a97aff80a3..bc2a64a245d 100644
--- a/src/whiteboard/side_actions.cpp
+++ b/src/whiteboard/side_actions.cpp
@@ -23,15 +23,12 @@
 namespace wb
 {
 
-side_actions::side_actions()
+side_actions::side_actions(): actions_()
 {
-	// TODO Auto-generated constructor stub
-
 }
 
 side_actions::~side_actions()
 {
-	// TODO Auto-generated destructor stub
 }
 
 const action_set& side_actions::actions() const
@@ -44,7 +41,6 @@ void side_actions::insert_move(unit& subject, const map_location& target_hex, ar
 	action_ptr action(new move(subject, target_hex, arrow));
 	assert(index < end());
 	actions_.insert(actions_.begin() + index, action);
-
 }
 
 void side_actions::queue_move(unit& subject, const map_location& target_hex, arrow& arrow)