mirror of
https://github.com/wesnoth/wesnoth
synced 2025-04-27 14:41:44 +00:00
Remove the fast path of [modify_unit] (#6223)
Always use the "slow path", because it stores and unstores the unit, triggering the desired side effects of unstoring a unit. Fixes issue #5133, and tests that with the new unit test. Fixes bug #4978, that changing the facing wasn't updating the display. This corresponds to 1.16's commit 13c5d8a96ead8d2a7f0435f60b8ec7f7e0af0972, that commit merely disabled the fast path to be a minimal change.
This commit is contained in:
Steve Cotton
GitHub
parent
16617aaa7c
commit
69ee817955
@ -1,201 +1,11 @@
|
||||
local utils = wesnoth.require "wml-utils"
|
||||
local wml_actions = wesnoth.wml_actions
|
||||
|
||||
-- The [modify_unit] implementation
|
||||
-- modify_unit has basicially two implementations, the optimized implementation
|
||||
-- in the first part of this file and the fallback (old) implementation in the
|
||||
-- second part of this file
|
||||
|
||||
local function make_set(t)
|
||||
local res = {}
|
||||
for i, v in ipairs(t) do
|
||||
res[v] = true
|
||||
end
|
||||
return res
|
||||
end
|
||||
|
||||
local known_attributes = make_set {
|
||||
"mode",
|
||||
"x",
|
||||
"y",
|
||||
"ai_special",
|
||||
"goto_x",
|
||||
"goto_y",
|
||||
"extra_recruit",
|
||||
"side",
|
||||
"name",
|
||||
"role",
|
||||
"facing",
|
||||
"attacks_left",
|
||||
"hitpoints",
|
||||
"max_hitpoints",
|
||||
"moves",
|
||||
"max_moves",
|
||||
"experience",
|
||||
"max_experience",
|
||||
"resting",
|
||||
"canrecruit",
|
||||
"type",
|
||||
"variation",
|
||||
"ellipse",
|
||||
"halo",
|
||||
"recall_cost",
|
||||
"description",
|
||||
"hidden",
|
||||
"unrenamable",
|
||||
"profile",
|
||||
"zoc",
|
||||
"usage",
|
||||
"upkeep",
|
||||
}
|
||||
|
||||
local known_tags = make_set {
|
||||
"object",
|
||||
"advancement",
|
||||
"trait",
|
||||
"effect",
|
||||
"filter",
|
||||
"status",
|
||||
"set_variable",
|
||||
-- todo: "set_variables",
|
||||
"clear_variable",
|
||||
"filter_recall",
|
||||
"variables",
|
||||
}
|
||||
|
||||
local function is_simple(cfg)
|
||||
for k, v in pairs(wml.shallow_literal(cfg)) do
|
||||
if type(k) == "string" then
|
||||
if not known_attributes[k] then
|
||||
return false
|
||||
end
|
||||
else
|
||||
if not known_tags[v[1]] then
|
||||
return false
|
||||
end
|
||||
end
|
||||
end
|
||||
return true
|
||||
end
|
||||
|
||||
local function simple_modify_unit(cfg)
|
||||
local filter = wml.get_child(cfg, "filter") or wml.error "[modify_unit] missing required [filter] tag"
|
||||
-- todo: investigate the following attrtibutes:
|
||||
-- id, alpha, flying, overlays
|
||||
local simple_attributes = {
|
||||
"ellipse",
|
||||
"halo",
|
||||
"recall_cost",
|
||||
"description",
|
||||
"hidden",
|
||||
"side",
|
||||
"name",
|
||||
"role",
|
||||
"facing",
|
||||
"attacks_left",
|
||||
"hitpoints",
|
||||
"max_hitpoints",
|
||||
"moves",
|
||||
"max_moves",
|
||||
"experience",
|
||||
"max_experience",
|
||||
"resting",
|
||||
"canrecruit",
|
||||
"zoc",
|
||||
"usage",
|
||||
"upkeep",
|
||||
}
|
||||
|
||||
local function handle_unit(u)
|
||||
---------- ATTRIBUTES THAT NEED SPECIAL HANDLING ----------
|
||||
if cfg.x or cfg.y then
|
||||
u.loc = { cfg.x or u.x , cfg.y or u.y }
|
||||
end
|
||||
if cfg.goto_x or cfg.goto_y then
|
||||
u["goto"] = { cfg.goto_x or u["goto"][1] , cfg.goto_y or u["goto"][2] }
|
||||
end
|
||||
if cfg.extra_recruit then
|
||||
u.extra_recruit = cfg.extra_recruit:split()
|
||||
end
|
||||
if cfg.ai_special == "guardian" then
|
||||
u.status.guardian = true
|
||||
end
|
||||
if cfg.profile ~= nil then
|
||||
u.portrait = cfg.profile
|
||||
end
|
||||
if cfg.unrenamable ~= nil then
|
||||
u.renamable = not cfg.unrenamable
|
||||
end
|
||||
---------- SIMPLE ATTRIBUTES ----------
|
||||
for i, name in ipairs(simple_attributes) do
|
||||
if cfg[name] ~= nil then
|
||||
u[name] = cfg[name]
|
||||
end
|
||||
end
|
||||
|
||||
---------- TAGS ----------
|
||||
local found_recall, found_variables = false, false
|
||||
for i, t in ipairs(wml.shallow_parsed(cfg)) do
|
||||
local tagname, tagcontent = t[1], t[2]
|
||||
if tagname == "object" or tagname == "trait" or tagname == "advancement" then
|
||||
if tagcontent.delayed_variable_substitution then
|
||||
tagcontent = wml.literal(tagcontent)
|
||||
else
|
||||
tagcontent = wml.parsed(tagcontent)
|
||||
end
|
||||
u:add_modification(tagname, tagcontent);
|
||||
elseif tagname == "effect" then
|
||||
local apply_to = tagcontent.apply_to
|
||||
if wesnoth.effects[apply_to] then
|
||||
wesnoth.effects[apply_to](u, tagcontent)
|
||||
else
|
||||
wml.error("[modify_unit] had invalid [effect]apply_to value")
|
||||
end
|
||||
elseif tagname == "status" then
|
||||
for i, v in pairs(tagcontent) do
|
||||
u.status[i] = v
|
||||
end
|
||||
elseif not found_recall and tagname == "filter_recall" then
|
||||
u.recall_filter = wml.merge(u.recall_filter, tagcontent, cfg.mode or "merge")
|
||||
found_recall = true -- Ignore all but the first
|
||||
elseif not found_variables and tagname == "variables" then
|
||||
u.variables.__cfg = wml.merge(u.variables.__cfg, tagcontent, cfg.mode or "merge")
|
||||
found_variables = true -- Ignore all but the first
|
||||
elseif tagname == "set_variable" then
|
||||
wesnoth.wml_actions.set_variable(tagcontent, u.variables)
|
||||
elseif tagname == "clear_variable" then
|
||||
wesnoth.wml_actions.clear_variable(tagcontent, u.variables)
|
||||
end
|
||||
end
|
||||
|
||||
-- handle 'type' and 'variation' last.
|
||||
if cfg.type == "" then
|
||||
u.experience = u.max_experience
|
||||
elseif cfg.type or cfg.variation then
|
||||
u.experience = 0
|
||||
u:transform(cfg.type or u.type, cfg.variation)
|
||||
end
|
||||
|
||||
-- always do an advancement here (not only when experience/max_experience/type was modified)
|
||||
-- for compatibility with old code.
|
||||
-- Skip for recall list units
|
||||
if u.valid == 'map' then
|
||||
u:advance()
|
||||
end
|
||||
end
|
||||
|
||||
local this_unit <close> = utils.scoped_var("this_unit")
|
||||
for i, u in ipairs(wesnoth.units.find(filter)) do
|
||||
wml.variables["this_unit"] = u.__cfg
|
||||
handle_unit(u)
|
||||
end
|
||||
end
|
||||
-- The [modify_unit] implementation.
|
||||
-- Implementation detail: every affected unit is passed through [unstore_unit], so gets
|
||||
-- that tag's implementation's handling of special cases such as x,y=recall,recall.
|
||||
|
||||
function wml_actions.modify_unit(cfg)
|
||||
if is_simple(cfg) then
|
||||
simple_modify_unit(cfg)
|
||||
return
|
||||
end
|
||||
local unit_variable = "LUA_modify_unit"
|
||||
|
||||
local replace_mode = cfg.mode == "replace"
|
||||
|
||||
60
data/test/scenarios/test_modify_unit.cfg
Normal file
60
data/test/scenarios/test_modify_unit.cfg
Normal file
@ -0,0 +1,60 @@
|
||||
# wmllint: no translatables
|
||||
|
||||
# In 1.14 and before, the implementation of [modify_unit] always stored and
|
||||
# unstored the unit. 1.16 introduced a "fast-path" which optimised setting some
|
||||
# attributes, but which changed the behavior of the API in some edge cases.
|
||||
#
|
||||
# The store and unstore also ensured that the animation code noticed the
|
||||
# change, and the fast path meant that changes for facing or low hitpoints
|
||||
# failed to change the animation (issue 4978). However, that's a UI issue which
|
||||
# doesn't affect the API, can't be automatically tested via the WML test
|
||||
# framework, and which could be fixed during a stable branch.
|
||||
|
||||
# In this test, changing the side of a unit on a side's recall list needs to
|
||||
# have the side-effect of moving it to the correct side's recall list.
|
||||
{GENERIC_UNIT_TEST "modify_unit_which_recall_list" (
|
||||
[event]
|
||||
name = start
|
||||
|
||||
# Create a unit on the recall list of side 1
|
||||
[unit]
|
||||
x=recall
|
||||
y=recall
|
||||
type=Orcish Grunt
|
||||
side=1
|
||||
id=Charlie
|
||||
role=TestSubject
|
||||
canrecruit=no
|
||||
[/unit]
|
||||
|
||||
[modify_unit]
|
||||
side=2
|
||||
[filter]
|
||||
id=Charlie
|
||||
[/filter]
|
||||
[/modify_unit]
|
||||
|
||||
[role]
|
||||
role=TestSubject
|
||||
reassign=no
|
||||
[auto_recall][/auto_recall]
|
||||
[/role]
|
||||
|
||||
# Check that Charlie has been recalled next to side 2's leader
|
||||
#
|
||||
# Currently [role][auto-recall] uses the recall list (not the side attribute)
|
||||
# to determine where on the map to recall the unit; if this changes then this
|
||||
# test could pass with a false positive.
|
||||
{ASSERT (
|
||||
[have_unit]
|
||||
id=Charlie
|
||||
[filter_location]
|
||||
location_id=2
|
||||
radius=1
|
||||
[/filter_location]
|
||||
[/have_unit]
|
||||
)}
|
||||
|
||||
{SUCCEED}
|
||||
[/event]
|
||||
)}
|
||||
@ -108,6 +108,7 @@
|
||||
0 modify_turns_four
|
||||
0 replace_schedule_prestart
|
||||
0 modify_unit_facing
|
||||
0 modify_unit_which_recall_list
|
||||
0 event_handlers_in_events_1
|
||||
0 event_handlers_in_events_3
|
||||
0 event_handlers_in_events_2
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user