[heal_unit] and status=unhealable

[white_haired_uncle]

Code: Select all

        [heal_unit]
            animate=no
            [filter]
                x,y=$x1,$y1
            [/filter]
            amount=2
            restore_statuses=no
        [/heal_unit]
 

where the unit at x1,y1 has status=unhealable. What is the expected behaviour? I'm leaning toward the unit should NOT being healed 2HP.

[Ravana]

Should be healed.

[white_haired_uncle]

Just to clarify, the unit is healed.

I'm just wondering if this is a bug, or if the wiki needs to be updated to make it clear that a unit with status=unhealable is really just "unhealable under certain circumstances: by a healer, by a village, but not by [heal_unit]" -- which should also include some mention of what happens for advancement, regeneration, recall, etc".

Personally, I'd call it either a bug or a really poorly named status. If it's unhealable, it shouldn't heal IMO.

[gnombat]

If it's unhealable, it shouldn't heal IMO.

But what if the developer really does want to heal the unit?

I think the rationale for this is that the status sets the default healing behavior, but it's not intended to prevent developers from healing the unit programmatically if that's what they want.

[white_haired_uncle]

If it's unhealable, it shouldn't heal IMO.

But what if the developer really does want to heal the unit?

I think the rationale for this is that the status sets the default healing behavior, but it's not intended to prevent developers from healing the unit programmatically if that's what they want.

Then they (temporarily) clear the unhealable status, or perhaps use harm_unit with a negative value.

Or, conversely, what if they don't want unhealable units to be healed, perhaps because they are unhealable, so healing them wouldn't make any sense?

Whatever the correct behaviour is, we're probably stuck with the current behaviour, since adjusting the docs won't break anything that's based on the current behaviour. Unfortunately. Either way, the docs should be clear on how it works, even if how it works is wrong.

[gnombat]

Then they (temporarily) clear the unhealable status,

Well, that might work, but:

1. It's a lot of additional code the developer would have to write.
2. It seems like it could have additional side effects; suppose that the unhealable status has some indicator or icon in the user interface. Would clearing and re-enabling the unhealable status cause this to flash off and on again? What if the developer doesn't want that?

or perhaps use harm_unit with a negative value.

Does that actually work? It doesn't seem to be documented.

Or, conversely, what if they don't want unhealable units to be healed, perhaps because they are unhealable, so healing them wouldn't make any sense?

Can't the developer just filter out the unhealable units?

My point is that the current behavior essentially allows the developer to make the decision about whether unhealable units should be healed or not. If the developer wants them to be healed, that will work. If the developer doesn't want them to be healed, that can be made to work too - the developer just needs to add one more filter.

Whatever the correct behaviour is, we're probably stuck with the current behaviour, since adjusting the docs won't break anything that's based on the current behaviour. Unfortunately. Either way, the docs should be clear on how it works, even if how it works is wrong.

Sure, the documentation could say that unhealable has no effect on [heal_unit].

[white_haired_uncle]

Then they (temporarily) clear the unhealable status,

Well, that might work, but:

1. It's a lot of additional code the developer would have to write.
2. It seems like it could have additional side effects; suppose that the unhealable status has some indicator or icon in the user interface. Would clearing and re-enabling the unhealable status cause this to flash off and on again? What if the developer doesn't want that?

It's also a lot of code to write if another developer wants unhealable to make units unhealable and has to work around the fact that it doesn't. There's no right answer that makes it easier for everyone, one's gain is another's loss. So unless there is a compelling reason to choose one behaviour over another, if it were up to me I would choose the one where the behaviour matches the name. Unfortunately, once a behaviour has been implemented, changing to a cleaner choice means potentially breaking code that it already based on the (incorrect) behaviour, so the "right" choice probably becomes altering the documentation.

And yes, in my case the unhealable icon does flash on at the beginning of the attack and back off when I'm done with it. It's actually rather useful. I'm not sure if that's a mainline thing, the author has been playing with adding status icons so it may be campaign specific. If I didn't want it, I'd look at removing the icon in the status table, or changing it to use blank.png, or something like that. Or I would if unhealable worked properly, but since it doesn't it's of no use to me here and I have to write a bunch of code to make units unhealable instead of just setting the status to unhealable when I want them to be unhealable so I can just not use it and not have to worry about the icon flashing (but since I like it I guess that's one more thing I'd have to code).

[Ravana]

> N_("This unit is unhealable. It cannot be healed by healers or villages and doesn’t benefit from resting."));

[heal_unit] is not one of conditions mentioned.

[gnombat]

I would if unhealable worked properly, but since it doesn't it's of no use to me here

I don't understand - isn't it just a matter of adding one more filter condition for the status? It seems like it should be pretty trivial to get it working, unless I'm misunderstanding what you're trying to do.

[white_haired_uncle]

> N_("This unit is unhealable. It cannot be healed by healers or villages and doesn’t benefit from resting."));

[heal_unit] is not one of conditions mentioned.

Exactly. The statement "this unit is unhealable" by its nature covers all conditions. Unhealable is binary, you either are or you are not. Adding specific instances where it happens to be true makes the first sentence untrue. If the unit was "resistant to healing" or "difficulty to heal" or whatever it would be fine, but unhealable means that it cannot be healed, period (Webster's lists the def as "incapable of being healed").

Now, if we were to accept that it's simply a matter of an unfortunate choice of a name and it's not practical to change, which I think is the reasonable position, the "correct" thing to do would then be to fix the documentation to say "This unit is unhealable. It cannot be healed, except by X, Y and Z." or "This unit is unhealable. It can only be healed by...".

I don't understand - isn't it just a matter of adding one more filter condition for the status? It seems like it should be pretty trivial to get it working, unless I'm misunderstanding what you're trying to do.

For most attacks/specials, it's just a matter of an extra filter condition or two, which is not all that difficult as long as you are careful to keep the type correct (unit vs second_unit, etc). That's not too bad. There are a couple of exceptions where adding an extra filter condition would require duplicating the event, one with filter and one without, or coding around the filter, either of which is not terrible. And I don't have a problem with any of this, because it's necessary for someone to do (if unhealable worked the other way, it would save me N lines of code, but cost someone else an extra N lines of code, so I can't really complain that I just happened to be the one who drew the short straw this time).

The difficult part is setting a unit to unhealable and watching it get healed for no apparent reason, then trying to figure out what is causing the healing in each case vs the cases where it doesn't, then determine what those end cases have in common, realizing that unhealable probably != unhealable and that [heal_unit] is one of the exceptions, and testing the theory. At that point, the coding is pretty straight forward, the only remaining difficult part is trying to think of every other end case that could cause healing and considering if not testing whether those end cases will effect you.

Probably the worst part of all of this is it happens at a time which is difficult if not impossible to interrupt (in my case during combat, it would be worse if I was trying to troubleshoot it happening during the healing that happens somewhere around turn refresh / side turn / whatever).

[gnombat]

> N_("This unit is unhealable. It cannot be healed by healers or villages and doesn’t benefit from resting."));

[heal_unit] is not one of conditions mentioned.

Exactly. The statement "this unit is unhealable" by its nature covers all conditions. Unhealable is binary, you either are or you are not. Adding specific instances where it happens to be true makes the first sentence untrue. If the unit was "resistant to healing" or "difficulty to heal" or whatever it would be fine, but unhealable means that it cannot be healed, period (Webster's lists the def as "incapable of being healed").

Now, if we were to accept that it's simply a matter of an unfortunate choice of a name and it's not practical to change, which I think is the reasonable position, the "correct" thing to do would then be to fix the documentation to say "This unit is unhealable. It cannot be healed, except by X, Y and Z." or "This unit is unhealable. It can only be healed by...".

Keep in mind - this particular string is intended to be displayed to the players / end-users, right? So I wouldn't expect it to start talking about WML syntax. That sort of thing should go in the wiki, either in the unhealable status documentation or the [heal_unit] documentation (or both).

[white_haired_uncle]

That sort of thing should go in the wiki, either in the unhealable status documentation or the [heal_unit] documentation (or both).

Yes, it should be in both.

And now that I think of it, [heal_unit] could take a parameter, honor_unhealable=yes|no (default no, since that's the way it works today). EDIT: Or is that just [filter_wml][not]status=unhealable ?

[gnombat]

Or is that just [filter_wml][not]status=unhealable ?

No need for [filter_wml] - just [filter][not]status=unhealable should work (status is part of the StandardUnitFilter).

[white_haired_uncle]

Or is that just [filter_wml][not]status=unhealable ?

No need for [filter_wml] - just [filter][not]status=unhealable should work (status is part of the StandardUnitFilter).

Yeah, I saw that somewhere after posting. But from what I've read about [filter_wml] it sounds like there's no penalty for using it where it is not necessary, and that way you don't need to remember if you need it or not in cases you don't deal with regularly. ?

[gnombat]

Yeah, I saw that somewhere after posting. But from what I've read about [filter_wml] it sounds like there's no penalty for using it where it is not necessary, and that way you don't need to remember if you need it or not in cases you don't deal with regularly. ?

The documentation for [filter_wml] says, "Note that this is slower than other methods, so if possible it's better to use other filter keys and tags..."