Replays crash when a training site construction window is open

This is the stacktrace for the issue:

#3 0x11162af90 in InputQueueDisplay::radiogroup_changed(int) (/Users/sirver/Desktop/Programming/cpp/widelands/bzr_repo/../build/debug/src/widelands:x86_64+0x101912f90)
... various stuff related to boost signal
#19 0x110fe76b6 in UI::Radiogroup::set_state(int) (/Users/sirver/Desktop/Programming/cpp/widelands/bzr_repo/../build/debug/src/widelands:x86_64+0
#20 0x11162340b in InputQueueDisplay::update_priority_buttons() (/Users/sirver/Desktop/Programming/cpp/widelands/bzr_repo/../build/debug/src/widelands:x86_64+0x10190b40b)
#21 0x11161f306 in InputQueueDisplay::max_size_changed() (/Users/sirver/Desktop/Programming/cpp/widelands/bzr_repo/../build/debug/src/widelands:x86_64+0x101907306)
#22 0x11162832e in InputQueueDisplay::think() (/Users/sirver/Desktop/Programming/cpp/widelands/bzr_repo/../build/debug/src/widelands:x86_64+0x101

The problem is that if a window of a building is open while watching the replay and the priority buttons have been clicked in the game, this triggers the callback of the UI::Radiogroup, which assumed that it would only be called through *clicking* the buttons.

Since in the callback we do not check if the igbase can_act() for our current building_, we just send the player command. The code assumed it was sufficient to disable the radiobuttons in the replay, so the replay watcher cannot click on them.

Similar bugs might be potentially in other places too (e.g. productionsite max wares adjustments, warehouse settings or economy settings change), but I did not find any more on a quick glance.

To trigger it, the settings must have been adjusted in game and the replay must have the window open of the building that it was adjusted to.

Btw, a nice trick to get a backtrace in debug now is to trigger asan:

std::vector<int> a; a[100] = 1;

I think we have more of those, and the design is looking a bit fragile to me.

How about having a can_act boolean in the Player object instead and check it at the PlayerCOmmand level?

I think that would fix the Symptome, not the cause.

The ui callbacks contain usually more logic than just sending player command. Some of that logic should run in a replay - in which case it should be pulled out into a different code path. These bugs are subtle though and will be hard to find.

Getting the exception thrown like it is now is at least an easy to diagnose bug and makes diagnosing wrong code easier.

> Am 02.12.2017 um 08:26 schrieb GunChleoc <email address hidden>:
>
> I think we have more of those, and the design is looking a bit fragile
> to me.
>
> How about having a can_act boolean in the Player object instead and
> check it at the PlayerCOmmand level?
>
> --
> You received this bug notification because you are a bug assignee.
> https://bugs.launchpad.net/bugs/1735090
>
> Title:
> Replays crash when a training site construction window is open
>
> Status in widelands:
> In Progress
>
> Bug description:
> To reproduce, unzip open the attached replay with Build 19. Wait for
> the yellow player to start building the arena and training camp and
> open their construction site windows.
>
> Fatal Exception: [b19/src/logic/replay_game_controller.cc:68] Trying
> to send a player command during replay
>
> The stack trace is masked by the emergency save, so we'll have to
> switch that off to investigate.
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/widelands/+bug/1735090/+subscriptions

OK, I'll trawl through the UI then to see if there are more of those that we missed.

I have had a look at the code & done some testing, i think these were the only ones.

I did attach a branch though were I remove the buttons from the economy options window - they don't carry any information for spectators, so we might as well not create them at all.

Fixed in build20-rc1