[modify_unit] disappearances/bug

[Ghite]

This is what I suspect to be an error in the Battle for Wesnoth 1.12.0.

I have the following code set inside an event every side turn in a scenario. The purpose it to add traits to a side when its leader begins a turn on a variable hex.

When the the extra traits are given to the side (during the [modify_unit] tag), all other leaders in the game disappear at the time of the [modify_unit].

Code: Select all

#define UPGRADEIFON SIDE
        first_time_only=no
        [if]
            [have_unit]
                canrecruit=yes
                x,y=$currdest[{SIDE}].x,$currdest[{SIDE}].y
                side={SIDE}
            [/have_unit]
            [then]
		[message]
			message="premodify"
		[/message]
                [modify_unit]
                    [filter]
                        side={SIDE}
                    [/filter]
                    {TRAIT_RESILIENT}
                    {TRAIT_INTELLIGENT}
                    {TRAIT_DEXTROUS}
                    {TRAIT_STRONG}
                    {TRAIT_QUICK}
                [/modify_unit]
		[message]
			message="postmodify"
		[/message]
            [/then]
        [/if]
#enddef

Advice, insights, and correction would be helpful and welcome.

[doofus-01]

I don't see what exactly is causing it, I suspect it's something outside the snippet you posted. It would be pretty strange if [modify_unit] tag killed all leaders. Three suggestions:
1. Try adding just one trait, see if that still causes problems
2. Check the indices on the $currdest[{SIDE}].x stuff. Should SIDE=1 reference the second element in that "currdest" array?
3. Post the entire thing, including all the event and everywhere currdest array is defined and called upon. That way we don't have to guess as much.

[tekelili]

I would suggest use :debug command and then :inspect command. take a look to units and check they were really killed after [modify_unit] (it could be just a image display bug). If they are beign really killed by [modify_unit] then I would suggest check if it also kills undead leaders with no traits, just to discard some weird behavior with them.

[Elvish_Hunter]

2. Check the indices on the $currdest[{SIDE}].x stuff. Should SIDE=1 reference the second element in that "currdest" array?

Yes, it should, because in WML (and almost any other programming language except Lua) arrays start with index 0.
By the way...

Code: Select all

                    {TRAIT_RESILIENT}
                    {TRAIT_INTELLIGENT}
                    {TRAIT_DEXTROUS}
                    {TRAIT_STRONG}
                    {TRAIT_QUICK}

Traits don't work like that. You'll have to wrap them inside a [modifications] tag (see this wiki page).

[doofus-01]

2. Check the indices on the $currdest[{SIDE}].x stuff. Should SIDE=1 reference the second element in that "currdest" array?

Yes, it should, because in WML (and almost any other programming language except Lua) arrays start with index 0.

Reason I asked is because SIDE=0 is strange, I don't think there is a side zero. How do you know what he intended?

By the way...

Code: Select all

                    {TRAIT_RESILIENT}
                    {TRAIT_INTELLIGENT}
                    {TRAIT_DEXTROUS}
                    {TRAIT_STRONG}
                    {TRAIT_QUICK}

Traits don't work like that. You'll have to wrap them inside a [modifications] tag (see this wiki page).

No, you don't need the [modifications] tag, unless something changed in the last few 1.11/1.12 releases, in which case the wiki needs to be updated ( http://wiki.wesnoth.org/DirectActionsWM ... fy_unit.5D )

[Elvish_Hunter]

Reason I asked is because SIDE=0 is strange, I don't think there is a side zero. How do you know what he intended?

OK, perhaps my answer was unclear. I was attempting to explain this behaviour, so: SIDE=1 should reference the second element in "currdest", because in WML (and almost any other programming language except Lua) arrays start with index 0. This means that, in order to access the array element corresponding to side's number, one should subtract 1 from that value, and this can be done either by using an intermediate variable (easier for beginners), or by using the FormulaAI syntax.

No, you don't need the [modifications] tag, unless something changed in the last few 1.11/1.12 releases, in which case the wiki needs to be updated ( http://wiki.wesnoth.org/DirectActionsWM ... fy_unit.5D )

Huh, so that's what "Can add traits with immediate effect" means . It's quite unclear, and also incomplete: I just checked the Lua source code for that tag, and found out that the same could be said for [object]s:

Code: Select all

elseif current_tag == "object" or current_tag == "trait" then
	...
	wesnoth.add_modification(unit, current_tag, current_table[2])

I'm going to edit the wiki.
EDIT: done. I replaced that phrase with It can add [trait]s and [object]s without needing to wrap them inside a [modifications] tag.

[Anonymissimus]

Huh, so that's what "Can add traits with immediate effect" means

No.
Back when [modify_unit] didn't exist yet, things, traits especially, often came into effect only once the unit leveled into itself for instance, and people were frequently asking about this in the wml workshop. The wesnoth.add_modification lua function used by modify_unit doesn't suffer from that. Please don't delete this.

[tekelili]

2. Check the indices on the $currdest[{SIDE}].x stuff. Should SIDE=1 reference the second element in that "currdest" array?

Yes, it should, because in WML (and almost any other programming language except Lua) arrays start with index 0.

Reason I asked is because SIDE=0 is strange, I don't think there is a side zero. How do you know what he intended?

Ehem... {SIDE} is a macro argument, and there is no reason why it could be zero unless coder call macro with such a value, no matter he uses that argument in an array. From my point of view, it is not helpfull for bug reports, burden reporter with out of point questions that require "extra work" to answer, when the fact is that any of us (me included) took a minute to test [modify_unit] with lot of traits in an scenario, just to be sure bug is not pretty easy to reproduce.

He introduced in his code [message] tags that works as debuging tags, so he deserves some respect in his efforts to report the issue (he could be wrong, but he took quite effort trying to avoid it).

edit: Even if he calls macro with zero as argument for {SIDE}, it should be harmless in a [have_unit] tag, and I would see as a bug that...

Code: Select all

[modify_unit]
    [filter]
        side=0
    [/filter]
[/mofify_unit]

...were affecting units from different sides.

[doofus-01]

2. Check the indices on the $currdest[{SIDE}].x stuff. Should SIDE=1 reference the second element in that "currdest" array?

Yes, it should, because in WML (and almost any other programming language except Lua) arrays start with index 0.

Reason I asked is because SIDE=0 is strange, I don't think there is a side zero. How do you know what he intended?

Ehem... {SIDE} is a macro argument, and there is no reason why it could be zero unless coder call macro with such a value, no matter he uses that argument in an array. From my point of view, it is not helpfull for bug reports, burden reporter with out of point questions that require "extra work" to answer, when the fact is that any of us (me included) took a minute to test [modify_unit] with lot of traits in an scenario, just to be sure bug is not pretty easy to reproduce.

He introduced in his code [message] tags that works as debuging tags, so he deserves some respect in his efforts to report the issue (he could be wrong, but he took quite effort trying to avoid it).

Ehem, yourself, chief. He said side={SIDE}, so there will be a side=0 if he tries to access the currdest[0]. And spare us the lectures please.

Huh, so that's what "Can add traits with immediate effect" means

No.
Back when [modify_unit] didn't exist yet, things, traits especially, often came into effect only once the unit leveled into itself for instance, and people were frequently asking about this in the wml workshop. The wesnoth.add_modification lua function used by modify_unit doesn't suffer from that. Please don't delete this.

Elvish_Hunter is right that the [modify_unit] tag and documentation was confusing though. By the way, when would someone want to use [object] inside [modify_unit]?

[tekelili]

Ehem, yourself, chief. He said side={SIDE}, so there will be a side=0 if he tries to access the currdest[0].

I still dont see why side=0 could cause behavior he described.

And spare us the lectures please.

Sure.

[tekelili]

Ghite: I tested these events in a 2p mainline map and couldnt reproduce bug you described. What else in your scenario do you think could be causing it?

Code: Select all

[event]
    name=turn 1


                    [modify_unit]
                        [filter]
                            side=0
                        [/filter]
                        {TRAIT_RESILIENT}
                        {TRAIT_INTELLIGENT}
                        {TRAIT_DEXTROUS}
                        {TRAIT_STRONG}
                        {TRAIT_QUICK}
                    [/modify_unit]
[/event]

Code: Select all

[event]
    name=turn 1


                    [modify_unit]
                        [filter]
                            side=1
                        [/filter]
                        {TRAIT_RESILIENT}
                        {TRAIT_INTELLIGENT}
                        {TRAIT_DEXTROUS}
                        {TRAIT_STRONG}
                        {TRAIT_QUICK}
                    [/modify_unit]
[/event]

[Elvish_Hunter]

Please don't delete this.

Fine, I'll correct it again. No big deal.
EDIT: Fixed in this way: "It can add [trait]s and [object]s without needing to wrap them inside a [modifications] tag, and their effects are applied immediately".

By the way, when would someone want to use [object] inside [modify_unit]?

Beats me But it's possible, so it should (and it is, now) be documented nevertheless.

I still dont see why side=0 could cause behavior he described.

SIDE=0 shouldn't cause anything, because in that case the filter won't match any unit. But the point is, the lowest possible value for sides is 1. So that makes the currdest array our prime suspect: what's its content? How is it created? How is it modified?
But it's also possible that another event happens in the same moment and causes this problem: Ghite, can you post the whole scenario?

[tekelili]

I still dont see why side=0 could cause behavior he described.

SIDE=0 shouldn't cause anything, because in that case the filter won't match any unit. But the point is, the lowest possible value for sides is 1. So that makes the currdest array our prime suspect: what's its content? How is it created? How is it modified?
But it's also possible that another event happens in the same moment and causes this problem: Ghite, can you post the whole scenario?

currdest array is only used inside a [have_unit] tag, so I dont see how any wrong value could cause anything but be fired/not fired at wrong moment the [modify_unit] tag. And if it were fired at wrong moments it would have probably been easyly detected by coder. I would really look for a better suspect.

[iceiceice]

Beats me But it's possible, so it should (and it is, now) be documented nevertheless.

I think I don't agree with this, as a general statement. It should only be documented if we intend to support it indefinitely. Not every undocumented aspect of the program's behavior is going to be the same in future versions.

If someone chose not document something, it might be that they just forgot to do this, but it might also be that they weren't happy with the feature, or had future plans that were abandoned, or forgot to remove it or something... just something to keep in mind.