event with variables in their name are handled incorrectly.

[gfgtdf]

wesnoth hndles events with variales in their names incorrectl in multiple ways:

1) The events are currently not executed in the order as they appear in the wml, (events without variables are executed first then event with variables are executed)

2) the namecheck now happens eariler than before, which is a behviour change in case that one event changes a variable then it then used in the evenname of another event.

3) THe space ->underscrore replacement breaks eventnamees, for eventnames with spaces in their inline formula (name = "turn $($num_turns - 1)") or an even easier example turn $num_turns end" which would become turn_$num_turns_end"

[Vultraz]

1 was never a feature. I don't understand 2. I can fix 3 by reverting the commit in question.

[gfgtdf]

sure 1 was a feature, events were executed in the order in which they appear in the wml file, and it's qute likeley that there were addons or maybe even mainline that relies on that. (we also have a similar test which does explcitly for the case that an eventname contaisn variables though https://github.com/wesnoth/wesnoth/blob/master/data/test/scenarios/order_of_nested_events.cfg)

about 2, this was about about of you for example have a name="die" and a name="$var", and the first event does {VARIABLE var die} the second event is now not fired which it might have been previously. But this is not really an important one.

actually i don't realyl know any usecases for events with variables in their names in the first place, which is also why i didn't mark this as a blocker immidiately. But you never know what workarounds umc devs come up with.

[Vultraz]

Do we guarantee 1 anywhere?

[gfgtdf]

well i don't know whether it's in the wiki but there are 1) many post on the forums by diferent devs saying people can rely on this behviour, 2) a wml unit test that checks exactly that.

[Vultraz]

God dammit. This damn API is like Medusa.

[Pentarctagon]

@gfgtdf That unit test doesn't actually show that. If it did, then it would fail since the nested start event occurs first in the WML file. Rather, it shows that events are executed in the order they are created, since the nested start event isn't created until the prestart event has been executed.

Regardless though, I'd suggest adding a unit test or two to specifically check this, such as:

{GENERIC_UNIT_TEST "order_of_variable_events1" (
    [event]
        name=start
        {VARIABLE t 1}
    [/event]
    [event]
        name="turn $t"
        first_time_only=no
        {VARIABLE_OP t add 1}
        [end_turn][/end_turn]
    [/event]
    [event]
        name=turn 2
        {RETURN ({VARIABLE_CONDITIONAL t equals 3})}
    [/event]
)}

and

{GENERIC_UNIT_TEST "order_of_variable_events2" (
    [event]
        name=start
        {VARIABLE t 1}
    [/event]
    [event]
        name=turn 2
        {RETURN ({VARIABLE_CONDITIONAL t equals 2})}
    [/event]
    [event]
        name="turn $t"
        first_time_only=no
        {VARIABLE_OP t add 1}
        [end_turn][/end_turn]
    [/event]
)}

edit- And for point 3 in the first post:

{GENERIC_UNIT_TEST "event_name_variable_substitution" (
    [event]
        name=start
        {VARIABLE t 1}
    [/event]
    [event]
        name="turn 1"
        [end_turn][/end_turn]
    [/event]
    [event]
        name="turn $t end"
        {RETURN ([true][/true])}
    [/event]
)}

[gfgtdf]

well the unit tests intends to test that the events are executd in the order in which they are added, and for toplevel events this is the order in which they appear in the wml file. This is in particular true since there isn't really a difference beween toplevel and nested events once they are added (the nested [event] tag is just a normal actionwml tag that adds an event to the end of the event list). This becomes even clearer when you look at the safefiles after the [event] actionwml is executed, where you can see the previously nesed event now at toplevel.

[CelticMinstrel]

actually i don't realyl know any usecases for events with variables in their names in the first place,

Uh, you gave the quintessential usecase in your previous post:

for eventnames with spaces in their inline formula (name = "turn $($num_turns - 1)") or an even easier example turn $num_turns end" which would become turn_$num_turns_end"

[CelticMinstrel]

(However, in my experience the use-case for variables in the event name involves them being evaluated at add time, not at trigger time. Did the engine always support the latter? I imagine there could be use-cases for it though.)

[gfgtdf]

Uh, you gave the quintessential usecase in your previous post:

hmm yes but they could easily be implemented with turn event + a filter. (if you want the variables to be evaluated at runtime).

However, in my experience the use-case for variables in the event name involves them being evaluated at add time, not at trigger time. Did the engine always support the latter?

Yes it did, but if you have a nested event with delayed_variable_subsitution=no then it will ofc evaluate right away.