From 0f5876fe38067c9783f41ab769e485dd8705e30e Mon Sep 17 00:00:00 2001 From: Chris Beck Date: Wed, 18 Jun 2014 20:26:01 -0400 Subject: [PATCH] change fake_unit class to fake_unit_ptr This is consistent with the introduction of the UnitPtr class. fake_units really aren't different from units, the only difference is their life time / allocation and ownership. Since we are trying to use reference counting for all units (to make them safe to use with animations), the fake units need to be managed by a reference counted pointer also. This is the easiest way to achieve that. I also remove some odd code the [move_units_fake] handler -- there was explicit code to remove the fake units from the fake unit manager, but this is redundant as it is the responsibility of the destructor. Code Blocks and VC project files are updated, but Code::Blocks Scons and Xcode are not. --- projectfiles/CodeBlocks/wesnoth.cbp | 4 +- projectfiles/VC9/wesnoth.vcproj | 8 +-- src/CMakeLists.txt | 2 +- src/SConscript | 2 +- src/actions/move.cpp | 6 +- src/actions/undo.cpp | 4 +- src/fake_unit_manager.cpp | 8 +-- src/fake_unit_manager.hpp | 8 +-- src/{fake_unit.cpp => fake_unit_ptr.cpp} | 56 ++++++++++++++--- src/{fake_unit.hpp => fake_unit_ptr.hpp} | 66 ++++++++++++++------ src/game_display.cpp | 3 +- src/game_events/action_wml.cpp | 32 ++++------ src/mouse_events.cpp | 8 +-- src/unit.cpp | 8 +++ src/unit_display.cpp | 78 +++++++++++------------- src/unit_display.hpp | 16 ++--- src/whiteboard/attack.cpp | 2 +- src/whiteboard/highlighter.cpp | 2 +- src/whiteboard/manager.cpp | 7 +-- src/whiteboard/move.cpp | 7 +-- src/whiteboard/recall.cpp | 10 +-- src/whiteboard/recruit.cpp | 8 +-- src/whiteboard/typedefs.hpp | 3 +- 23 files changed, 208 insertions(+), 140 deletions(-) rename src/{fake_unit.cpp => fake_unit_ptr.cpp} (61%) rename src/{fake_unit.hpp => fake_unit_ptr.hpp} (50%) diff --git a/projectfiles/CodeBlocks/wesnoth.cbp b/projectfiles/CodeBlocks/wesnoth.cbp index 50d11e593580..44b6e44fea8a 100644 --- a/projectfiles/CodeBlocks/wesnoth.cbp +++ b/projectfiles/CodeBlocks/wesnoth.cbp @@ -296,10 +296,10 @@ - - + + diff --git a/projectfiles/VC9/wesnoth.vcproj b/projectfiles/VC9/wesnoth.vcproj index 7fbc2b132e59..153050349f2b 100644 --- a/projectfiles/VC9/wesnoth.vcproj +++ b/projectfiles/VC9/wesnoth.vcproj @@ -20137,19 +20137,19 @@ > get_goto(); // Move the unit. - unit_display::move_unit(rev_route, *u, true, starting_dir); + unit_display::move_unit(rev_route, u.get_shared_ptr(), true, starting_dir); units.move(u->get_location(), rev_route.back()); unit::clear_status_caches(); @@ -957,7 +957,7 @@ bool undo_list::move_action::redo(int side) starting_moves = u->movement_left(); // Move the unit. - unit_display::move_unit(route, *u); + unit_display::move_unit(route, u.get_shared_ptr()); units.move(u->get_location(), route.back()); u = units.find(route.back()); unit::clear_status_caches(); diff --git a/src/fake_unit_manager.cpp b/src/fake_unit_manager.cpp index cafae7b726d8..313dfb5dcff3 100644 --- a/src/fake_unit_manager.cpp +++ b/src/fake_unit_manager.cpp @@ -15,7 +15,7 @@ #include "fake_unit_manager.hpp" #include "display.hpp" -#include "fake_unit.hpp" +#include "fake_unit_ptr.hpp" #include "log.hpp" #include "unit.hpp" #include "unit_animation_component.hpp" @@ -23,7 +23,7 @@ static lg::log_domain log_engine("engine"); #define ERR_NG LOG_STREAM(err, log_engine) -void fake_unit_manager::place_temporary_unit(unit *u) +void fake_unit_manager::place_temporary_unit(internal_ptr_type u) { if(std::find(fake_units_.begin(),fake_units_.end(), u) != fake_units_.end()) { ERR_NG << "In fake_unit_manager::place_temporary_unit: attempt to add duplicate fake unit." << std::endl; @@ -33,10 +33,10 @@ void fake_unit_manager::place_temporary_unit(unit *u) } } -int fake_unit_manager::remove_temporary_unit(unit *u) +int fake_unit_manager::remove_temporary_unit(internal_ptr_type u) { int removed = 0; - std::deque::iterator it = + std::deque::iterator it = std::remove(fake_units_.begin(), fake_units_.end(), u); if (it != fake_units_.end()) { removed = std::distance(it, fake_units_.end()); diff --git a/src/fake_unit_manager.hpp b/src/fake_unit_manager.hpp index eee3f17e31b4..78ec1083aa99 100644 --- a/src/fake_unit_manager.hpp +++ b/src/fake_unit_manager.hpp @@ -21,17 +21,17 @@ class display; class unit; -class fake_unit; +class fake_unit_ptr; class fake_unit_manager { public: fake_unit_manager(display & disp) : fake_units_(), my_display_(disp) {} //Anticipate making place_temporary_unit and remove_temporary_unit private to force exception safety - friend class fake_unit; + friend class fake_unit_ptr; - typedef unit* internal_ptr_type; - typedef boost::iterator_range::const_iterator> range; + typedef unit const * internal_ptr_type; + typedef boost::iterator_range::const_iterator> range; range get_unit_range() const { return boost::make_iterator_range(fake_units_.begin(), fake_units_.end()); } diff --git a/src/fake_unit.cpp b/src/fake_unit_ptr.cpp similarity index 61% rename from src/fake_unit.cpp rename to src/fake_unit_ptr.cpp index 3fbb09442a1c..5a37a83f3c21 100644 --- a/src/fake_unit.cpp +++ b/src/fake_unit_ptr.cpp @@ -12,9 +12,33 @@ See the COPYING file for more details. */ -#include "fake_unit.hpp" +#include "fake_unit_ptr.hpp" #include "fake_unit_manager.hpp" +#include "unit.hpp" +#include "unit_ptr.hpp" +#include "unit_types.hpp" + +#include + +fake_unit_ptr::fake_unit_ptr() : unit_(), my_manager_(NULL) {} +fake_unit_ptr::fake_unit_ptr(const internal_ptr & u) : unit_(u), my_manager_(NULL) {} +fake_unit_ptr::fake_unit_ptr(const internal_ptr & u, fake_unit_manager * mgr) : unit_(u), my_manager_(NULL) +{ + place_on_fake_unit_manager(mgr); +} +fake_unit_ptr::fake_unit_ptr(const fake_unit_ptr & ptr) : unit_(ptr.unit_), my_manager_(NULL) {} + +void fake_unit_ptr::swap (fake_unit_ptr & o) { + boost::swap(unit_, o.unit_); + std::swap(my_manager_, o.my_manager_); +} + +fake_unit_ptr & fake_unit_ptr::operator=(fake_unit_ptr other) { + swap(other); + return *this; +} + /** * Assignment operator, taking a unit. @@ -27,7 +51,7 @@ * The overriding function can be almost the same, except "new (this)" should * be followed by the derived class instead of "fake_unit(a)". */ -fake_unit & fake_unit::operator=(unit const & a) +/*fake_unit & fake_unit::operator=(unit const & a) { if ( this != &a ) { fake_unit_manager * mgr = my_manager_; @@ -41,12 +65,30 @@ fake_unit & fake_unit::operator=(unit const & a) place_on_fake_unit_manager(mgr); } return *this; +}*/ + +void fake_unit_ptr::reset() +{ + remove_from_fake_unit_manager(); + unit_.reset(); +} + +void fake_unit_ptr::reset(const internal_ptr & ptr) +{ + if (unit_.get() != ptr.get()) { + fake_unit_manager * mgr = my_manager_; + + remove_from_fake_unit_manager(); + unit_ = ptr; + if (mgr) + place_on_fake_unit_manager(mgr); + } } /** * Removes @a this from the fake_units_ list if necessary. */ -fake_unit::~fake_unit() +fake_unit_ptr::~fake_unit_ptr() { try { // The fake_unit class exists for this one line, which removes the @@ -62,10 +104,10 @@ fake_unit::~fake_unit() * This will be added at the end (drawn last, over all other units). * Duplicate additions are not allowed. */ -void fake_unit::place_on_fake_unit_manager(fake_unit_manager * manager){ +void fake_unit_ptr::place_on_fake_unit_manager(fake_unit_manager * manager){ assert(my_manager_ == NULL); //Can only be placed on 1 fake_unit_manager my_manager_=manager; - my_manager_->place_temporary_unit(this); + my_manager_->place_temporary_unit(unit_.get()); } /** @@ -73,10 +115,10 @@ void fake_unit::place_on_fake_unit_manager(fake_unit_manager * manager){ * @returns the number of fake_units deleted, which should be 0 or 1 * (any other number indicates an error). */ -int fake_unit::remove_from_fake_unit_manager(){ +int fake_unit_ptr::remove_from_fake_unit_manager(){ int ret(0); if(my_manager_ != NULL){ - ret = my_manager_->remove_temporary_unit(this); + ret = my_manager_->remove_temporary_unit(unit_.get()); my_manager_=NULL; } return ret; diff --git a/src/fake_unit.hpp b/src/fake_unit_ptr.hpp similarity index 50% rename from src/fake_unit.hpp rename to src/fake_unit_ptr.hpp index bf1c3f65b4ab..699a84e14877 100644 --- a/src/fake_unit.hpp +++ b/src/fake_unit_ptr.hpp @@ -15,7 +15,8 @@ #ifndef INCL_FAKE_UNIT_HPP_ #define INCL_FAKE_UNIT_HPP_ -#include "unit.hpp" +#include "unit_ptr.hpp" +#include "unit_types.hpp" class fake_unit_manager; @@ -27,31 +28,62 @@ class fake_unit_manager; The intent is to provide exception safety when the code creating the temp unit is unexpectedly forced out of scope. */ -class fake_unit : public unit { +class fake_unit_ptr { public: - explicit fake_unit(unit const & u) : unit(u), my_manager_(NULL) {ref_count_ = ref_count_ + 1; } //prevent UnitPtr from deleting this - fake_unit(fake_unit const & u) : unit(u), my_manager_(NULL) {ref_count_ = ref_count_ + 1; } - fake_unit(const unit_type& t, int side, unit_race::GENDER gender = unit_race::NUM_GENDERS) - : unit(t, side, false, gender) - , my_manager_(NULL) - {ref_count_ = ref_count_ + 1; } - /// Assignment operator, taking a fake_unit. - /// If already in the queue, @a this will be moved to the end of the - /// queue (drawn last). The queue (if any) of the parameter is ignored. - fake_unit & operator=(fake_unit const & u) - { return operator=(static_cast(u)); } - /// Assignment operator, taking a unit. - virtual fake_unit & operator=(unit const & u); + typedef UnitPtr internal_ptr; + typedef UnitConstPtr internal_const_ptr; + + fake_unit_ptr(); + explicit fake_unit_ptr(const internal_ptr & u); + fake_unit_ptr(const internal_ptr & u, fake_unit_manager * mgr); + fake_unit_ptr(const fake_unit_ptr & ptr); + + void swap (fake_unit_ptr & o); + + fake_unit_ptr & operator=(fake_unit_ptr other); + + void reset(); + void reset(const internal_ptr & ptr); + + internal_ptr operator->() { return unit_; } + internal_const_ptr operator->() const { return unit_; } + + internal_ptr get_unit_ptr() { return unit_; } + internal_const_ptr get_unit_ptr() const { return unit_; } + + unit & operator*() { return *unit_; } + unit * get() { return unit_.get(); } + /// Removes @a this from the fake_units_ list if necessary. - ~fake_unit(); + ~fake_unit_ptr(); /// Place @a this on @a manager's fake_units_ dequeue. void place_on_fake_unit_manager(fake_unit_manager * d); /// Removes @a this from whatever fake_units_ list it is on (if any). int remove_from_fake_unit_manager(); - private : +private : + internal_ptr unit_; fake_unit_manager * my_manager_; + +#ifndef HAVE_CXX11 + struct safe_bool_impl { void nonnull() {} }; + /** + * Used as t he return type of the conversion operator for boolean contexts. + * Needed, since the compiler would otherwise consider the following + * conversion (C legacy): cfg["abc"] -> "abc"[bool(cfg)] -> 'b' + */ + typedef void (safe_bool_impl::*safe_bool)(); +#endif + +public: +#ifdef HAVE_CXX11 + explicit operator bool() const + { return unit_; } +#else + operator safe_bool() const + { return unit_ ? &safe_bool_impl::nonnull : NULL; } +#endif }; #endif diff --git a/src/game_display.cpp b/src/game_display.cpp index 0b835baf40e1..b96ea697bdd9 100644 --- a/src/game_display.cpp +++ b/src/game_display.cpp @@ -35,8 +35,8 @@ Growl_Delegate growl_obj; #include "cursor.hpp" #include "drawable_unit.hpp" -#include "fake_unit.hpp" #include "fake_unit_manager.hpp" +#include "fake_unit_ptr.hpp" #include "game_board.hpp" #include "game_preferences.hpp" #include "halo.hpp" @@ -48,6 +48,7 @@ Growl_Delegate growl_obj; #include "resources.hpp" #include "tod_manager.hpp" #include "sound.hpp" +#include "unit.hpp" #include "whiteboard/manager.hpp" #ifdef _WIN32 #include "windows_tray_notification.hpp" diff --git a/src/game_events/action_wml.cpp b/src/game_events/action_wml.cpp index 102324455c01..0f462523d8b9 100644 --- a/src/game_events/action_wml.cpp +++ b/src/game_events/action_wml.cpp @@ -30,8 +30,8 @@ #include "../actions/vision.hpp" #include "../ai/manager.hpp" #include "../dialogs.hpp" -#include "../fake_unit.hpp" #include "../fake_unit_manager.hpp" +#include "../fake_unit_ptr.hpp" #include "../game_classification.hpp" #include "../game_display.hpp" #include "../game_preferences.hpp" @@ -226,7 +226,7 @@ namespace { // Support functions return map_location(x, y); } - fake_unit *create_fake_unit(const vconfig& cfg) + fake_unit_ptr create_fake_unit(const vconfig& cfg) { std::string type = cfg["type"]; std::string variation = cfg["variation"]; @@ -238,8 +238,8 @@ namespace { // Support functions unit_race::GENDER gender = string_gender(cfg["gender"]); const unit_type *ut = unit_types.find(type); - if (!ut) return NULL; - fake_unit * fake = new fake_unit(*ut, side_num, gender); + if (!ut) return fake_unit_ptr(); + fake_unit_ptr fake = fake_unit_ptr(UnitPtr(new unit(*ut, side_num, gender))); if(!variation.empty()) { config mod; @@ -1326,7 +1326,7 @@ WML_HANDLER_FUNCTION(modify_turns, /*event_info*/, cfg) /// that is just moving for the visual effect WML_HANDLER_FUNCTION(move_unit_fake, /*event_info*/, cfg) { - util::unique_ptr dummy_unit(create_fake_unit(cfg)); + fake_unit_ptr dummy_unit(create_fake_unit(cfg)); if(!dummy_unit.get()) return; @@ -1341,7 +1341,7 @@ WML_HANDLER_FUNCTION(move_unit_fake, /*event_info*/, cfg) const std::vector& path = fake_unit_path(*dummy_unit, xvals, yvals); if (!path.empty()) { // Always scroll. - unit_display::move_unit(path, *dummy_unit, true, map_location::NDIRECTIONS, force_scroll); + unit_display::move_unit(path, dummy_unit.get_unit_ptr(), true, map_location::NDIRECTIONS, force_scroll); } } @@ -1351,8 +1351,8 @@ WML_HANDLER_FUNCTION(move_units_fake, /*event_info*/, cfg) const vconfig::child_list unit_cfgs = cfg.get_children("fake_unit"); size_t num_units = unit_cfgs.size(); - boost::scoped_array > units( - new util::unique_ptr[num_units]); + std::vector units; + units.reserve(num_units); std::vector > paths; paths.reserve(num_units); @@ -1364,8 +1364,8 @@ WML_HANDLER_FUNCTION(move_units_fake, /*event_info*/, cfg) const std::vector xvals = utils::split(config["x"]); const std::vector yvals = utils::split(config["y"]); int skip_steps = config["skip_steps"]; - fake_unit *u = create_fake_unit(config); - units[paths.size()].reset(u); + fake_unit_ptr u = create_fake_unit(config); + units[paths.size()] = u; paths.push_back(fake_unit_path(*u, xvals, yvals)); if(skip_steps > 0) paths.back().insert(paths.back().begin(), skip_steps, paths.back().front()); @@ -1373,7 +1373,7 @@ WML_HANDLER_FUNCTION(move_units_fake, /*event_info*/, cfg) DBG_NG << "Path " << paths.size() - 1 << " has length " << paths.back().size() << '\n'; u->set_location(paths.back().front()); - u->place_on_fake_unit_manager(resources::fake_units); + u.place_on_fake_unit_manager(resources::fake_units); } LOG_NG << "Units placed, longest path is " << longest_path << " long\n"; @@ -1388,19 +1388,13 @@ WML_HANDLER_FUNCTION(move_units_fake, /*event_info*/, cfg) DBG_NG << "Moving unit " << un << ", doing step " << step << '\n'; path_step[0] = paths[un][step - 1]; path_step[1] = paths[un][step]; - unit_display::move_unit(path_step, *units[un]); + unit_display::move_unit(path_step, units[un].get_unit_ptr()); units[un]->set_location(path_step[1]); units[un]->anim_comp().set_standing(); } } LOG_NG << "Units moved\n"; - - for(size_t un = 0; un < num_units; ++un) { - units[un]->remove_from_fake_unit_manager(); - } - - LOG_NG << "Units removed\n"; } WML_HANDLER_FUNCTION(object, event_info, cfg) @@ -2321,7 +2315,7 @@ WML_HANDLER_FUNCTION(teleport, event_info, cfg) teleport_path.push_back(src_loc); teleport_path.push_back(vacant_dst); bool animate = cfg["animate"].to_bool(); - unit_display::move_unit(teleport_path, *u, animate); + unit_display::move_unit(teleport_path, u.get_shared_ptr(), animate); resources::units->move(src_loc, vacant_dst); unit::clear_status_caches(); diff --git a/src/mouse_events.cpp b/src/mouse_events.cpp index 93f96be210f0..e46d5e73da0f 100644 --- a/src/mouse_events.cpp +++ b/src/mouse_events.cpp @@ -215,7 +215,7 @@ void mouse_handler::mouse_motion(int x, int y, const bool browse, bool update, m gui().clear_attack_indicator(); } - unit* un; //will later point to unit at mouseover_hex_ + UnitPtr un; //will later point to unit at mouseover_hex_ // the destination is the pointed hex or the adjacent hex // used to attack it @@ -271,9 +271,9 @@ void mouse_handler::mouse_motion(int x, int y, const bool browse, bool update, m unit_map::iterator iter = mouseover_unit; if (iter != board_.units_.end()) - un = &*iter; + un = iter.get_shared_ptr(); else - un = NULL; + un.reset(); } //end planned unit map scope if ( (!selected_hex_.valid()) && un && current_paths_.destinations.empty() && @@ -286,7 +286,7 @@ void mouse_handler::mouse_motion(int x, int y, const bool browse, bool update, m pathfind::marked_route route; { // start planned unit map scope wb::future_map_if_active raii; - route = get_route(un, go_to, current_team()); + route = get_route(un.get(), go_to, current_team()); } // end planned unit map scope gui().set_route(&route); } diff --git a/src/unit.cpp b/src/unit.cpp index 06ea00fe6056..31aa94824ad6 100644 --- a/src/unit.cpp +++ b/src/unit.cpp @@ -86,6 +86,10 @@ void intrusive_ptr_add_ref(const unit * u) // or if you are sure that the refcounting system works // then feel free to remove the next line assert(u->ref_count_ < 100000); + LOG_UT << "Adding a reference to a unit: id = " << u->id() << ", uid = " << u->underlying_id() << ", refcount = " << u->ref_count() << " ptr:" << u << std::endl; + if (u->ref_count_ == 0) { + LOG_UT << "Freshly constructed" << std::endl; + } ++(u->ref_count_); } @@ -93,8 +97,12 @@ void intrusive_ptr_release(const unit * u) { assert(u->ref_count_ >= 1); assert(u->ref_count_ < 100000); //See comment in intrusive_ptr_add_ref + LOG_UT << "Removing a reference to a unit: id = " << u->id() << ", uid = " << u->underlying_id() << ", refcount = " << u->ref_count() << " ptr:" << u << std::endl; if (--(u->ref_count_) == 0) + { + LOG_UT << "Deleting a unit: id = " << u->id() << ", uid = " << u->underlying_id() << std::endl; delete u; + } } /** diff --git a/src/unit_display.cpp b/src/unit_display.cpp index 3ccbe3c4c7a3..83946efb455a 100644 --- a/src/unit_display.cpp +++ b/src/unit_display.cpp @@ -17,8 +17,8 @@ #include "global.hpp" #include "unit_display.hpp" -#include "fake_unit.hpp" #include "fake_unit_manager.hpp" +#include "fake_unit_ptr.hpp" #include "game_board.hpp" #include "game_display.hpp" #include "game_preferences.hpp" @@ -116,7 +116,7 @@ static void teleport_unit_between(const map_location& a, const map_location& b, * INT_MIN indicates that no animation is pending. */ static int move_unit_between(const map_location& a, const map_location& b, - unit& temp_unit, unsigned int step_num, + UnitPtr temp_unit, unsigned int step_num, unsigned int step_left, unit_animator & animator, display& disp) { @@ -124,10 +124,10 @@ static int move_unit_between(const map_location& a, const map_location& b, return INT_MIN; } - temp_unit.set_location(a); + temp_unit->set_location(a); disp.invalidate(a); - temp_unit.set_facing(a.get_relative_dir(b)); - animator.replace_anim_if_invalid(&temp_unit,"movement",a,b,step_num, + temp_unit->set_facing(a.get_relative_dir(b)); + animator.replace_anim_if_invalid(temp_unit.get(),"movement",a,b,step_num, false,"",0,unit_animation::INVALID,NULL,NULL,step_left); animator.start_animations(); animator.pause_animation(); @@ -165,10 +165,10 @@ unit_mover::unit_mover(const std::vector& path, bool animate, bool force_scroll_(force_scroll), animator_(), wait_until_(INT_MIN), - shown_unit_(NULL), + shown_unit_(), path_(path), current_(0), - temp_unit_ptr_(NULL), + temp_unit_ptr_(), // Somewhat arbitrary default values. was_hidden_(false), is_enemy_(true) @@ -187,8 +187,6 @@ unit_mover::~unit_mover() update_shown_unit(); // For safety, clear the animator before deleting the temp unit. animator_.clear(); - // The temp unit's destructor will remove it from the display. - delete temp_unit_ptr_; } @@ -202,28 +200,24 @@ unit_mover::~unit_mover() /* Note: Hide the unit in its current location; do not actually remove it. * Otherwise the status displays will be wrong during the movement. */ -void unit_mover::replace_temporary(unit & u) +void unit_mover::replace_temporary(UnitPtr u) { if ( disp_ == NULL ) // No point in creating a temp unit with no way to display it. return; // Save the hidden state of the unit. - was_hidden_ = u.get_hidden(); + was_hidden_ = u->get_hidden(); // Make our temporary unit mostly match u... - if ( temp_unit_ptr_ == NULL ) { - temp_unit_ptr_ = new fake_unit(u); - temp_unit_ptr_->place_on_fake_unit_manager(resources::fake_units); - } - else - *temp_unit_ptr_ = u; + temp_unit_ptr_ = fake_unit_ptr(UnitPtr(new unit(*u)), resources::fake_units); + // ... but keep the temporary unhidden and hide the original. temp_unit_ptr_->set_hidden(false); - u.set_hidden(true); + u->set_hidden(true); // Update cached data. - is_enemy_ = (*resources::teams)[u.side()-1].is_enemy(disp_->viewing_side()); + is_enemy_ = (*resources::teams)[u->side()-1].is_enemy(disp_->viewing_side()); } @@ -234,11 +228,11 @@ void unit_mover::replace_temporary(unit & u) */ void unit_mover::update_shown_unit() { - if ( shown_unit_ != NULL ) { + if ( shown_unit_ ) { // Switch the display back to the real unit. shown_unit_->set_hidden(was_hidden_); temp_unit_ptr_->set_hidden(true); - shown_unit_ = NULL; + shown_unit_.reset(); } } @@ -247,15 +241,15 @@ void unit_mover::update_shown_unit() * Initiates the display of movement for the supplied unit. * This should be called before attempting to display moving to a new hex. */ -void unit_mover::start(unit& u) +void unit_mover::start(UnitPtr u) { // Nothing to do here if there is nothing to animate. if ( !can_draw_ ) return; // If no animation then hide unit until end of movement if ( !animate_ ) { - was_hidden_ = u.get_hidden(); - u.set_hidden(true); + was_hidden_ = u->get_hidden(); + u->set_hidden(true); return; } @@ -294,15 +288,15 @@ void unit_mover::start(unit& u) disp_->draw(true); // extra immobile movement animation for take-off - animator_.add_animation(temp_unit_ptr_, "pre_movement", path_[0], path_[1]); + animator_.add_animation(temp_unit_ptr_.get(), "pre_movement", path_[0], path_[1]); animator_.start_animations(); animator_.wait_for_end(); animator_.clear(); // Switch the display back to the real unit. - u.set_facing(temp_unit_ptr_->facing()); - u.anim_comp().set_standing(false); // Need to reset u's animation so the new facing takes effect. - u.set_hidden(was_hidden_); + u->set_facing(temp_unit_ptr_->facing()); + u->anim_comp().set_standing(false); // Need to reset u's animation so the new facing takes effect. + u->set_hidden(was_hidden_); temp_unit_ptr_->set_hidden(true); } @@ -318,7 +312,7 @@ void unit_mover::start(unit& u) * wait (another call to proceed_to() or finish() will implicitly wait). The * unit must remain valid until the wait is finished. */ -void unit_mover::proceed_to(unit& u, size_t path_index, bool update, bool wait) +void unit_mover::proceed_to(UnitPtr u, size_t path_index, bool update, bool wait) { // Nothing to do here if animations cannot be shown. if ( !can_draw_ || !animate_ ) @@ -327,14 +321,14 @@ void unit_mover::proceed_to(unit& u, size_t path_index, bool update, bool wait) // Handle pending visibility issues before introducing new ones. wait_for_anims(); - if ( update || temp_unit_ptr_ == NULL ) + if ( update || !temp_unit_ptr_ ) // Replace the temp unit (which also hides u and shows our temporary). replace_temporary(u); else { // Just switch the display from the real unit to our fake one. temp_unit_ptr_->set_hidden(false); - u.set_hidden(true); + u->set_hidden(true); } // Safety check. @@ -367,7 +361,7 @@ void unit_mover::proceed_to(unit& u, size_t path_index, bool update, bool wait) if ( tiles_adjacent(path_[current_], path_[current_+1]) ) wait_until_ = move_unit_between(path_[current_], path_[current_+1], - *temp_unit_ptr_, current_, + temp_unit_ptr_.get_unit_ptr(), current_, path_.size() - (current_+2), animator_, *disp_); else if ( path_[current_] != path_[current_+1] ) @@ -376,10 +370,10 @@ void unit_mover::proceed_to(unit& u, size_t path_index, bool update, bool wait) } // Update the unit's facing. - u.set_facing(temp_unit_ptr_->facing()); - u.anim_comp().set_standing(false); // Need to reset u's animation so the new facing takes effect. + u->set_facing(temp_unit_ptr_->facing()); + u->anim_comp().set_standing(false); // Need to reset u's animation so the new facing takes effect. // Remember the unit to unhide when the animation finishes. - shown_unit_ = &u; + shown_unit_ = u; if ( wait ) wait_for_anims(); } @@ -417,7 +411,7 @@ void unit_mover::wait_for_anims() * If @a dir is not supplied, the final direction will be determined by (the * last two traversed hexes of) the path. */ -void unit_mover::finish(unit &u, map_location::DIRECTION dir) +void unit_mover::finish(UnitPtr u, map_location::DIRECTION dir) { // Nothing to do here if the display is not valid. if ( !can_draw_ ) @@ -438,13 +432,13 @@ void unit_mover::finish(unit &u, map_location::DIRECTION dir) temp_unit_ptr_->set_facing(final_dir); // Animation - animator_.add_animation(temp_unit_ptr_, "post_movement", end_loc); + animator_.add_animation(temp_unit_ptr_.get(), "post_movement", end_loc); animator_.start_animations(); animator_.wait_for_end(); animator_.clear(); // Switch the display back to the real unit. - u.set_hidden(was_hidden_); + u->set_hidden(was_hidden_); temp_unit_ptr_->set_hidden(true); events::mouse_handler* mousehandler = events::mouse_handler::get_singleton(); @@ -455,12 +449,12 @@ void unit_mover::finish(unit &u, map_location::DIRECTION dir) else { // Show the unit at end of skipped animation - u.set_hidden(was_hidden_); + u->set_hidden(was_hidden_); } // Facing gets set even when not animating. - u.set_facing(dir == map_location::NDIRECTIONS ? final_dir : dir); - u.anim_comp().set_standing(true); // Need to reset u's animation so the new facing takes effect. + u->set_facing(dir == map_location::NDIRECTIONS ? final_dir : dir); + u->anim_comp().set_standing(true); // Need to reset u's animation so the new facing takes effect. // Redraw path ends (even if not animating). disp_->invalidate(path_.front()); @@ -484,7 +478,7 @@ void unit_mover::finish(unit &u, map_location::DIRECTION dir) * so that while the unit is moving status etc. * will still display the correct number of units. */ -void move_unit(const std::vector& path, unit& u, +void move_unit(const std::vector& path, UnitPtr u, bool animate, map_location::DIRECTION dir, bool force_scroll) { diff --git a/src/unit_display.hpp b/src/unit_display.hpp index 15d45fa14d38..c39a2584d5bc 100644 --- a/src/unit_display.hpp +++ b/src/unit_display.hpp @@ -20,11 +20,11 @@ #ifndef UNIT_DISPLAY_HPP_INCLUDED #define UNIT_DISPLAY_HPP_INCLUDED +#include "fake_unit_ptr.hpp" #include "map_location.hpp" #include "unit_animation.hpp" class attack_type; -class fake_unit; class fake_unit_manager; class game_board; class game_display; @@ -49,13 +49,13 @@ class unit_mover : public boost::noncopyable { explicit unit_mover(const std::vector& path, bool animate=true, bool force_scroll=false); ~unit_mover(); - void start(unit& u); - void proceed_to(unit& u, size_t path_index, bool update=false, bool wait=true); + void start(UnitPtr u); + void proceed_to(UnitPtr u, size_t path_index, bool update=false, bool wait=true); void wait_for_anims(); - void finish(unit &u, map_location::DIRECTION dir = map_location::NDIRECTIONS); + void finish(UnitPtr u, map_location::DIRECTION dir = map_location::NDIRECTIONS); private: // functions - void replace_temporary(unit & u); + void replace_temporary(UnitPtr u); void update_shown_unit(); private: // data @@ -65,10 +65,10 @@ class unit_mover : public boost::noncopyable { const bool force_scroll_; unit_animator animator_; int wait_until_; /// The animation potential to wait until. INT_MIN for no wait; INT_MAX to wait for end. - unit * shown_unit_; /// The unit to be (re-)shown after an animation finishes. + UnitPtr shown_unit_; /// The unit to be (re-)shown after an animation finishes. const std::vector& path_; size_t current_; - fake_unit * temp_unit_ptr_; + fake_unit_ptr temp_unit_ptr_; bool was_hidden_; bool is_enemy_; }; @@ -77,7 +77,7 @@ class unit_mover : public boost::noncopyable { /** * Display a unit moving along a given path. */ -void move_unit(const std::vector& path, unit& u, +void move_unit(const std::vector& path, UnitPtr u, bool animate=true, map_location::DIRECTION dir=map_location::NDIRECTIONS, bool force_scroll=false); diff --git a/src/whiteboard/attack.cpp b/src/whiteboard/attack.cpp index dff4cc065c60..d1c88c72b5c7 100644 --- a/src/whiteboard/attack.cpp +++ b/src/whiteboard/attack.cpp @@ -23,7 +23,7 @@ #include "arrow.hpp" #include "config.hpp" -#include "fake_unit.hpp" +#include "fake_unit_ptr.hpp" #include "game_board.hpp" #include "play_controller.hpp" #include "resources.hpp" diff --git a/src/whiteboard/highlighter.cpp b/src/whiteboard/highlighter.cpp index 18516117e156..7084ddf664ea 100644 --- a/src/whiteboard/highlighter.cpp +++ b/src/whiteboard/highlighter.cpp @@ -35,7 +35,7 @@ #include "arrow.hpp" #include "config.hpp" -#include "fake_unit.hpp" +#include "fake_unit_ptr.hpp" #include "game_board.hpp" #include "game_display.hpp" #include "game_errors.hpp" diff --git a/src/whiteboard/manager.cpp b/src/whiteboard/manager.cpp index ba2c00284391..34de296da111 100644 --- a/src/whiteboard/manager.cpp +++ b/src/whiteboard/manager.cpp @@ -32,8 +32,8 @@ #include "actions/undo.hpp" #include "arrow.hpp" #include "chat_events.hpp" -#include "fake_unit.hpp" #include "fake_unit_manager.hpp" +#include "fake_unit_ptr.hpp" #include "formula_string_utils.hpp" #include "game_board.hpp" #include "game_preferences.hpp" @@ -714,12 +714,11 @@ void manager::create_temp_move() if(!fake_unit) { // Create temp ghost unit - fake_unit.reset(new class fake_unit(*temp_moved_unit)); - fake_unit->place_on_fake_unit_manager( resources::fake_units); + fake_unit = fake_unit_ptr(UnitPtr (new unit(*temp_moved_unit)), resources::fake_units); fake_unit->anim_comp().set_ghosted(true); } - unit_display::move_unit(path, *fake_unit, false); //get facing right + unit_display::move_unit(path, fake_unit.get_unit_ptr(), false); //get facing right fake_unit->anim_comp().invalidate(*game_display::get_singleton()); fake_unit->set_location(*curr_itor); fake_unit->anim_comp().set_ghosted(true); diff --git a/src/whiteboard/move.cpp b/src/whiteboard/move.cpp index e6ded93e559a..ca4ca92e5df9 100644 --- a/src/whiteboard/move.cpp +++ b/src/whiteboard/move.cpp @@ -25,8 +25,8 @@ #include "arrow.hpp" #include "config.hpp" -#include "fake_unit.hpp" #include "fake_unit_manager.hpp" +#include "fake_unit_ptr.hpp" #include "game_board.hpp" #include "game_end_exceptions.hpp" #include "mouse_events.hpp" @@ -132,12 +132,11 @@ move::move(config const& cfg, bool hidden) arrow_->set_path(route_->steps); // Construct fake_unit_ - fake_unit_.reset(new fake_unit(*get_unit()) ); + fake_unit_ = fake_unit_ptr( UnitPtr(new unit(*get_unit())) , resources::fake_units ); if(hidden) fake_unit_->set_hidden(true); - fake_unit_->place_on_fake_unit_manager(resources::fake_units); fake_unit_->anim_comp().set_ghosted(true); - unit_display::move_unit(route_->steps, *fake_unit_, false); //get facing right + unit_display::move_unit(route_->steps, fake_unit_.get_unit_ptr(), false); //get facing right fake_unit_->set_location(route_->steps.back()); this->init(); diff --git a/src/whiteboard/recall.cpp b/src/whiteboard/recall.cpp index d0d7ae2ab8e1..f055c2b30245 100644 --- a/src/whiteboard/recall.cpp +++ b/src/whiteboard/recall.cpp @@ -24,8 +24,8 @@ #include "visitor.hpp" #include "actions/create.hpp" -#include "fake_unit.hpp" #include "fake_unit_manager.hpp" +#include "fake_unit_ptr.hpp" #include "game_display.hpp" #include "recall_list_manager.hpp" #include "resources.hpp" @@ -62,7 +62,7 @@ recall::recall(size_t team_index, bool hidden, const unit& unit, const map_locat action(team_index,hidden), temp_unit_(new class unit(unit)), recall_hex_(recall_hex), - fake_unit_(new fake_unit(unit) ) + fake_unit_(UnitPtr( new class unit(unit) ) ) { this->init(); } @@ -79,7 +79,7 @@ recall::recall(config const& cfg, bool hidden) { if(recall_unit->underlying_id()==underlying_id) { - temp_unit_.reset(new unit(*recall_unit)); //TODO: is it necessary to make a copy? + temp_unit_.reset(new class unit(*recall_unit)); //TODO: is it necessary to make a copy? break; } } @@ -87,7 +87,7 @@ recall::recall(config const& cfg, bool hidden) throw action::ctor_err("recall: Invalid underlying_id"); } - fake_unit_.reset(new fake_unit(*temp_unit_)); //makes copy of temp_unit_ + fake_unit_.reset(UnitPtr(new class unit(*temp_unit_))); //makes copy of temp_unit_ this->init(); } @@ -101,7 +101,7 @@ void recall::init() fake_unit_->set_movement(0, true); fake_unit_->set_attacks(0); fake_unit_->anim_comp().set_ghosted(false); - fake_unit_->place_on_fake_unit_manager( resources::fake_units); + fake_unit_.place_on_fake_unit_manager( resources::fake_units); } recall::~recall() diff --git a/src/whiteboard/recruit.cpp b/src/whiteboard/recruit.cpp index 2c1feaa6ec8c..339a214d6b4a 100644 --- a/src/whiteboard/recruit.cpp +++ b/src/whiteboard/recruit.cpp @@ -23,8 +23,8 @@ #include "utility.hpp" #include "visitor.hpp" -#include "fake_unit.hpp" #include "fake_unit_manager.hpp" +#include "fake_unit_ptr.hpp" #include "menu_events.hpp" #include "play_controller.hpp" #include "resources.hpp" @@ -58,7 +58,7 @@ recruit::recruit(size_t team_index, bool hidden, const std::string& unit_name, c unit_name_(unit_name), recruit_hex_(recruit_hex), temp_unit_(create_corresponding_unit()), //auto-ptr ownership transfer - fake_unit_(new fake_unit(*temp_unit_)), //temp_unit_ *copied* into new fake unit + fake_unit_(UnitPtr(new unit(*temp_unit_))), //temp_unit_ *copied* into new fake unit cost_(0) { this->init(); @@ -78,7 +78,7 @@ recruit::recruit(config const& cfg, bool hidden) // Construct temp_unit_ and fake_unit_ temp_unit_ = create_corresponding_unit(); //auto-ptr ownership transfer - fake_unit_.reset(new class fake_unit(*temp_unit_)), //temp_unit_ copied into new fake_unit + fake_unit_.reset(UnitPtr (new unit(*temp_unit_))), //temp_unit_ copied into new fake_unit this->init(); } @@ -89,7 +89,7 @@ void recruit::init() fake_unit_->set_movement(0, true); fake_unit_->set_attacks(0); fake_unit_->anim_comp().set_ghosted(false); - fake_unit_->place_on_fake_unit_manager(resources::fake_units); + fake_unit_.place_on_fake_unit_manager(resources::fake_units); cost_ = fake_unit_->type().cost(); } diff --git a/src/whiteboard/typedefs.hpp b/src/whiteboard/typedefs.hpp index 7626166289d8..6535e37f1bdb 100644 --- a/src/whiteboard/typedefs.hpp +++ b/src/whiteboard/typedefs.hpp @@ -37,11 +37,11 @@ static lg::log_domain log_whiteboard("whiteboard"); #include #include +#include "../fake_unit_ptr.hpp" #include "../unit_ptr.hpp" class arrow; class config; -class fake_unit; class fake_unit_manager; class game_board; struct map_location; //not used in the typedefs, saves a few forward declarations @@ -66,7 +66,6 @@ class side_actions; typedef boost::shared_ptr whiteboard_lock; typedef boost::shared_ptr arrow_ptr; -typedef boost::shared_ptr fake_unit_ptr; typedef boost::shared_ptr action_ptr; typedef boost::shared_ptr action_const_ptr;