0ct0pu5/wesnoth
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Move and Suppose_Dead actions: Instead of keeping a pointer around,

Browse source 
...store the underlying id of the owner unit and use that to find it
when needed.

Fixes bug #19222
This commit is contained in:
Gabriel Morin 2012-01-21 23:16:05 +00:00
parent 5e819ce7b8
commit 5ededa2d37
8 changed files with 51 additions and 45 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
changelog
View file

@ -44,6 +44,7 @@ Version 1.9.14+svn:
[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)

1
players_changelog
View file

@ -23,6 +23,7 @@ Version 1.9.14+svn:
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)

11
src/whiteboard/manager.cpp
View file

@ -306,18 +306,13 @@ void manager::pre_delete_action(action_ptr action)
&& 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 @@ void manager::post_delete_action(action_ptr action)
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);
}
}
}

40
src/whiteboard/move.cpp
View file

@ -61,7 +61,7 @@ std::ostream& move::print(std::ostream &s) const
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(size_t team_index, bool hidden, unit& u, const pathfind::marked_route
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 @@ move::move(config const& cfg, bool hidden)
, 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 @@ move::move(config const& cfg, bool hidden)
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 @@ move::move(config const& cfg, bool hidden)
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 @@ void move::init()
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 @@ void move::execute(bool& success, bool& complete)
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,16 +304,25 @@ void move::execute(bool& success, bool& complete)
{
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
{
assert(route_ && !route_->steps.empty());
@ -457,13 +466,12 @@ void move::set_valid(bool valid)
}
}
///@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 @@ config move::to_config() const
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 @@ void move::calculate_move_cost()
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();

4
src/whiteboard/move.hpp
View file

@ -49,7 +49,7 @@ public:
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 @@ protected:
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_;

27
src/whiteboard/suppose_dead.cpp
View file

@ -62,7 +62,7 @@ namespace wb
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 @@ namespace wb
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();
}
@ -102,6 +100,15 @@ namespace wb
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)
{
v.visit(shared_from_this());
@ -118,7 +125,7 @@ namespace wb
<< "] 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 @@ namespace wb
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 @@ namespace wb
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;

4
src/whiteboard/suppose_dead.hpp
View file

@ -41,7 +41,7 @@ namespace wb {
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 @@ namespace wb {
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_;

8
src/whiteboard/validate_visitor.cpp
View file

@ -90,13 +90,9 @@ validate_visitor::VALIDITY validate_visitor::evaluate_move_validity(move_ptr m_p
//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 @@ void validate_visitor::visit(suppose_dead_ptr sup_d)
if(unit_it == resources::units->end())
{
sup_d->set_valid(false);
sup_d->unit_ = NULL;
}
}
@ -310,7 +305,6 @@ void validate_visitor::visit(suppose_dead_ptr sup_d)
if(sup_d->valid_ && sup_d->unit_id_ != unit_it->id())
{
sup_d->set_valid(false);
sup_d->unit_ = NULL;
}
if(!sup_d->valid_)