Skip to content

Commit

Permalink
partially fix 23076 (don't hold std::vector<unit> in recall dialog)
Browse files Browse the repository at this point in the history 
After this commit 9e7dc5b which
introduced reference counted unit pointers, the recall dialog
began to fail assertions. The problem was that it was making
private copies of all units, and when units get deleted this list
got out of sync. I'm not totally sure exactly what mechanism was
making it work before, but this commit which shares the units list
between the different gui elements prevents the assertion failure.

Note that there is still a bug, in that the unit preview pane at
left still gets out of sync somehow...
  • Loading branch information
@cbeck88
cbeck88 committed Dec 28, 2014
1 parent 8539cb7 commit 924fb82
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 41 deletions.
40 changes: 20 additions & 20 deletions src/actions/attack.cpp
View file
Expand Up @@ -1528,34 +1528,34 @@ void attack_unit_and_advance(const map_location &attacker, const map_location &d
}


unit get_advanced_unit(const unit &u, const std::string& advance_to)
unit_ptr get_advanced_unit(const unit &u, const std::string& advance_to)
{
const unit_type *new_type = unit_types.find(advance_to);
if (!new_type) {
throw game::game_error("Could not find the unit being advanced"
" to: " + advance_to);
}
unit new_unit(u);
new_unit.set_experience(new_unit.experience() - new_unit.max_experience());
new_unit.advance_to(*new_type);
new_unit.heal_all();
new_unit.set_state(unit::STATE_POISONED, false);
new_unit.set_state(unit::STATE_SLOWED, false);
new_unit.set_state(unit::STATE_PETRIFIED, false);
new_unit.set_user_end_turn(false);
new_unit.set_hidden(false);
unit_ptr new_unit(new unit(u));
new_unit->set_experience(new_unit->experience() - new_unit->max_experience());
new_unit->advance_to(*new_type);
new_unit->heal_all();
new_unit->set_state(unit::STATE_POISONED, false);
new_unit->set_state(unit::STATE_SLOWED, false);
new_unit->set_state(unit::STATE_PETRIFIED, false);
new_unit->set_user_end_turn(false);
new_unit->set_hidden(false);
return new_unit;
}


/**
* Returns the AMLA-advanced version of a unit (with traits and items retained).
*/
unit get_amla_unit(const unit &u, const config &mod_option)
unit_ptr get_amla_unit(const unit &u, const config &mod_option)
{
unit amla_unit(u);
amla_unit.set_experience(amla_unit.experience() - amla_unit.max_experience());
amla_unit.add_modification("advance", mod_option);
unit_ptr amla_unit(new unit(u));
amla_unit->set_experience(amla_unit->experience() - amla_unit->max_experience());
amla_unit->add_modification("advance", mod_option);
return amla_unit;
}

Expand Down Expand Up @@ -1593,19 +1593,19 @@ void advance_unit(map_location loc, const std::string &advance_to,

// Create the advanced unit.
bool use_amla = mod_option != NULL;
unit new_unit = use_amla ? get_amla_unit(*u, *mod_option) :
unit_ptr new_unit = use_amla ? get_amla_unit(*u, *mod_option) :
get_advanced_unit(*u, advance_to);
if ( !use_amla )
{
statistics::advance_unit(new_unit);
preferences::encountered_units().insert(new_unit.type_id());
LOG_CF << "Added '" << new_unit.type_id() << "' to the encountered units.\n";
statistics::advance_unit(*new_unit);
preferences::encountered_units().insert(new_unit->type_id());
LOG_CF << "Added '" << new_unit->type_id() << "' to the encountered units.\n";
}
u = resources::units->replace(loc, new_unit).first;
u = resources::units->replace(loc, *new_unit).first;

// Update fog/shroud.
actions::shroud_clearer clearer;
clearer.clear_unit(loc, new_unit);
clearer.clear_unit(loc, *new_unit);

// "post_advance" event.
if(fire_event)
Expand Down
4 changes: 2 additions & 2 deletions src/actions/attack.hpp
View file
Expand Up @@ -230,12 +230,12 @@ void advance_unit_at(const advance_unit_params& params);
/**
* Returns the advanced version of a unit (with traits and items retained).
*/
unit get_advanced_unit(const unit &u, const std::string &advance_to);
unit_ptr get_advanced_unit(const unit &u, const std::string &advance_to);

/**
* Returns the AMLA-advanced version of a unit (with traits and items retained).
*/
unit get_amla_unit(const unit &u, const config &mod_option);
unit_ptr get_amla_unit(const unit &u, const config &mod_option);

/**
* Function which will advance the unit at @a loc to 'advance_to'.
Expand Down
26 changes: 10 additions & 16 deletions src/dialogs.cpp
View file
Expand Up @@ -91,13 +91,13 @@ namespace
class delete_recall_unit : public gui::dialog_button_action
{
public:
delete_recall_unit(display& disp, gui::filter_textbox& filter, const std::vector<unit_const_ptr >& units) : disp_(disp), filter_(filter), units_(units) {}
delete_recall_unit(display& disp, gui::filter_textbox& filter, std::vector<unit_const_ptr >& units) : disp_(disp), filter_(filter), units_(units) {}
private:
gui::dialog_button_action::RESULT button_pressed(int menu_selection);

display& disp_;
gui::filter_textbox& filter_;
const std::vector<unit_const_ptr >& units_;
std::vector<unit_const_ptr >& units_;
};

template<typename T> void dump(const T & units)
Expand Down Expand Up @@ -144,6 +144,9 @@ gui::dialog_button_action::RESULT delete_recall_unit::button_pressed(int menu_se
return gui::CONTINUE_DIALOG;
}
}
// Remove the item from our dialog's list
units_.erase(units_.begin() + index);

// Remove the item from filter_textbox memory
filter_.delete_item(menu_selection);
//add dismissal to the undo stack
Expand Down Expand Up @@ -178,10 +181,10 @@ int advance_unit_dialog(const map_location &loc)

std::vector<std::string> lang_options;

std::vector<unit> sample_units;
std::vector<unit_const_ptr> sample_units;
for(std::vector<std::string>::const_iterator op = options.begin(); op != options.end(); ++op) {
sample_units.push_back(::get_advanced_unit(*u, *op));
const unit& type = sample_units.back();
const unit& type = *sample_units.back();

#ifdef LOW_MEM
lang_options.push_back(IMAGE_PREFIX
Expand All @@ -198,7 +201,7 @@ int advance_unit_dialog(const map_location &loc)
{
if (mod["always_display"].to_bool()) always_display = true;
sample_units.push_back(::get_amla_unit(*u, mod));
const unit& type = sample_units.back();
const unit& type = *sample_units.back();
if (!mod["image"].empty()) {
lang_options.push_back(IMAGE_PREFIX + mod["image"].str() + COLUMN_SEPARATOR + mod["description"].str());
} else {
Expand Down Expand Up @@ -313,7 +316,7 @@ void show_unit_list(display& gui)
items.push_back(heading);

std::vector<map_location> locations_list;
std::vector<unit> units_list;
std::vector<unit_const_ptr> units_list;

int selected = 0;

Expand Down Expand Up @@ -395,7 +398,7 @@ void show_unit_list(display& gui)
items.push_back(row.str());

locations_list.push_back(i->get_location());
units_list.push_back(*i);
units_list.push_back(i.get_shared_ptr());
}

{
Expand Down Expand Up @@ -1394,15 +1397,6 @@ units_list_preview_pane::units_list_preview_pane(const std::vector<unit_const_pt
{
}

units_list_preview_pane::units_list_preview_pane(const std::vector<unit> &units,
const gui::filter_textbox* filter, TYPE type, bool on_left_side) :
unit_preview_pane(filter, type, on_left_side),
units_(units.size())
{
for (unsigned i = 0; i < units.size(); ++i)
units_[i] = unit_const_ptr (new unit(units[i]));
}

size_t units_list_preview_pane::size() const
{
return units_.size();
Expand Down
3 changes: 0 additions & 3 deletions src/dialogs.hpp
View file
Expand Up @@ -119,9 +119,6 @@ class units_list_preview_pane : public dialogs::unit_preview_pane
units_list_preview_pane(const std::vector<unit_const_ptr > &units,
const gui::filter_textbox *filter = NULL,
TYPE type = SHOW_ALL, bool left_side = true);
units_list_preview_pane(const std::vector<unit> &units,
const gui::filter_textbox *filter = NULL,
TYPE type = SHOW_ALL, bool left_side = true);

private:
size_t size() const;
Expand Down

0 comments on commit 924fb82

Please sign in to comment.