v1.18 have_unit and die event

[Spannerbag]

https://github.com/wesnoth/wesnoth/pull/9164

That appears to fix the issue, but I would be worried that there may be existing code depending on the old behavior (since it has worked that way since version 1.10 or earlier).

True, but I'd really like to see this fixed in 1.19/20.
It might mean a bit of work on mainline campaigns (maybe) but I think it would be worth it.
A new event would also probably work but I personally would rather have the existing inconsistency fixed.
But then it won't be me doing the coding...

Cheers!
-- Spannerbag

[Ravana]

I mentioned another difference between fight kill and [kill] in pr. If you think that should be changed too someone should create separate issue.

[Spannerbag]

I mentioned another difference between fight kill and [kill] in pr. If you think that should be changed too someone should create separate issue.

I presume you mean that healing units in last_breath or die stops them dying when slain in combat but does not do so when they are simply [kill]ed?
(Note: units "resurrected" in this way do not garner any xp. )

Making healing a unit inside a last_breath or die event consistent would be good.

In combat or traps etc. a unit can die at any time due to player or ai actions/RNG.
However, kill is usually used, in my experience anyway, by campaign creators to permanently remove one or more units (otherwise they'd use [store_unit], [put_to_recall_list] etc.)?
That is, the situation is usually very specific and not intended to be undone?

IMHO I'd be inclined to allow kill to be undone inside die for consistency even if it will probably only be very rarely used provided it's a quick and easy fix, not sure it's worth a lot of time and effort from volunteers?

Cheers!
-- Spannerbag

[white_haired_uncle]

However, kill is usually used, in my experience anyway, by campaign creators to permanently remove one or more units (otherwise they'd use [store_unit], [put_to_recall_list] etc.)?
That is, the situation is usually very specific and not intended to be undone?

Permanently? Not at all in my experience.

Try storing a unit, editing it, and then unstoring it a different location. Presto, two copies of Riker. One of them must die.

[Spannerbag]

However, kill is usually used, in my experience anyway, by campaign creators to permanently remove one or more units (otherwise they'd use [store_unit], [put_to_recall_list] etc.)?
That is, the situation is usually very specific and not intended to be undone?

Permanently? Not at all in my experience.

Try storing a unit, editing it, and then unstoring it a different location. Presto, two copies of Riker. One of them must die.

Sorry, wasn't clear: I meant that (in my experience) a common use of [kill] is to remove a unit permanently from a campaign with no intent to resurrect it (e.g. because it isn't needed anymore or needs to die a heroic death etc.). Yes you can do all sorts of clever stuff is you want... but I wasn't talking about that/those.

Cheers!
-- Spannerbag

[beetlenaut]

A "die" event is triggered automatically by the engine when a unit has 0 hitpoints. Raising the unit's hitpoints during that event causes it to not finish doing its work. (That was a choice by developers obviously, but it makes sense.) On the other hand, [kill] is fired manually by the coder who wants a unit not to be there any more. Raising the unit's hitpoints in the "die" event works as expected, but then control is passed back to [kill], and the unit is removed as the coder requested. It doesn't make much sense to me that you could prevent [kill] from finishing just by raising the unit's hitpoints. That would actually be less consistent in my opinion. If you need it, you could request a key inside [kill] that prevents it from terminating, but that would be a separate feature.

[Spannerbag]

Apologies for a longish post but I want to fully detail my understanding of the issue here.

A "die" event is triggered automatically by the engine when a unit has 0 hitpoints. Raising the unit's hitpoints during that event causes it to not finish doing its work. (That was a choice by developers obviously, but it makes sense.) On the other hand, [kill] is fired manually by the coder who wants a unit not to be there any more. Raising the unit's hitpoints in the "die" event works as expected, but then control is passed back to [kill], and the unit is removed as the coder requested. It doesn't make much sense to me that you could prevent [kill] from finishing just by raising the unit's hitpoints. That would actually be less consistent in my opinion. If you need it, you could request a key inside [kill] that prevents it from terminating, but that would be a separate feature.

I think this has slightly drifted off-topic, but I'll respond to your thoughts first:

You make a good point but it depends I guess on where you want behaviours to be consistent?
I agree that [kill] is generally used to remove a unit by the developer.

As I understand things either:

1. [kill] is absolute and raising hitpoints in [die] won't work in this case but will if the unit, for example, died in combat.
   So [die] is inconsistent but [kill] is consistent; i.e. always kills the specified unit(s).
2. Or, raising hitpoints in [die] always works.
   So [die] is consistent but [kill] can be aborted/cancelled.

I don't have a fundamental conceptual issue with either method/interpretation of how these work.
However for the practical reasons below, I would prefer the second option (b.) above.

My problem was with [have_unit] which worked (or did not) depending upon how the unit died which was a result of the dying unit having >0 hitpoints (or not) within the [die] event (i.e. [kill]ed in the former, otherwise died by reducing hitpoints to zero or less by combat, [harm_unit] etc.).

If a unit has >0 hitpoints then [have_unit] considers that unit to exist and this was the inconsistency that snagged me.
Specifically, one of the victory conditions involved trapping when the last side 1 unit departed the scenario and it (usually) didn't matter whether said unit did so by escaping or dying.

The scenario involved lethal terrain which immediately [kill]ed any unit entering said terrain.
So my use of [kill] was indeed to remove unit(s) from the campaign with no intention of reversing the [kill] action later.

During playtesting I was caught by the possibility where victory arises by the last unit being [kill]ed by this lethal terrain.
In this specific instance [have_unit] (within [die]) failed in that count=0 returned false when it should have been true.
Yet, weirdly, the actual [objectives] did update correctly which used exactly the same logical test (indeed the same macro!) so I was going quietly insane trying to figure out what I'd done to cause this.

Now the proposed fix, as I understand it, was to have [kill] also reduce the dying unit's hitpoints to zero so that [have_unit] was fed a consistent unit state to thereby always correctly include or exclude units in the process of dying.
However, as I understand this, a side effect of this is to allow [kill]ed units to be brought back by raising their hitpoints in [die] the same way as is possible now when a unit has died through hitpoint attrition (combat, [harm_unit] etc.).
I don't think this makes much difference because it won't change gameplay and content creators won't generally want to reverse [kill] - as you point out.

Also, this solution does not require any change to the wiki whereas otherwise it may be necessary to add a note to [have_unit] (and arguably [kill]) that there is an exception to [have_unit] behaviour within [die] (and [last breath]).

Hope this all makes sense.

Cheers!
-- Spannerbag

[Ravana]

Included as first item of viewtopic.php?t=58532

[Spannerbag]

Included as first item of viewtopic.php?t=58532

Thanks!

Cheers,
-- Spannerbag