0ct0pu5/wesnoth
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Add attribute apply_to_vision to [effect]apply_to=movement

Browse source 
When processing [effect]apply_to=movement, if apply_to_vision is 'yes'
(which is the default) then the vision points will change by the same amount.
Previously, it sometimes affected vision and sometimes didn't; for most cases
where it makes a difference, I expect it to be a change from unanticipated
behavior to expected behavior.

Please refer to the new unit test added in this commit for more detailed docs;
that test is also a rough draft for the Wiki update needed when this merges.

The reason it sometimes affected vision was that the special value of -1 vision
points was interpreted as "use the value of the max movement points instead".
The special value of -1 is still supported and frequently used, and refactor that
is out of scope for this commit - it's easy to check when the code path changed
in this commit is used, however it's considerably more complex to find all
routes that create a unit with vision set to -1.

I'm expecting one add-on, Rebirth In Nature, to need a large update for this;
as well as a trivial change needed to the Add Creature Pack.

There are several mods that have their own handling for vision, recalculate the
values frequently, and are expected to continue working as before, as they'll
just overwrite any changes that the engine makes.
* Proper Flying Mod / Proper Vision Mod
* LotI
* Blessed Altar Mod
* Shards Era's `CALCULATE_WEAPONS_ONLY` macro
* Castle of Evil Spirit

Rebirth In Nature is an RPG-style campaign that expects the player's unit to
have vision separated from movement along with right-click menus that change
the character's movement, this will probably need `apply_to_vision=no` in
many places. OTOH, its `item_id=mobility` uses `apply_to=movement` followed by
`apply_to=vision` to implement in 1.14 what this change will make default
behavior in 1.16.

There are a few sharpshooter units which have vision better than their
movement. If given a buff which boosts their movement, this will give
them a buff to vision too; likely a change towards expected behavior.
* Archaic Era's Royal Ranger
* Ageless Era's Royal Ranger and Dwarvish Forest Sniper
* Eastern Europe at War's Yacht and Great Yacht
* Era of More Units's Lone Wolf
* Southernerns has several sharpshooters
* WWII Era has several sharpshooters
* War of Dominions has some guard towers with very low movement

Silver Age has abilities that modify vision based on time-of-day, but unlike
the Proper Flying Mod and LotI these are done by adding and removing objects
that add or subtract one vision point, these should be unaffected by this
change. Unrelated to that, it declares values for `[unit_type]vision=` even
when the value is the same as max moves - like the sharpshooters this means
that buffs to their movement wouldn't have buffed their vision but now do.

There are few units with vision less than movement. Generally these seem to be
missile weapons that are represented as units, and they probably won't meet any
apply_to=movement statements - even if a movement power-up is available, the
player would probably choose a different unit.

The Add Creature Pack has two creatures (the Cactose Elder and Carnivore Fatal
Plant) that have reduced vision, along with AMLAs that increase those units'
movement; these AMLAs will need to use `apply_to_vision=no`. These two
creatures are used in Castle of Evil Spirit, but they won't get enough
experience to get an AMLA there.

Fixes #3356.
This commit is contained in:
Steve Cotton 2021-01-14 08:34:46 +01:00 • committed by Steve Cotton
parent 7eedae4ee8
commit c560b0efab
5 changed files with 118 additions and 19 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
changelog.md
View file

@ -27,6 +27,7 @@
* Modify implementation of overwrite_specials attribute for replace yes/no parameter by none/one_side/both_sides and select abilities used like weapons and specials who must be overwrited(owned by fighter where special applied or both)
* Add a `ability_id_active` attribute to `[filter]`
* `[terrain_mask]` now accepts `mask_file` as an alternative to an inline mask. The file is loaded from the same place as `map_file` in the `[scenario]` tag (ie, a maps/ subdirectory of your binary path). Anyone who prefers to keep masks separate from regular maps is free to make a subdirectory for their masks (or just keep all their masks inline).
* `[effect]apply_to=movement` now always affects vision too, except when given the `apply_to_vision=no` attribute
### Miscellaneous and Bug Fixes
* More optimization in the UI drawing code, fixes the crash displaying the full credits (issue #5043).
* Made GUI.pyw compatible with Python 3.9 (issue #5719).

8
data/schema/units/modifications.cfg
View file

@ -51,7 +51,13 @@
[/tag]
[/case]
[case]
value=movement,vision,jamming,experience,max_experience,recall_cost
value=movement
{SIMPLE_KEY increase s_int_percent}
{SIMPLE_KEY set s_int}
{DEFAULT_KEY apply_to_vision s_bool yes}
[/case]
[case]
value=vision,jamming,experience,max_experience,recall_cost
{SIMPLE_KEY increase s_int_percent}
{SIMPLE_KEY set s_int}
[/case]

102
data/test/scenarios/test_effect_on_vision.cfg
View file

@ -134,6 +134,91 @@
[/event]
)}
# Test that changing the max_moves has a corresponding effect on vision points, even when the vision points have already been set to a separate value.
# This behavior is different to that in 1.14.
{GENERIC_UNIT_TEST "effect_move_separated_vision" (
[event]
name = side 1 turn 1
# Ignore the default values for an Elvish Archer, this test needs to be able to determine whether increase=50% was calculated based on the
# max movement points or the vision points, so it needs the values to be different.
[modify_unit]
[filter]
id=alice
[/filter]
[effect]
apply_to=movement
set=15
[/effect]
[effect]
apply_to=vision
set=3
[/effect]
[/modify_unit]
{ASSERT_ALICE_STAT max_moves equals 15}
{ASSERT_VISION_EFFECTIVELY equals 3}
{ASSERT_ALICE_STAT jamming equals 0}
# Increasing movement should affect vision
[modify_unit]
[filter]
id=alice
[/filter]
[effect]
apply_to=movement
increase=1
[/effect]
[/modify_unit]
{ASSERT_ALICE_STAT max_moves equals 16}
{ASSERT_VISION_EFFECTIVELY equals 4}
{ASSERT_ALICE_STAT jamming equals 0}
# Increasing movement by 50% increases vision by (50% of movement) not by (50% of vision)
[modify_unit]
[filter]
id=alice
[/filter]
[effect]
apply_to=movement
increase=50%
[/effect]
[/modify_unit]
{ASSERT_ALICE_STAT max_moves equals 24}
{ASSERT_VISION_EFFECTIVELY equals 12}
{ASSERT_ALICE_STAT jamming equals 0}
# Decreasing movement should affect vision
[modify_unit]
[filter]
id=alice
[/filter]
[effect]
apply_to=movement
set=16
[/effect]
[/modify_unit]
{ASSERT_ALICE_STAT max_moves equals 16}
{ASSERT_VISION_EFFECTIVELY equals 4}
{ASSERT_ALICE_STAT jamming equals 0}
# Vision shouldn't go below zero
[modify_unit]
[filter]
id=alice
[/filter]
[effect]
apply_to=movement
increase=-50%
[/effect]
[/modify_unit]
{ASSERT_ALICE_STAT max_moves equals 8}
{ASSERT_VISION_EFFECTIVELY equals 0}
{ASSERT_ALICE_STAT jamming equals 0}
{SUCCEED}
[/event]
)}
# Test that apply_to=vision works
{GENERIC_UNIT_TEST "effect_vision" (
[event]
@ -174,7 +259,7 @@
[/event]
)}
# Test that setting vision will avoid changing vision afterwards.
# Test that apply_to_vision=no will avoid changing vision
{GENERIC_UNIT_TEST "effect_move_ignores_vision" (
[event]
name = side 1 turn 1
@ -184,18 +269,6 @@
{ASSERT_VISION_EFFECTIVELY equals 6}
{ASSERT_ALICE_STAT jamming equals 0}
# In 1.14, this is a test that the default behavior hasn't changed, that setting vision to a value other than -1 will break the link between movement and vision.
# In 1.16, this will be replaced by a test of the also_apply_to_vision=no attribute.
[modify_unit]
[filter]
id=alice
[/filter]
[effect]
apply_to=vision
set=6
[/effect]
[/modify_unit]
# Make a copy so that each subpart of this test can reset her to this state
[store_unit]
[filter]
@ -212,6 +285,7 @@
[effect]
apply_to=movement
set=7
apply_to_vision=no
[/effect]
[/modify_unit]
{ASSERT_ALICE_STAT max_moves equals 7}
@ -229,6 +303,7 @@
[effect]
apply_to=movement
increase=4
apply_to_vision=no
[/effect]
[/modify_unit]
{ASSERT_ALICE_STAT max_moves equals 10}
@ -246,6 +321,7 @@
[effect]
apply_to=movement
increase=50%
apply_to_vision=no
[/effect]
[/modify_unit]
{ASSERT_ALICE_STAT max_moves equals 9}

25
src/units/unit.cpp
View file

@ -1965,8 +1965,16 @@ void unit::apply_builtin_effect(std::string apply_to, const config& effect)
hit_points_ = 1;
}
} else if(apply_to == "movement") {
const std::string& increase = effect["increase"];
const bool apply_to_vision = effect["apply_to_vision"].to_bool(true);
// Unlink vision from movement, regardless of whether we'll increment both or not
if(vision_ < 0) {
vision_ = max_movement_;
}
const int old_max = max_movement_;
const std::string& increase = effect["increase"];
if(!increase.empty()) {
set_total_movement(utils::apply_modifier(max_movement_, increase, 1));
}
@ -1976,12 +1984,19 @@ void unit::apply_builtin_effect(std::string apply_to, const config& effect)
if(movement_ > max_movement_) {
movement_ = max_movement_;
}
} else if(apply_to == "vision") {
const std::string& increase = effect["increase"];
if(apply_to_vision) {
vision_ = std::max(0, vision_ + max_movement_ - old_max);
}
} else if(apply_to == "vision") {
// Unlink vision from movement, regardless of which one we're about to change.
if(vision_ < 0) {
vision_ = max_movement_;
}
const std::string& increase = effect["increase"];
if(!increase.empty()) {
const int current_vision = vision_ < 0 ? max_movement_ : vision_;
vision_ = utils::apply_modifier(current_vision, increase, 1);
vision_ = utils::apply_modifier(vision_, increase, 1);
}
vision_ = effect["set"].to_int(vision_);

1
wml_test_schedule
View file

@ -215,6 +215,7 @@
# [effect]apply_to=movement and [effect]apply_to=vision
0 effect_move_affects_vision
0 effect_move_ignores_vision
0 effect_move_separated_vision
0 effect_vision
# [store_locations]
0 store_reachable_locations_vision