mail[Wesnoth-commits] r52731 - in /trunk: ./ src/whiteboard/


Others Months | Index by Date | Thread Index
>>   [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Header


Content

Posted by gabrielmorin on January 22, 2012 - 00:16:
Author: gabba
Date: Sun Jan 22 00:16:05 2012
New Revision: 52731

URL: http://svn.gna.org/viewcvs/wesnoth?rev=52731&view=rev
Log:
Move and Suppose_Dead actions: Instead of keeping a pointer around, store the 
underlying id of the owner unit and use that to find it when needed.
Fixes bug #19222

Modified:
    trunk/changelog
    trunk/players_changelog
    trunk/src/whiteboard/manager.cpp
    trunk/src/whiteboard/move.cpp
    trunk/src/whiteboard/move.hpp
    trunk/src/whiteboard/suppose_dead.cpp
    trunk/src/whiteboard/suppose_dead.hpp
    trunk/src/whiteboard/validate_visitor.cpp

Modified: trunk/changelog
URL: 
http://svn.gna.org/viewcvs/wesnoth/trunk/changelog?rev=52731&r1=52730&r2=52731&view=diff
==============================================================================
--- trunk/changelog (original)
+++ trunk/changelog Sun Jan 22 00:16:05 2012
@@ -44,6 +44,7 @@
      [replace_map] or [terrain_mask]
    * Disable wml menu items in linger mode without debug mode (bug #16262)
  * Whiteboard:
+   * Fixed bug #19222 : After 'delete planned action', the unit is almost 
invisible
    * Fixed bug #19221 : Assert when a whiteboard move-attack wins a scenario
    * Fixed bug #19142 : attacks can be simulated between units (for which 
this
      shouldn't be possible)

Modified: trunk/players_changelog
URL: 
http://svn.gna.org/viewcvs/wesnoth/trunk/players_changelog?rev=52731&r1=52730&r2=52731&view=diff
==============================================================================
--- trunk/players_changelog (original)
+++ trunk/players_changelog Sun Jan 22 00:16:05 2012
@@ -23,6 +23,7 @@
      turn time runs out in attack selection dialog.
      
  * Whiteboard:
+   * Fixed bug #19222 : After 'delete planned action', the unit is almost 
invisible
    * Fixed bug #19221 : Assert when a whiteboard move-attack wins a scenario
    * Fixed bug #19142 : attacks can be simulated between units (for which 
this
      shouldn't be possible)

Modified: trunk/src/whiteboard/manager.cpp
URL: 
http://svn.gna.org/viewcvs/wesnoth/trunk/src/whiteboard/manager.cpp?rev=52731&r1=52730&r2=52731&view=diff
==============================================================================
--- trunk/src/whiteboard/manager.cpp (original)
+++ trunk/src/whiteboard/manager.cpp Sun Jan 22 00:16:05 2012
@@ -306,18 +306,13 @@
                        && boost::dynamic_pointer_cast<move>(action)
                        && 
viewer_actions()->count_actions_of(action->get_unit()) == 1)
        {
-               //but first, check that the unit's still around and that it's 
the same one
-               unit_map::iterator unit_it =
-                               
resources::units->find(boost::static_pointer_cast<move>(action)->get_dest_hex());
-               if (unit_it != resources::units->end() && &*unit_it == 
action->get_unit()) {
-                       action->get_unit()->set_standing(true);
-               }
+               action->get_unit()->set_standing(true);
        }
 }
 
 void manager::post_delete_action(action_ptr action)
 {
-       // The ghost of the last fake unit in a chain of planned actions is 
supposed to look different
+       // The fake unit representing the destination of a chain of planned 
moves should have the regular animation.
        // If the last remaining action of the unit that owned this move is a 
move as well,
        // adjust its appearance accordingly.
 
@@ -329,7 +324,7 @@
                if (move_ptr move = boost::dynamic_pointer_cast<class 
move>(*action_it))
                {
                        if (move->get_fake_unit())
-                               move->get_fake_unit()->set_ghosted(true);
+                               move->get_fake_unit()->set_standing(true);
                }
        }
 }

Modified: trunk/src/whiteboard/move.cpp
URL: 
http://svn.gna.org/viewcvs/wesnoth/trunk/src/whiteboard/move.cpp?rev=52731&r1=52730&r2=52731&view=diff
==============================================================================
--- trunk/src/whiteboard/move.cpp (original)
+++ trunk/src/whiteboard/move.cpp Sun Jan 22 00:16:05 2012
@@ -61,7 +61,7 @@
 move::move(size_t team_index, bool hidden, unit& u, const 
pathfind::marked_route& route,
                arrow_ptr arrow, fake_unit_ptr fake_unit)
 : action(team_index,hidden),
-  unit_(&u),
+  unit_underlying_id_(u.underlying_id()),
   unit_id_(),
   route_(new pathfind::marked_route(route)),
   movement_cost_(0),
@@ -84,7 +84,7 @@
 
 move::move(config const& cfg, bool hidden)
        : action(cfg,hidden)
-       , unit_()
+       , unit_underlying_id_(0)
        , unit_id_()
        , route_(new pathfind::marked_route())
        , movement_cost_(0)
@@ -98,10 +98,10 @@
        , fake_unit_hidden_(false)
 {
        // Construct and validate unit_
-       unit_map::iterator unit_itor = resources::units->find(cfg["unit_"]);
+       unit_map::iterator unit_itor = 
resources::units->find(cfg["underlying_id"]);
        if(unit_itor == resources::units->end())
                throw action::ctor_err("move: Invalid underlying_id");
-       unit_ = &*unit_itor;
+       unit_underlying_id_ = unit_itor->underlying_id();
 
        // Construct and validate route_
        config const& route_cfg = cfg.child("route_");
@@ -127,7 +127,7 @@
        arrow_->set_path(route_->steps);
 
        // Construct fake_unit_
-       fake_unit_.reset(new game_display::fake_unit(*unit_) );
+       fake_unit_.reset(new game_display::fake_unit(*get_unit()) );
        if(hidden)
                fake_unit_->set_hidden(true);
        fake_unit_->place_on_game_display(resources::screen);
@@ -140,8 +140,8 @@
 
 void move::init()
 {
-       assert(unit_);
-       unit_id_ = unit_->id();
+       assert(get_unit());
+       unit_id_ = get_unit()->id();
 
        //This action defines the future position of the unit, make its fake 
unit more visible
        //than previous actions' fake units
@@ -150,7 +150,7 @@
                fake_unit_->set_ghosted(true);
        }
        side_actions_ptr side_actions = 
resources::teams->at(team_index()).get_side_actions();
-       side_actions::iterator action = 
side_actions->find_last_action_of(unit_);
+       side_actions::iterator action = 
side_actions->find_last_action_of(get_unit());
        if (action != side_actions->end())
        {
                if (move_ptr move = boost::dynamic_pointer_cast<class 
move>(*action))
@@ -214,7 +214,7 @@
 
        set_arrow_brightness(ARROW_BRIGHTNESS_HIGHLIGHTED);
        hide_fake_unit();
-       unghost_owner_unit(unit_);
+       unghost_owner_unit(get_unit());
 
        events::mouse_handler& mouse_handler = 
resources::controller->get_mouse_handler_base();
        std::set<map_location> adj_enemies = 
mouse_handler.get_adj_enemies(get_dest_hex(), side_number());
@@ -304,14 +304,23 @@
        {
                set_arrow_brightness(ARROW_BRIGHTNESS_STANDARD);
                show_fake_unit();
-               ghost_owner_unit(unit_);
+               ghost_owner_unit(get_unit());
        }
 
        //if unit has other moves besides this one, set it back to ghosted 
visuals
        //@todo handle this in a more centralized fashion
-       if 
(resources::teams->at(team_index()).get_side_actions()->count_actions_of(unit_)
1) {
-               ghost_owner_unit(unit_);
-       }
+       if 
(resources::teams->at(team_index()).get_side_actions()->count_actions_of(get_unit())
1) {
+               ghost_owner_unit(get_unit());
+       }
+}
+
+unit* move::get_unit() const
+{
+       unit_map::iterator itor = resources::units->find(unit_underlying_id_);
+       if (itor.valid())
+               return &*itor;
+       else
+               return NULL;
 }
 
 map_location move::get_source_hex() const
@@ -457,13 +466,12 @@
        }
 }
 
-///@todo Find a better way to serialize unit_ because underlying_id isn't 
cutting it
 config move::to_config() const
 {
        config final_cfg = action::to_config();
 
        final_cfg["type"]="move";
-       final_cfg["unit_"]=static_cast<int>(unit_->underlying_id());
+       final_cfg["underlying_id"]=static_cast<int>(unit_underlying_id_);
 //     final_cfg["movement_cost_"]=movement_cost_; //Unnecessary
 //     final_cfg["unit_id_"]=unit_id_; //Unnecessary
 
@@ -496,7 +504,7 @@
 
 void move::calculate_move_cost()
 {
-       assert(unit_);
+       assert(get_unit());
        assert(route_);
        if (get_source_hex().valid() && get_dest_hex().valid() && 
get_source_hex() != get_dest_hex())
        {
@@ -507,7 +515,7 @@
                        WRN_WB << "Move defined with insufficient movement 
left.\n";
                }
 
-               // If unit finishes move in a village it captures, set the 
move cost to unit_.movement_left()
+               // If unit finishes move in a village it captures, set the 
move cost to unit's movement_left()
                 if (route_->marks[get_dest_hex()].capture)
                 {
                         movement_cost_ = get_unit()->movement_left();

Modified: trunk/src/whiteboard/move.hpp
URL: 
http://svn.gna.org/viewcvs/wesnoth/trunk/src/whiteboard/move.hpp?rev=52731&r1=52730&r2=52731&view=diff
==============================================================================
--- trunk/src/whiteboard/move.hpp (original)
+++ trunk/src/whiteboard/move.hpp Sun Jan 22 00:16:05 2012
@@ -49,7 +49,7 @@
        virtual void execute(bool& success, bool& complete);
 
        /** Return the unit targeted by this action. Null if unit doesn't 
exist. */
-       virtual unit* get_unit() const { return unit_; }
+       virtual unit* get_unit() const;
        /** @return pointer to the fake unit used only for visuals */
        virtual fake_unit_ptr get_fake_unit() { return fake_unit_; }
 
@@ -95,7 +95,7 @@
 
        void calculate_move_cost();
 
-       unit* unit_;
+       size_t unit_underlying_id_;
        std::string unit_id_;
        boost::scoped_ptr<pathfind::marked_route> route_;
        int movement_cost_;

Modified: trunk/src/whiteboard/suppose_dead.cpp
URL: 
http://svn.gna.org/viewcvs/wesnoth/trunk/src/whiteboard/suppose_dead.cpp?rev=52731&r1=52730&r2=52731&view=diff
==============================================================================
--- trunk/src/whiteboard/suppose_dead.cpp (original)
+++ trunk/src/whiteboard/suppose_dead.cpp Sun Jan 22 00:16:05 2012
@@ -62,7 +62,7 @@
 
        suppose_dead::suppose_dead(size_t team_index, bool hidden, unit& 
curr_unit, map_location const& loc)
        : action(team_index,hidden)
-       , unit_(&curr_unit)
+       , unit_underlying_id_(curr_unit.underlying_id())
        , unit_id_(curr_unit.id())
        , loc_(loc)
        , valid_(true)
@@ -72,20 +72,18 @@
 
        suppose_dead::suppose_dead(config const& cfg, bool hidden)
        : action(cfg,hidden)
-       , unit_()
+       , unit_underlying_id_(0)
        , unit_id_()
        , loc_(cfg.child("loc_")["x"],cfg.child("loc_")["y"])
        , valid_(true)
        {
                // Construct and validate unit_
-               unit_map::iterator unit_itor = 
resources::units->find(cfg["unit_"]);
+               unit_map::iterator unit_itor = 
resources::units->find(cfg["underlying_id"]);
                if(unit_itor == resources::units->end())
                        throw action::ctor_err("suppose_dead: Invalid 
underlying_id");
-               unit_ = &*unit_itor;
 
-               /// @todo Why do we even have the unit_id_ field?
-               // Construct unit_id_
-               unit_id_ = unit_->id();
+               unit_underlying_id_ = unit_itor->underlying_id();
+               unit_id_ = unit_itor->id();
 
                this->init();
        }
@@ -100,6 +98,15 @@
                //invalidate hex so that skull indicator is properly cleared
                if(resources::screen)
                        resources::screen->invalidate(loc_);
+       }
+
+       unit* suppose_dead::get_unit() const
+       {
+               unit_map::iterator itor = 
resources::units->find(unit_underlying_id_);
+               if (itor.valid())
+                       return &*itor;
+               else
+                       return NULL;
        }
 
        void suppose_dead::accept(visitor& v)
@@ -118,7 +125,7 @@
                                << "] from (" << loc_ << ")\n";
 
                // Just check to make sure we removed the unit we expected to 
remove
-               assert(unit_ == removed_unit);
+               assert(get_unit() == removed_unit);
        }
 
        void suppose_dead::remove_temp_modifier(unit_map& unit_map)
@@ -128,7 +135,7 @@
                assert(unit_it == resources::units->end());
 
                // Restore the unit
-               unit_map.insert(unit_);
+               unit_map.insert(get_unit());
        }
 
        void suppose_dead::draw_hex(const map_location& hex)
@@ -156,7 +163,7 @@
                config final_cfg = action::to_config();
 
                final_cfg["type"]="suppose_dead";
-               final_cfg["unit_"]=static_cast<int>(unit_->underlying_id());
+               
final_cfg["underlying_id"]=static_cast<int>(unit_underlying_id_);
                final_cfg["unit_id_"]=unit_id_;
 
                config loc_cfg;

Modified: trunk/src/whiteboard/suppose_dead.hpp
URL: 
http://svn.gna.org/viewcvs/wesnoth/trunk/src/whiteboard/suppose_dead.hpp?rev=52731&r1=52730&r2=52731&view=diff
==============================================================================
--- trunk/src/whiteboard/suppose_dead.hpp (original)
+++ trunk/src/whiteboard/suppose_dead.hpp Sun Jan 22 00:16:05 2012
@@ -41,7 +41,7 @@
                virtual ~suppose_dead();
 
                /** Return the unit targeted by this action. Null if unit 
doesn't exist. */
-               virtual unit* get_unit() const { return unit_; }
+               virtual unit* get_unit() const;
                /** @return null pointer */
                virtual fake_unit_ptr get_fake_unit() { return 
fake_unit_ptr(); }
                /** Return the location at which this action was planned. */
@@ -77,7 +77,7 @@
                        return 
boost::static_pointer_cast<suppose_dead>(action::shared_from_this());
                }
 
-               unit* unit_;
+               size_t unit_underlying_id_;
                std::string unit_id_;
                map_location loc_;
 

Modified: trunk/src/whiteboard/validate_visitor.cpp
URL: 
http://svn.gna.org/viewcvs/wesnoth/trunk/src/whiteboard/validate_visitor.cpp?rev=52731&r1=52730&r2=52731&view=diff
==============================================================================
--- trunk/src/whiteboard/validate_visitor.cpp (original)
+++ trunk/src/whiteboard/validate_visitor.cpp Sun Jan 22 00:16:05 2012
@@ -90,12 +90,8 @@
 
        //check if the unit in the source hex has the same unit id as before,
        //i.e. that it's the same unit
-       if (m.unit_id_ != unit_it->id())
+       if (m.unit_id_ != unit_it->id() || m.unit_underlying_id_ != 
unit_it->underlying_id())
                return WORTHLESS;
-
-       //Now that we've reliably identified the unit owning this planned 
move, update the
-       //pointer in case there has been some funny business in the unit map
-       m.unit_ = &*unit_it;
 
        //check that the path is good
        if (m.get_source_hex() != m.get_dest_hex()) //skip zero-hex move used 
by attack subclass
@@ -301,7 +297,6 @@
                if(unit_it == resources::units->end())
                {
                        sup_d->set_valid(false);
-                       sup_d->unit_ = NULL;
                }
        }
 
@@ -310,7 +305,6 @@
        if(sup_d->valid_ && sup_d->unit_id_ != unit_it->id())
        {
                sup_d->set_valid(false);
-               sup_d->unit_ = NULL;
        }
 
        if(!sup_d->valid_)




Related Messages


Powered by MHonArc, Updated Sun Jan 22 00:20:05 2012