Skip to content

Commit

Permalink
Remove code from replay_controller that's already handled by the hotk…
Browse files Browse the repository at this point in the history 
…ey system

Fixes a crash when playing a replay that includes a theme, when that theme doesn't
have all of the buttons that the removed code expected. For example, this replay
from SXRPG 5.2.3 has two replay themes, of which one lacks button-nextmove:
* https://replays.wesnoth.org/1.14/20190701/SXRPG_TempleOfBones_Turn_8_(53897).bz2

All of the button::enable() calls were unnecessary logic, because the hotkey
system will query replay_controller::can_execute_command() and enable or
disable the buttons to match. However, the enable() calls appear to have had
the side-effect of triggering the hotkey system to check the buttons' required
states and redraw.

The code in build_replay_theme() was unreachable, because replay_controller
doesn't attach itself to the completely_redrawn event.
  • Loading branch information
@stevecotton
stevecotton committed Jul 28, 2019
1 parent 37f02d8 commit e4cbfa2
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 155 deletions.
161 changes: 23 additions & 138 deletions src/replay_controller.cpp
View file
Expand Up @@ -102,178 +102,55 @@ void replay_controller::add_replay_theme()
}
}


void replay_controller::rebuild_replay_theme()
{
const config &theme_cfg = controller_.get_theme(game_config_manager::get()->game_config(), controller_.theme());
if (const config &res = theme_cfg.child("resolution"))
{
if (const config &replay_theme_cfg = res.child("replay")) {
controller_.get_display().get_theme().modify(replay_theme_cfg);
}
controller_.get_display().get_theme().modify_label("time-icon", _ ("current local time"));
//Make sure we get notified if the theme is redrawn completely. That way we have
//a chance to restore the replay controls of the theme as well.
controller_.get_display().invalidate_theme();
}
}

std::shared_ptr<gui::button> replay_controller::play_button()
{
return controller_.get_display().find_action_button("button-playreplay");
}

std::shared_ptr<gui::button> replay_controller::stop_button()
{
return controller_.get_display().find_action_button("button-stopreplay");
}

std::shared_ptr<gui::button> replay_controller::reset_button()
{
return controller_.get_display().find_action_button("button-resetreplay");
}

std::shared_ptr<gui::button> replay_controller::play_turn_button()
{
return controller_.get_display().find_action_button("button-nextturn");
}

std::shared_ptr<gui::button> replay_controller::play_side_button()
{
return controller_.get_display().find_action_button("button-nextside");
}

std::shared_ptr<gui::button> replay_controller::play_move_button()
{
return controller_.get_display().find_action_button("button-nextmove");
}

void replay_controller::update_replay_ui()
{
//check if we have all buttons - if someone messed with theme then some buttons may be missing
//if any of the buttons is missing, we just disable every one
if(!replay_ui_has_all_buttons()) {
std::shared_ptr<gui::button> play_b = play_button(), stop_b = stop_button(),
reset_b = reset_button(), play_turn_b = play_turn_button(),
play_side_b = play_side_button(), play_move_b = play_move_button();

if(play_b) {
play_b->enable(false);
}

if(stop_b) {
stop_b->enable(false);
}

if(reset_b) {
reset_b->enable(false);
}

if(play_turn_b) {
play_turn_b->enable(false);
}

if(play_side_b) {
play_side_b->enable(false);
}

if (play_move_b) {
play_move_b->enable(false);
}
}
}

void replay_controller::replay_ui_playback_should_start()
{
if(!replay_ui_has_all_buttons())
return;

play_button()->enable(false);
reset_button()->enable(false);
play_turn_button()->enable(false);
play_side_button()->enable(false);
play_move_button()->enable(false);
}

void replay_controller::replay_ui_playback_should_stop()
{
if(!replay_ui_has_all_buttons())
return;

if(!resources::recorder->at_end()) {
play_button()->enable(true);
reset_button()->enable(true);
play_turn_button()->enable(true);
play_side_button()->enable(true);
play_move_button()->enable(true);

play_button()->release();
play_turn_button()->release();
play_side_button()->release();
play_move_button()->release();
} else {
reset_button()->enable(true);
stop_button()->enable(false);
}

if(stop_condition_->should_stop()) {
//user interrupted
stop_button()->release();
}
}

void replay_controller::reset_replay_ui()
{
if(!replay_ui_has_all_buttons())
return;

play_button()->enable(true);
stop_button()->enable(true);
reset_button()->enable(true);
play_turn_button()->enable(true);
play_side_button()->enable(true);
}


void replay_controller::stop_replay()
{
stop_condition_.reset(new replay_stop_condition());
update_enabled_buttons();
}

void replay_controller::replay_next_turn()
{
stop_condition_.reset(new replay_play_turn(controller_.gamestate().tod_manager_.turn()));
update_enabled_buttons();
}

void replay_controller::replay_next_side()
{
stop_condition_.reset(new replay_play_side());
update_enabled_buttons();
}

void replay_controller::replay_next_move()
{
stop_condition_.reset(new replay_play_moves(1));
update_enabled_buttons();
}

//move all sides till stop/end
void replay_controller::play_replay()
{
stop_condition_.reset(new replay_play_nostop());
update_enabled_buttons();
}

void replay_controller::update_gui()
{
controller_.get_display().recalculate_minimap();
controller_.get_display().redraw_minimap();
controller_.get_display().invalidate_all();
controller_.get_display().redraw_everything();
}

void replay_controller::handle_generic_event(const std::string& name)
void replay_controller::update_enabled_buttons()
{
controller_.get_display().invalidate_theme();
controller_.get_display().redraw_everything();
}

if( name == "completely_redrawn" ) {
update_replay_ui();
} else {
void replay_controller::handle_generic_event(const std::string& name)
{
// this is only attached to one event - the theme_reset_event
if(name == "theme_reset") {
add_replay_theme();
}
if(std::shared_ptr<gui::button> skip_animation_button = controller_.get_display().find_action_button("skip-animation")) {
Expand All @@ -288,6 +165,7 @@ bool replay_controller::recorder_at_end() const

REPLAY_RETURN replay_controller::play_side_impl()
{
update_enabled_buttons();
while(!return_to_play_side_ && !static_cast<playsingle_controller&>(controller_).get_player_type_changed())
{
if(!stop_condition_->should_stop())
Expand All @@ -313,11 +191,18 @@ REPLAY_RETURN replay_controller::play_side_impl()
}
}
controller_.play_slice(false);

// Update the buttons once, on the transition from not-stopped to stopped.
if(stop_condition_->should_stop()) {
update_enabled_buttons();
}
}
else
{
// Don't move the update_enabled_buttons() call here. This play_slice() should block
// until the next event occurs, but on X11/Linux update_enabled_buttons() seems to put
// an event in the queue, turning this into a busy loop.
controller_.play_slice(true);
replay_ui_playback_should_stop();
}
}
return REPLAY_FOUND_END_MOVE;
Expand Down
27 changes: 10 additions & 17 deletions src/replay_controller.hpp
View file
Expand Up @@ -61,26 +61,19 @@ class replay_controller : public events::observer
void add_replay_theme();
void init();
void update_gui();
void rebuild_replay_theme();
void handle_generic_event(const std::string& name) override;

void reset_replay_ui();
void update_replay_ui();
/**
* Refresh the states of the replay-control buttons, this will cause the
* hotkey framework to query can_execute_command() for each button and then
* set the enabled/disabled state based on that query.
*
* The ids for the associated buttons are: "button-playreplay",
* "button-stopreplay", "button-resetreplay", "button-nextturn",
* "button-nextside", and "button-nextmove".
*/
void update_enabled_buttons();

void replay_ui_playback_should_start();
void replay_ui_playback_should_stop();

std::shared_ptr<gui::button> play_button();
std::shared_ptr<gui::button> stop_button();
std::shared_ptr<gui::button> reset_button();
std::shared_ptr<gui::button> play_turn_button();
std::shared_ptr<gui::button> play_side_button();
std::shared_ptr<gui::button> play_move_button();

bool replay_ui_has_all_buttons() {
return play_button() && stop_button() && reset_button() &&
play_turn_button() && play_side_button();
}
play_controller& controller_;
std::unique_ptr<replay_stop_condition> stop_condition_;
events::command_disabler disabler_;
Expand Down

0 comments on commit e4cbfa2

Please sign in to comment.