Skip to content

Commit

Permalink
change fake_unit class to fake_unit_ptr
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
cbeck88 committed Jun 23, 2014
1 parent e803a9d commit 0f5876f
Show file tree
Hide file tree
Showing 23 changed files with 208 additions and 140 deletions.
4 changes: 2 additions & 2 deletions projectfiles/CodeBlocks/wesnoth.cbp
Expand Up @@ -296,10 +296,10 @@
<Unit filename="..\..\src\events.cpp" />
<Unit filename="..\..\src\events.hpp" />
<Unit filename="..\..\src\exceptions.hpp" />
<Unit filename="..\..\src\fake_unit.cpp" />
<Unit filename="..\..\src\fake_unit.hpp" />
<Unit filename="..\..\src\fake_unit_manager.cpp" />
<Unit filename="..\..\src\fake_unit_manager.hpp" />
<Unit filename="..\..\src\fake_unit_ptr.cpp" />
<Unit filename="..\..\src\fake_unit_ptr.hpp" />
<Unit filename="..\..\src\filechooser.cpp" />
<Unit filename="..\..\src\filechooser.hpp" />
<Unit filename="..\..\src\filesystem.cpp" />
Expand Down
8 changes: 4 additions & 4 deletions projectfiles/VC9/wesnoth.vcproj
Expand Up @@ -20137,19 +20137,19 @@
>
</File>
<File
RelativePath="..\..\src\fake_unit.cpp"
RelativePath="..\..\src\fake_unit_manager.cpp"
>
</File>
<File
RelativePath="..\..\src\fake_unit.hpp"
RelativePath="..\..\src\fake_unit_manager.hpp"
>
</File>
<File
RelativePath="..\..\src\fake_unit_manager.cpp"
RelativePath="..\..\src\fake_unit_ptr.cpp"
>
</File>
<File
RelativePath="..\..\src\fake_unit_manager.hpp"
RelativePath="..\..\src\fake_unit_ptr.hpp"
>
</File>
<File
Expand Down
2 changes: 1 addition & 1 deletion src/CMakeLists.txt
Expand Up @@ -729,8 +729,8 @@ set(wesnoth-main_SRC
editor/editor_preferences.cpp
editor/toolkit/editor_toolkit.cpp
editor/map/context_manager.cpp
fake_unit.cpp
fake_unit_manager.cpp
fake_unit_ptr.cpp
filechooser.cpp
flg_manager.cpp
floating_textbox.cpp
Expand Down
2 changes: 1 addition & 1 deletion src/SConscript
Expand Up @@ -262,8 +262,8 @@ wesnoth_sources = Split("""
editor/editor_preferences.cpp
editor/toolkit/brush.cpp
editor/toolkit/editor_toolkit.cpp
fake_unit.cpp
fake_unit_manager.cpp
fake_unit_ptr.cpp
filechooser.cpp
flg_manager.cpp
floating_textbox.cpp
Expand Down
6 changes: 3 additions & 3 deletions src/actions/move.cpp
Expand Up @@ -513,7 +513,7 @@ namespace { // Private helpers for move_unit()

// Show this move.
const size_t current_tracking = game_events::wml_tracking();
animator.proceed_to(*move_it_, step_to - begin_,
animator.proceed_to(move_it_.get_shared_ptr(), step_to - begin_,
current_tracking != do_move_track_, false);
do_move_track_ = current_tracking;
disp.redraw_minimap();
Expand Down Expand Up @@ -942,7 +942,7 @@ namespace { // Private helpers for move_unit()

// Prepare to animate.
unit_display::unit_mover animator(route_, show);
animator.start(*move_it_);
animator.start(move_it_.get_shared_ptr());

// Traverse the route to the hex where we need to stop.
// Each iteration performs the move from real_end_-1 to real_end_.
Expand Down Expand Up @@ -1003,7 +1003,7 @@ namespace { // Private helpers for move_unit()

if ( move_it_.valid() ) {
// Finish animating.
animator.finish(*move_it_);
animator.finish(move_it_.get_shared_ptr());
// Check for the moving unit being seen.
event_mutated_ = actor_sighted(*move_it_, &not_seeing);
}
Expand Down
4 changes: 2 additions & 2 deletions src/actions/undo.cpp
Expand Up @@ -740,7 +740,7 @@ bool undo_list::move_action::undo(int side, undo_list & /*undos*/)
goto_hex = u->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();

Expand Down Expand Up @@ -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();
Expand Down
8 changes: 4 additions & 4 deletions src/fake_unit_manager.cpp
Expand Up @@ -15,15 +15,15 @@
#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"

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;
Expand All @@ -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<unit*>::iterator it =
std::deque<internal_ptr_type>::iterator it =
std::remove(fake_units_.begin(), fake_units_.end(), u);
if (it != fake_units_.end()) {
removed = std::distance(it, fake_units_.end());
Expand Down
8 changes: 4 additions & 4 deletions src/fake_unit_manager.hpp
Expand Up @@ -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<std::deque<unit*>::const_iterator> range;
typedef unit const * internal_ptr_type;
typedef boost::iterator_range<std::deque<internal_ptr_type>::const_iterator> range;

range get_unit_range() const { return boost::make_iterator_range(fake_units_.begin(), fake_units_.end()); }

Expand Down
56 changes: 49 additions & 7 deletions src/fake_unit.cpp → src/fake_unit_ptr.cpp
Expand Up @@ -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 <boost/swap.hpp>

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.
Expand All @@ -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_;
Expand All @@ -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
Expand All @@ -62,21 +104,21 @@ 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());
}

/**
* Removes @a this from whatever fake_units_ list it is on (if any).
* @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;
Expand Down
66 changes: 49 additions & 17 deletions src/fake_unit.hpp → src/fake_unit_ptr.hpp
Expand Up @@ -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;

Expand All @@ -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<unit const &>(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
3 changes: 2 additions & 1 deletion src/game_display.cpp
Expand Up @@ -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"
Expand All @@ -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"
Expand Down

0 comments on commit 0f5876f

Please sign in to comment.