unhealable for incinerate

[white-haired-uncle]

Incinerate (others?) may double-heal. For example, if an incinerated unit goes to a village wesnoth will heal them, and then a turn event will remove incinerate. Wondering if incinerate should set unhealable. If so, turn event may also need to remove poison assuming poison and incinerate should be removed on same turn.

[Dugy]

Do you mean that he simultaneously heals from village and loses incinerate, rather than loses incinerate in the first turn and heals in the following one?

What exactly is unhealable? Does it prevent the unit from healing in that turn and disappear afterwards, like slow?

[white-haired-uncle]

Yes, wesnoth heals him and we cure him on the same turn. If we set unhealable, wesnoth will leave him alone and we can deal with it in the incineration code, removing unhealable when incinerate is removed.

unhealable: if set to 'yes', the unit cannot be healed. This includes the healing by resting.

I assume it stays set until we remove it. Be annoying if not.

I've been meaning to request a heal/cure event. There are a few things that would work much better if there was such a thing. Hard to believe there's an event for petrify, but not heals?

[Dugy]

It looks like unhealable persists. The wiki for slowed says that it's removed at the end of its turn, but there is no such claim for any other status. So the unit would probably stay unhealable until I don't know when, so the incinerate event would have to set unhealable on side turn and remove it on turn refresh.

Though, I am a little demotivated from making changes to incinerate because I am happy that it finally seems to be without bugs.

[white-haired-uncle]

Note: see use of unhealable for furious attacks #756

I just noticed amla_default includes remove poison/slow. This made me wonder if incinerate is cured by advancement (I think it should be). Need to check this at some point.

[Dugy]

I don't think I thought of writing code for removing incinerate on advancement.

[white-haired-uncle]

Looking at this, with out trying to figure out for i = 1,#unit do it looks like it might be clearing all statuses?

I also note it's doing something special for the Elf Ass execution attack -- didn't you also add that to Predator?

(called by advance event):

function wesnoth.wml_actions.advance_stuff(cfg)
--    wesnoth.interface.add_chat_message("advance_stuff")
    local unit = wesnoth.units.find_on_map(cfg)[1].__cfg
    local m = wml.get_child(unit, "modifications")

        local function clear_potions()
                for i = #m, 1, -1 do
                        if m[i][2].sort ~= nil and string.find(m[i][2].sort, "potion") then
                                table.remove(m, i)
                        end
                end
        end

    if loti_needs_advance == nil then
        if unit.type == "Elvish Assassin" then
--          wesnoth.interface.add_chat_message("is assassin")
            local a = { "advancement", { max_times = 1, always_display = true, id = "execution", image = "attacks/bow-elven-magic.png", strict_amla = true, require_amla="",
                { "effect", { apply_to = "remove_attack", name = "execution" }},
                { "effect", { apply_to = "bonus_attack", name = "execution", description = _"execution", icon = "attacks/bow-elven-magic.png", range = "ranged", defense_weight = "0", damage = "-40", merge = true, force_original_attack = "longbow" }}
            }}
            table.insert(m, a)
            clear_potions()
            wesnoth.units.to_map(unit)
        end
        return
    end
    unit = clear_advancements(unit)
    clear_potions()
    for i = 1,#unit do
        if unit[i][1] == "status" then
                unit[i][2] = {}
        end
    end
    wesnoth.units.to_map(unit)
    loti_needs_advance = nil
end

[Dugy]

Looking at this, with out trying to figure out for i = 1,#unit do it looks like it might be clearing all statuses?

Yes.

I also note it's doing something special for the Elf Ass execution attack -- didn't you also add that to Predator?

Yes, Elvish Assassin is supposed to start with an attack that can be provided only as an [effect], so it gets it as an advancement right at the start. This turned out to be a rather bad idea because it resulted in a large number of bugs that took a lot of time to fix.

Predator might get that too, but I was recently giving him an ability to do double damage to demons, so I am not so inclined to give him more improvements.

[white-haired-uncle]

As part of the furious attack stuff, I introduced a couple of counting semaphores:

unit.variables.status.unhealable
unit.variables.status.undrainable

so that if some event needs to set unhealable it will know whether it should clear unit.status.unhealable when it no longer needs the unit to be unhealable (but some other action might). So if an advance event is going to clear the status, it should (probably) clear the semaphores as well. But trying to follow the flow of pre_advance_stuff and advance_stuff has me confused (no matter how many times I look at it, the use of loti_needs_advance just looks backwards). So, if I do need to clear the semaphore when the status gets cleared, where would I put that? Can I just stick it in pre_advance_stuff where it's already working on unit.variables?

And I just realized there's at least one problem with my implementation that has nothing to do with advancement. I need to capture the original value so I know what to set it back to when count=0.

And if status/sem are cleared by advancement, I need to check sem status on completion of "event" and not just decrement a counter which would then go negative.

Ugh.

I think it's time to write a general purpose tool for this.

[white-haired-uncle]

I wrote a generic tool, like:

[unit_status_semaphore]
    id=Efraim
    status=unhealable
    action=incr
[/unit_status_semaphore]

First time you call incr it saves the original status. A call with decr that causes count == 0 will restore original status. That works well. Also added some other untested options like "get/clear/reset" and limited support for status=all.

So if, for example, a furious attack sets the attacker to unhealable in event=attack and then incinerate wants to set the attacker unhealable in defender_hits then at attack_end furious won't remove the unhealable that it originally set because the count > 0.

I'm not sure what to do about advancement clearing the status(es). Advancement can call u_s_s with action=clear status=all, but then if something still thinks it needs to call decr (I can't think of an example of this) it will throw an error. Since there's a action=get_count, that could be checked in WML before calling decr. Or something like ignore_error=yes could be added.

Only works in one direction though. The assumption is that you would always want to set a status and remove it when you're done with it, not the other way around (e.g. you might make a unit temporarily undrainable, but you wouldn't make an undead unit temporarily drainable).

[Dugy]

Yeah, this approach makes sense.

[white-haired-uncle]

BTW, just ran into [specials][heal_on_hit], which sounds like suck if you could apply the right filters?

[Dugy]

BTW, just ran into [specials][heal_on_hit], which sounds like suck if you could apply the right filters?

Yes, I implemented suck before [heal_on_hit] was added.