Partial revert of commit ecbb15e1c6d1106ff2f8e540c584523481426c04.
Replacing bounded_add() with std::clamp() in these contexts disables
time-of-day bonuses, since ToD bonuses (the base value in these functions)
are almost always outside the range set in terrain properties.
* Dropped bounded_add for clamp. There's a slight behavior change with this, namely that
base is no longer returned if it falls outside the bounds. However, for both usecases of
bounded_add, that behavior doesn't make sense. The wiki clearly states min/max_light=
define the bounds for light=.
* Replaced gcd with Boost's gcd function. We already use the latter in the Preferences
dialog, so this is more consistent.
* Simplified the implementation of in_ranges.
actual bugs found:
* backwards_compatibility.lua (undeclared global "helper")
* core.lua (use of undeclared global "helper")
* wml_tags.transform_unit had wrong code to deal with recall_cost
* wrong variable name in cave_map_generator
__builtin_expect is something that should only be used if you're very sure it will result in
an optimization (see http://blog.man7.org/2012/10/how-much-do-builtinexpect-likely-and.html).
This code was added back in 2008. It seems it was part of an effort to get the preprocessor
functioning faster. It looks like mordante played some role in the decision of where to use
the LIKELY and UNLIKELY macros, so it's possible it had some use at the time (he's a rather
experienced programmer).
However, it's better to let the compiler automatically optimize code itself. I haven't done
any profiling on the preprocessor speed with or without __builtin_expect, but it doesn't seem
worth keeping this around and having to test every so often whether it's still useful or has
become a performance hit instead.
It's also worth noting that I myself noticed what seems a perceptible performance improvement
in the game (again, not backed by profiling) when I switched from TDM GCC to MSVC 2017. IIRC,
I'm using at least quick LTO for my builds. Apples and oranges, yes, but it also proves there
are likely various compiler options (such as LTO) which could improve performance on their own,
and better, than trying to point the program down branch paths ourselves.
On Windows, win32 programs such as Wesnoth aren't allowed to use control
characters in filenames, and therefore attempting to create a save file
whose name contains control characters will always fail. In addition, some
control characters (newlines in particular) also break Wesnoth's UI, as
regular text fields aren't intended to display multiple lines.
Fixes#2366.
This reverts part of commit 468c6e0f494793d57b0b1264894cd7f0b0383fa2.
Ctrl+A was removed because its emacs meaning is surprising to some
people. Ctrl+E was removed because it was the counterpart of Ctrl+A.
Ctrl+U however doesn't conflict with anything else, and it's useful
to some people (me), so reinstate it.
* moved the new function next to valid_id()
* renamed to valid_tag(), and renamed valid_id() to valid_attribute()
* removed unnecessary parentheses I forgot in
* changed the name of valid_id()'s parameter from "id" to "name"
* changed WML parser to use valid_tag() when it validates tag names
Thanks to @CelticMinstrel who told me about config::valid_id().
This was introduced this development cycle (1.13) in 2015 (commit 6010455f563f4b16bb89f2df6893908a36251cdc)
and removed in 33c2e6aa67c9377f170f9302204a8de10c200564. Presumably the action was left for backwards
compatibility, but we do not guarantee backwards compatibility for changes that never made it to a stable
release.
The implementation differs from already-existing
wml.variable.proxy in that it does not try to proxy table sub-fields,
and is fast & simple.
Example usage:
wml.variables.test = 123
print(wml.variables.test)
Since events can be added and removed constantly, this should be more performance-friendly than
a vector since it avoids reallocations. Though, since this container only holds shared_ptrs instead
of the objects themselves, it probably doesn't make that much difference right now, but I might
switch to object-direct storage in the future. Not sure.
It turned out that it works fine as long as it's called with a char
rather than unsigned char. (But to add to the confusion, the other
variant of the function in <cctype> *requires* that cast.)
The function seems to just throw std::bad_cast() in libstdc++ and libc++.
Fortunately it's very easy to write our own implementation instead.
Thanks to @Pentarctagon for the stack trace.
Tags that the WML parser rejects shouldn't be possible to create with Lua,
either. This ensures that a UMC author can't accidentally, or deliberately,
inject such tags into the game state and make saving the game impossible.
Fixes#2375.
Only happened if the player used the buttons in the addon list: the buttons
in the right panel worked fine. The problem was that the addon list caught
the addons_client::user_exit exception separately and didn't rethrow it
when necessary, unlike the addon manager itself.
I decided to simply throw a different exception instead.
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.
Empty action chains are not allowed, attempting to partially undo an empty
chain causes a fatal error.
This commit fixes some of the breakage reported in #2306, but not all.
