'kill'= misleading name

[Simons Mith]

An idle observation, if I may:

While kill= is often used to kill units, it's also used for unit modifications (e.g. stopping a fight partway, as in another recent thread). Wouldn't it be less misleading if this ever so commonly-used function was called 'remove' instead? If you save a unit's current state, then 'kill' it, then restore it, then you didn't really 'kill' it in the first place. I appreciate that making such a wide-reaching change is somewhat unlikely, but from a coding viewpoint I definitely would prefer to use 'remove' and 'restore' when that's what I'm doing, and only use 'kill' when I really mean 'kill'. Alternatively, what's the opinion of having aliases for commands, or having a new 'remove' command that 'saves-and-'kills'' while the 'kill' command just 'kills', as now?

[Pentarctagon]

personally i like it the way it is now fine, just because the death is 'temporary' doesn't mean that it didn't die.

[Captain_Wrathbow]

I'm afraid I don't see much of a problem with the current way, either.

[Ken_Oh]

I kind of see where you're coming from, but using kill=yes inside store_unit kills it just as much as by any other means (barring firing events and animation). There's no difference between the two scenarios below. Indeed, kill=yes replaces the basic function of [kill].

Code: Select all

# scenario 1

[store_unit]
    variable=unit
    [filter]
        x,y=1,1
    [/filter]
[/store_unit]

[kill]
    x,y=1,1
[/kill]

[unstore_unit]
    variable=unit
[/unstore_unit]


# scenario 2

[store_unit]
    variable=unit
    [filter]
        x,y=1,1
    [/filter]
    kill=yes
[/store_unit]

[unstore_unit]
    variable=unit
[/unstore_unit]

[Reepurr]

I don't see a problem with [kill]'s name. The only problem I see with it is that animate=yes doesn't show a death animation, only a fade.

[Anonymissimus]

However, it may be nice to support fire_event=yes|no and animate=yes|no if kill=yes. The name kill makes more sense then since you can actually see the unit die.

@Simons Mith Make the thread title better, I thought of [kill] too at first.

EDIT
https://gna.org/patch/?2031

[Captain_Wrathbow]

The only problem I see with it is that animate=yes doesn't show a death animation, only a fade.

That's the default death animation when a custom one isn't specified in the unitfile. At this point most units don't have custom death animations, so fading out is the unit's death anim. IIRC, death anims aren't very high priority for the artists right now...
But this is a completely different topic.

[Tet]

you could add a remove order including no-death animation. it would be allmost similar to kill.

[Astoria]

you could add a remove order including no-death animation. it would be allmost similar to kill.

Ahem...

Code: Select all

    [lua]
        code = <<
            local function handler(t)
                wesnoth.fire("kill",
                  { animate = "no", fire_event = t.event })
            end
            wesnoth.register_wml_action("remove", handler)
        >>
    [/lua]

Now you have a [remove] tag.

[silene]

Now you have a [remove] tag.

You have lost a bit of data in the process, so it would probably remove all the units from the map. And by default, there are no death animations, so there is a simpler way to get a [remove] tag:

Code: Select all

wesnoth.register_wml_action("remove", function(cfg) wesnoth.fire("kill", cfg) end)

If you also want to modify [kill] so that it always performs death animation and events, forget the previous code and use instead: (1.8-style syntax)

Code: Select all

local old_kill = wesnoth.register_wml_action("kill", function(cfg)
    local cfg = cfg.__literal
    cfg.animate = true
    cfg.fire_event = true
    old_kill(cfg)
end)
wesnoth.register_wml_action("remove", old_kill)

You will get two tags [remove] and [kill] with clearer names. Just make sure not to put the [lua] tag outside your campaign, or you will break every single scenario out there.