Fix a typo in the add_sub_separated test, because it was testing
exactly the same code as add_sub_cumulative.
Add two new tests and clarify documentation, because in these tests
the order of the abilities determines whether the add or sub value is
used, it isn't that sub always overrides add.
(cherry picked from commit 547de5fd93)
The tag_name check is deficient for two reasons:
1.when apply_to=both is used, in the case of [damage_type] [filter_opponent][filter_weapon]type= will check the modified type for application to the owner and the original type for application to the opponent, hence a risk of infinite recursion, especially if [filter_self][filter_weapon]type= is used in the same weapon.
2. in the case of apply_to=attacker/defender, the check for [filter_attacker/defender] must take into account both the value of the variable is_attacker but also the application AFFECT_SELF/OTHER, otherwise in the case apply_to=defender with [filter_defender][filter_weapon]type=, and[filter_attacker][filter_weapon]type= otherwise we end up in the same configuration as for apply_to=both
Finally tag_name is renamed check_if_recursion to mean that this variable is used to ensure that a recursion cannot be set up even if 743b146efc prevents it from causing a crash but with an error log
this is a bachport of https://github.com/wesnoth/wesnoth/commits?author=stevecotton4afdc92f13 comit because one change of behavior is to prevent crashes of game
This adds a recursion counter which runs on a per-weapon basis; if the
recursion limit is reached then the last level of recursion is assumed
to have returned false. A warning is printed (with error severity).
At the user-visible layer, that gives the following logic:
* Abilities that depend on another ability being active will be
inactive.
* Pairs of abilities, where X depends on Y being inactive and Y
depends on X being inactive, will end up with them both inactive.
The limit is defined by ATTACK_RECURSION_LIMIT in attack_type.cpp.
The minimum for passing all the unit tests is 2, but I've left some
headroom by setting it to 4.
With the recursion limit set to 1, the following tests fail, which
seems reasonable because they're checking that the special is on
a particular type of weapon.
* event_test_filter_attack_specials
* event_test_filter_attack_opponent_weapon_condition
* event_test_filter_attack_student_weapon_condition
Output from the four_cycle_recursion_branching test:
```
error unit: Recursion limit reached for weapon 'melee_attack' while checking filter 'special_type_active = parry'
error unit: Recursion limit reached for weapon 'melee_attack' while checking filter 'special_type_active = poison'
error unit: Recursion limit reached for weapon 'melee_attack' while checking filter 'special_type_active = damage'
```
There's deduplication to prevent the logfiles getting spammed during
AI turns. As implemented, the two tests mentioned below print 3
messages each; these tests are added in separate commits because the
tests are shared between PRs while we discuss the right mechanism for
guarding against recursion.
With no rate limit:
* four_cycle_recursion_branching prints 1280 logs
* event_test_filter_attack_student_weapon_condition prints 320
With a rate limit via a flag that's reset in recursion_guard's destructor:
* four_cycle_recursion_branching prints 80 logs
* event_test_filter_attack_student_weapon_condition prints 160
Resetting the flag in specials_context_t's destructor gets better numbers,
but splits the logic across .cpp files. I think the trade-off isn't worth it:
* four_cycle_recursion_branching prints 40 logs
* event_test_filter_attack_student_weapon_condition prints 132
Co-authored-by: newfrenchy83
Changes:
* CHECK_STRIKES macro now takes a comma-delimited list of strike counts for when units have a different number of strikes for different weapons, and updates the existing usage in the attacks tests
* Fixed a typo in the attacks_zero test
* Adds tests for berserk as a weapon special ability.
Berserk's handling differs from attacks as a weapon special ability:
* A value less than 1 is treated as effectively infinite rounds of combat (undocumented)
* The cumulative attribute is handled differently - in some cases it sums the values instead of using the highest single value whereas for attacks it always uses the highest single value
Also now exclude data/tests/ from scons pot-update since it was hitting the argument limit for number of arguments to a script.
A weapon with poison, which does extra damage to poisoned opponents.
This tests that the weapon-special's filter is checked on each strike,
instead of being tested once and then cached during the battle.
A surprising result on 1.18, the max_value attributes get added together.
In 1.19, 82499d0785 changes this to something
logical, using `min(each individual max_value)` and disregarding the cumulative
attributes.
These tests clarify what "cumulative" means for non-leadership abilites,
which since Wesnoth 1.1.3 doesn't seem to fit the English word.
For cumulative abilities, if the base value from the unit is higher that
the ability's value, then the base value is used instead. In the case of
a Skeleton with 40% base resistance, "cumulative=yes" is equivalent to
"value,cumulative=40,no".
In all of these cases, a single ability's value is used; "cumulative"
doesn't mean that the values from multiple abilities interact.
This is all when using value=, not when using add= or sub=.
The Barrow Wight has the Shroud ability: "When defending, this unit has 50%
Fire and Arcane resistance." However, if the attack's original damage type
isn't Fire or Arcane, the Shroud ability fails to trigger. The Barrow Wight
takes massive Arcane damage (50% weakness) when defending against such attacks.
Because this is the stable branch, the API can't fix the root cause, which is
that ability filters don't consider the weapon's alternative damage type. This
adds tests to verify that the API still works as before, but changes the items
in EI that give units alternative damage types.
* Fix#8460 [effect] apply_to=variation
Previously the code could apply the variation effects
last, so that codes like
```
[effect]
apply_to=variation
..
[/effect]
[effect]
apply_to=hitpoints
heal_full=yes
[/effect]
```
Would not set the unit hitpoints to the new variations
hitpoints because the variation effect was applied after
the healing effect.
In 1.16 this worked because healing was applied a little
too often but that lead also to bugs like #8342
* f prev
* f prev
* f prev
* f prev
* f prev
* Create modification_effect_type_variation.cfg
* Update wml_test_schedule
Change the Lua implementation to directly call the
non-deprecated function, this is related to 9daa10a9f2.
Clean up the C++ header files by removing the declarations of
functions that were removed in 3c8de46771.
Use [endlevel]end_credits=no in the test scenario (trigger it
by moving to hex 9,3). If you debug with :next_level without
triggering the moveto, "The End" will show; trigger the moveto
first, and it won't show the credits.
The units on the outsides of a backstab don't have to be allied
to each other, they just have to be enemies of the victim. That's
hardcoded into the AI via attack.cpp's backstab_check() function,
hence a unit test to check that WEAPON_SPECIAL_BACKSTAB activates
in that situation.
Add a new map with 4 sides all on separate castles. This is a
variation of the generic 2p unit test map, extended in the south
with a horizontal reflection of existing map.
Add a MAP_FILE argument to COMMON_KEEP_A_B_C_D_UNIT_TEST,
because it's the same setup except for the map file.
This is testing experience=yes and experience=no, with the expectation that
some new values for that attribute will be added afterwards.
Adds a new generic macro ASSERT_UNIT_HP_XP. Looking thorugh the ability tests,
ASSERT_UNIT_HP (without the XP) could be a heavily-used macro, but it wouldn't
be used by this particular commit, so I'm not adding it in this commit.
previously it needed multiple calls to remove multiple types of modifications (which could be slow since each call involves a full rebuild of the unit).
The ability's code in UtBS hasn't changed since 1.16, but there's now a bug about
healing sometimes not working in UtBS S02. The new test was written to check
for an engine bug when a healer has two units next to it, only one of which
passes the filter; although it's passing, it seems reasonable to keep the test.
Also, use `{ASSERT ...}` for better logging in the old healing test.
At the suggestion of @stevecotton, I propose a special 'replacement_type' and 'alternative_type' attribute capable of modifying the type of attack used when the conditions are met.
Also make Holy water combine arcane damage with native type of weapon
Like holy water imbued ordinary weapon, it's seem logic what arcane damage dominant what if more efficient what original type(water can't altered pierce or blading of spear or sword)
Fix https://github.com/wesnoth/wesnoth/issues/7923 .when we want to detect an ability with value=0-20 and an ability with add=15 is present, this is detected because the system assigns a value of 0 when 'value' is not present, and so the system matches abilities that do not use 'value' or even no numerical value at all. it is therefore necessary to decide that the absence of an attribute is a non-correspondence to the criteria.
There are questions about how this filter should treat empty values, and the
use cases that we can think of can be implemented in other ways, for example,
if we want to look for big damage boosts, then we could filter for
`add=5-infinity` `[or]` `multiply=2-infinity`.
The simple answer seems to be to remove it from the API before we freeze for 1.18.
There's more discussion in issue 7944.
using overwrite_specials alone means that we have only two possibilities, either one_side is chosen and in this case if the ability used as a weapon carrying the attribute is applied to the unit (apply_to=self), the other abilities are the same type applied also to 'self' not carrying the attribute will be overwritten, but those of the opponent with apply_to=opponent will be kept in the list; or else both_sides is chosen and all abilities of the same type that do not carry the attribute will be overwritten. To be able to use the attribute in abilities like [damage] for example, it is necessary to be able to be even more selective as for a 'charge' type leadership with multiply=2.5 but which must not be combined with the classic charge and without overwriting the aute [damage] as backstab, [overwrite_filter] to only match damage with apply_to=both and active_on=offense is then interesting.
adding priority allows you to select that it ability can use its overwrite_specials attribute while the others can be overwritten in the same way as an ability without the attribute. Finally, this system makes it unnecessary to limit the use of the attribute to abilities used as weapons but also to special weapons.
give abilities support of halo or overlay so that the unit benefits from a second halo or overlay when conditions are matched
One of the things that bothers me is the permanent character of the halos of the Mage of Light and other units with the "illuminates" ability, which forces them to program only a permanent illumination applied only to the possessor of the ability.
Adding the halo attribute to ability does not change anything about the behavior of the unit, but can be used in several cases:
1 allowing the use of ""illuminates" whose activity would be variable, in this case encoding the halo in [illuminates] ability and not in the unit_type allows to modulate the appearance of the halo under the same conditions
2. Applying illumination to adjacent units, I know it's pretty cheesy but a set developer might consider it easier if the hlo display follows the same logic.
3 The halo used to illustrate the possession in the unit of a special weapon used as leadership, the halo would be used to raise the possessor of the ability.
for overlay, same logic for illustrate possession of a special weapon used as leadership, or influence on student
with the "halo_image" attribute, it is now possible to give Sun Sylph units an illumination ability with an activity depending on the incarnate sun attack instead of giving the ability and the halo via obect and therefore allowing the player to have access to the description of the ability even when it is inactive.
