Skip to content

Commit

Permalink
fixup safebool syntax in halo::handle
Browse files Browse the repository at this point in the history
There was a subtle flaw introduced by using safebool in halo_record,
but defining the handle as a pointer to this -- the pointer also
is castable to bool so it makes mistakes very easy. We replace the
safe bool stuff with a function "valid" of halo_record, and fixup
the places that were supposed to be using this test.

Also move the NO_HALO variable back to the header.

fixes up commit 82c6b98
  • Loading branch information
cbeck88 committed Jun 27, 2014
1 parent 83d9976 commit 6666dca
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 28 deletions.
6 changes: 2 additions & 4 deletions src/halo.cpp
Expand Up @@ -28,8 +28,6 @@
namespace halo
{

const int NO_HALO = 0;

class halo_impl
{

Expand Down Expand Up @@ -522,8 +520,8 @@ halo_record::halo_record(int id, const boost::shared_ptr<halo_impl> & my_manager

halo_record::~halo_record()
{
if (id_ == NO_HALO) return;
if (my_manager_.expired()) return;
if (!valid()) return;

boost::shared_ptr<halo_impl> man = my_manager_.lock();

if (man) {
Expand Down
27 changes: 6 additions & 21 deletions src/halo.hpp
Expand Up @@ -28,6 +28,7 @@ class display;
namespace halo
{


class halo_impl;

class halo_record;
Expand All @@ -36,6 +37,8 @@ typedef boost::shared_ptr<halo_record> handle;

enum ORIENTATION { NORMAL, HREVERSE, VREVERSE, HVREVERSE };

const int NO_HALO = 0;

class manager
{
public:
Expand Down Expand Up @@ -82,33 +85,15 @@ class halo_record : public boost::noncopyable
halo_record(int id, const boost::shared_ptr<halo_impl> & my_manager);
~halo_record();

bool valid() const {
return id_ != NO_HALO && !my_manager_.expired();
}

friend class manager;
private:
int id_;
boost::weak_ptr<halo_impl> my_manager_;

//Begin safe_bool impl

#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 id_ != 0 && !my_manager_.expired(); }
#else
operator safe_bool() const
{ return (id_ != 0 && !my_manager_.expired()) ? &safe_bool_impl::nonnull : NULL; }
#endif

};

} // end namespace halo
Expand Down
7 changes: 4 additions & 3 deletions src/unit_drawer.cpp
Expand Up @@ -161,13 +161,14 @@ void unit_drawer::redraw_unit (const unit & u) const
const int x = static_cast<int>(adjusted_params.offset * xdst + (1.0-adjusted_params.offset) * xsrc) + hex_size_by_2;
const int y = static_cast<int>(adjusted_params.offset * ydst + (1.0-adjusted_params.offset) * ysrc) + hex_size_by_2;

if(!ac.unit_halo_ && !u.image_halo().empty()) {
bool has_halo = ac.unit_halo_ && ac.unit_halo_->valid();
if(!has_halo && !u.image_halo().empty()) {
ac.unit_halo_ = halo_man.add(0, 0, u.image_halo()+u.TC_image_mods(), map_location(-1, -1));
}
if(ac.unit_halo_ && u.image_halo().empty()) {
if(has_halo && u.image_halo().empty()) {
halo_man.remove(ac.unit_halo_);
ac.unit_halo_ = halo::handle(); //halo::NO_HALO;
} else if(ac.unit_halo_) {
} else if(has_halo) {
halo_man.set_location(ac.unit_halo_, x, y - height_adjust);
}

Expand Down

0 comments on commit 6666dca

Please sign in to comment.