Previously the config class had an operator bool and
it was a common pattern to use if(const config& = cfg.child(..)).
While this pattern was nice to use. It has severe drawbacks, in
particular it was unclear whether a function that took a config&
parameter allowed "invalid" configs, while most functions
did not, there were some that did. Furtheremore it lead to a few
buggy codes that were unconvered by this change (Not fixed though!),
in particular codes that tested local config objects that were
not references to being invalid, which could never be the case.
This commits replaces those with just `true` in order to not
change behaviour.
Some obvious cases were also removed including for example
things like `assert(config());` There is ony case in the ai code
that i'm not 100% sure of where one implementation of a virtual
function checked for an invalid config and another one that didn't.
With this, all code that check for a config child to be
present now uses config::optional_child which returns an object
that behaves similar to optional<(const) config&>, so it throws
on invalid dereferencing. But it also has operator[string] for
convinience, in particular to make is similary
easy to use the the previous `if (config& = .. child())`.
Also it has a tool DEBUG_CONFIG which tests whether all
optional_config values are checked before they are derefereneced.
Another method manditory_child was
added that throws when the key is not found, which replaces all
occurances of child() that did not check whether the result was
valid. This was neccecary (this= adding a new method instead of
renaming .child) to keep track of converted changes, and be sure
no occurances of child() were accidentally changed to the
throwing version.
We might want to rename one of mandatory_child or optional_child
to just child later. Not sure which one yet. I think it's better
to keep it in the current state (no config::child() ) for a while
though, so that people that currently used child() in their open
prs or other work get an error and not wrongly rely on the previous
behviour of config::child.
The interface of vconfig was not changed in this commit.
Generating the chackup data before sending the action
means that other clients would have the checkup available.
(if the "default_checkup" is used the "mp_debug_checkup"
also works after the action is sent). Note however that
its very possible that the action was already sended
before this code is reached, in those cases this change
will have no effect.
This also removes some outdated comments. In
particular sqashing undo actions into one (like
a comment suggeested) would probably bring the
undo stack out of sync with the replay stack.
Not having the shroud updated while dsu is disabled
could lead to shroud not being updated at all or worse
It is a bit annoying to works on code that should actually
be removed (DSU), but the whiteboard still doesn't work perferctly.
We don't want to return from play_idle_loop() when
the game is waiting for a [init_side] or [end_turn].
These actions should be delayed until the side has a
real controller.
1) Renamed !can_undo -> undo_blocked
2) We now automaticially clear the undo stack after every
action that blocked undoing. Since the playturn code
checks undo_stack::can_undo() to check whether to send
data or not this should make it more robust against
errors caused by unsent turn data.
This caused the end_turn button to remain 'pressed' when
using "Replay from turn".
There are now two variables end_turn_requested_ which is
true when the end turn button was pressed, and
end_turn_forced_, which is true when [end_turn] was used.
The later is considered part of the gamestate, to it is
stored in savefiles, the first isn't.
Alternatively we could make sure the synced action that
invokes the [end_turn] ends the turn directly instead of
using a seperate [end_turn] action for that.
But then we'd need to come up with a way to notify the
server about the turn change. (which wouldn't be hard
though, we'd use a similar mechanic as we aleady use
for side changes done by wml)
This function also was missing a send_data() call to send the
last action (which probably caused the game to end), i actually
don't know how this passed the ./mp_tests before.
(even if never populated) rather than only the needed cache. The index was also never reset for the lifetime of the program.
This new design uses a hashmap based on the locators themselves.
wmi_manager::set_item would replace a wml_menu_item object with a new one whose state was copied
from the old, but the old one was destroyed *after* the new one was constructed. That means when
the new object (and hence the new record) was created, it would not update the entry in the master
hotkey command info list, since the old still existed. The old object (and its record) would then
be destroyed, performing cleanup in the process, and resulting in the bug.
A new wml_menu_item object constructed from the state of the old one now takes control of its
record as well, so the new object has sole control over its lifetime and the hotkey info can be
appropriately updated.
