Implicitly constructing a log_domain from a string has the effect of resetting its severity to 1 (or whatever severity was passed to the constructor). This is probably fine when it's declared as a file static, but no good if passing the log_domain to a log_scope2() or dont_log() call.
Fixes#2312. Turns out constructing a list of matching handlers breaks some undocumented
behavior, namely events being executed in the order they're added (usually the order in
which they appear in the WML). It also broke certain variable substitution by moving variable
interpolation wholly before any event execution, as opposed to between each event.
This commit implements an even simpler method: simply iterating through every registered
handler in order, checking for a name match on each one, and if it passes, immediately
executing the provided function. Cleanup will only occur once the toplevel call to
execute_on_events is completed.
This commit also removes the immediately cleanup upon removing a handler. This should
ensure (I didn't test explicitly, but from other incidental issues I'm sure this is the
case) the size of the main vector doesn't change during iteration.
I also changed the pre-add standardization code to only affect event names without variables.
Variable names are standardized later after variable substitution.
This ensures all names are always valid to use in by-name lookup and are stored consistently.
Also fixes a potential issue in 2c12d1328ba6. I should have called standardize_name on each
name entry in the cleanup phase like when done in the registration phase, but this commit
ensures that's not needed anymore.
* Fixed assertion failure when removing single-use custom events. Not actually something
broken, I just forgot to add a "not already disabled" check when doing so since I added
that assertion in event_handler::disable.
* Added a message to the aforementioned assertion.
* Ensure handler cleanup actually happens when removing custom events.
* Account for events with multiple names in cleanup.
This throws out the custom smart_list class in favor of a plain std::list. It also greatly simplifies
a few things. First, event handlers no longer remove themselves from the main list in event_handlers.
Now they just flag themselves as disabled (which means they will never execute once marked) and cleaned
up later in a newly added cleanup stage. This means a handler no longer needs to keep its index in the
active handler vector.
This removal of reliance on indices also means I could add the aforementioned cleanup stage. With the
smart_list code, event handlers were never actually removed from the active vector, nor any weak_ptrs
pointing to them removed either. This wasn't exactly a problem, since the handlers were stored via
shared_ptrs which would then simply be null after one deleted itself. Still, it's cleaner to drop any
invalid ones (and unlockable weak_ptrs) from any relevant containers. I've opted to do this in
manager::execute_on_events. Seems a good enough place as any.
The net result of this is the code is much cleaner. We're able to get rid of a bunch of unnecessary
feelers into various classes. This also makes the manager::iteration dereference code a lot easier
to understand. There certainly could be further refactoring, but I think this is a good start.
