Call for Code Samples / WML Unit Tests

The place to post your WML questions and answers.

Moderator: Forum Moderators

Forum rules
  • Please use [code] BBCode tags in your posts for embedding WML snippets.
  • To keep your code readable so that others can easily help you, make sure to indent it following our conventions.
User avatar
iceiceice
Posts: 1056
Joined: August 23rd, 2013, 2:10 am

Call for Code Samples / WML Unit Tests

Post by iceiceice »

I'm writing to announce a development feature that I hope will help to solve two problems.
Problem #1: UMC Code Samples / Backwards Compatibility.
Wesnoth is now a mature project, there are many developers who were at the beginning but are no longer active. There is a steady stream of new faces, however it looks like most of the time there are perhaps 5 to 10 times as many active UMC developers as there are active wesnoth developers (who are actively submitting C++ patches). Many of the active wesnoth developers are relatively recent, or may have only learned of wesnoth through the GSOC program.

This all has the consequence that when we develop wesnoth, we basically have to focus just on mainline, and it is already hard enough to test this and keep it working (still need those custom AI's that can play through mainline campaigns automatically ;) ). Even if you have a great add-on, a developer actively working on wesnoth may or may not even have heard of it, let alone understand its workings, and most developers may only be familiar with a tiny fraction of UMC.

This played out recently in a patch submitted by Elvish_Hunter here: we had a great patch come through, and we only realized that it might be incompatible with some UMC after a forum announcement was made and other devs looked at it. This is not a criticism of Elvish_Hunter, who did a great job here, the point is that the system is somewhat flawed.

The fact is, if someone makes an innocuous development change that subtly breaks your add-on, it is totally impractical for us to test for this manually, and so the only way that we will know is if
  1. You tell us.
  2. A robot tells us.
We actually already have a robot, named travis, whose job it is to tell us when we break things. He is now configured to tell us about the new kind of test as well.

If you submit a few simple code samples, placed in scenarios called "unit tests" that test the basic functionality of your add-on, then as soon as we commit changes which break them, travis will come to the irc channel and tell us that it broke.

This isn't a guarantee that we will never decide that we have to break and revise unit tests, but it's a much better scenario because
  1. it doesn't rely on manual human testing
  2. it happens when the developer who made the change is active. If the change happens, and then we wait 6 months to a year before we learn of the problem, that developer may be inactive or otherwise busy... moreover she may have worked for months based on a faulty assumption, and fixing it may be much harder than if she realized the issue in the first place.
If we decide we must revise a unit test you submit to us, if your name is on it then we can tell you that this happened, and show you what the alternative solution is, rather than forcing you to scour the wiki many months later. If we are forced to revise unit tests that we accepted, we might even be able to add a fix to wmllint, so that UMC can be automatically updated, if the fix is simple enough. Hopefully the tests will help to ensure that your add-on will work properly for years to come.

So the new feature is in part intended to be a new way to help us understand and be responsive to the needs of UMC.
Problem #2: Automated testing.
With a mature project like wesnoth, great new features continue to be added, but we aren't still asking ourselves "what is mainline wesnoth going to be when it grows up". There is greater importance than before on, how can we consolidate, improve and maintain what we have.

We have a collection of C++ unit tests which test elementary pieces of the project, and run very quickly at the push of a button. We want more of these, but they are also sometimes difficult to maintain because they may require more specialized knowledge to understand.

However, until now we haven't had any unit tests of the WML / lua API, which is a very important piece of the project. The new feature is a way to do this *without* writing C++ code -- the test is instead just a WML scenario.

Having a solid collection of tests related to WML / lua would help us greatly to be able to safely refactor some parts of the game and fix some segfaults / crashes that are difficult to fix right now.

Additionally, git has special support for the kinds of unit tests we have, which means that if a unit test breaks, we can automatically pinpoint the commit which broke it. This can save a lot of time when a bug recurs.

So the new feature is also intended to generally make it easier to maintain and develop wesnoth, and increase the overall reliability.
What this means for you:
  1. If you are a beginning WML programmer, or a veteran who is learning about a new tag or lua feature, it is quite common to have a simple test scenario where you paste in code to experiment with and see what happens. Don't throw those away! Tests like that may have long term value to the project, please recycle them by taking or minute or two to write some automatic checks, and submitting them to us here. We will likely commit them to the project permanently -- this will help us to avoid breaking them in the future, and also it means that the things you learn from running these tests are more likely to remain true in future versions of wesnoth.

    Even if you consider yourself a "total noob", that may only make your tests more valuable, as you may ask more simple questions that others take for granted. That's exactly the kind of thing we want to be testing. :)
  2. If you are a veteran maintaining a large add-on, please consider taking some small, key samples of your project (maybe a complicated filter you wrote, maybe a lua routine, maybe a subtle interaction between two parts of the engine which you rely on working a certain way, maybe something completely different...) and formatting them as unit test scenarios. This will help us maintain backwards compatibility with your add-on and hopefully save everyone work in the long run. (If you like, you can use unit tests yourself in your development process. Test-driven development is a popular software development philosophy.)
  3. If you are maintaining a mod that replaces core wesnoth mechanics, then this might be particularly helpful to you, as you have to maintain complicated code quite far from the things we normally test, so you might want push-button tests to make it all easier to maintain. But besides this, you have to make many fine assumptions about how the wesnoth engine works, many of which are likely undocumented. The unit test system slightly changes the contract between the developers and the users -- you can submit the tests that capture your assumptions about how the engine does things. Not everything can be tested this way with the current system, but many things can be.
The test system is only on the 1.13 (master) branch, as it was only developed after feature freeze for 1.12. However, 1.12 and 1.13 are not diverged too far yet. The tests that you write may help us to catch bugs in 1.12, esp. if you haven't ported your add-on to 1.12 yet.

The idea is that the tests will complement the wiki documentation to help characterize the actual behavior of the wesnoth engine in various circumstances.
What exactly is a unit test?
A unit test checks that some piece of the wesnoth engine behaves correctly in some circumstance. For this discussion, it is a wesnoth scenario which is meant to run noninteractively. It cannot require human input, or a person looking at the screen. A unit test should ideally test just one WML / lua command, or one macro, or one aspect of the wesnoth engine. If the test passes, then the scenario should end with victory, otherwise it should end in defeat. Like other test scenarios, these tests must use [test] at scenario level. ScenarioWML

It is important that the test is
  • simple, so that we can easily see what's wrong if it breaks,
  • fast, so that we can run hundreds of these things,
  • deterministic, the outcome should always be the same, and not random, so that when a unit test fails it represents an easily reproducible bug that we can fix.
  • commented, all tests should be accompanied by a simple explanation of what is going on. Tests that not everyone can understand are fairly useless...
Almost anything is fair game for a test. If it's not written on the wiki, but it's important for your add-on, that's perfectly fine. If it is written on the wiki, that's fine too -- ideally we would have some tests for every piece of the wiki WML reference that can be tested this way, although we probably won't get so many tests as that. If it's just testing some macro or script you wrote and has nothing to do with the wiki, that's great as well.

If the test is based on some behavior that we feel is... "unexpected"... we will likely tell you so and offer a better solution. Otherwise, if you felt it was a good thing to test, then it probably is.

What NOT to do:
  • You should not try to find a way to copy your entire add-on into a single scenario, peppered with test assertions. For a test to be useful, it needs to be possible to explain in one or two sentences exactly what is being tested. More complicated tests need to be broken up into a series of smaller tests.
  • If testing a macro, you should not reproduce the functionality of the macro in the test. The test should always be much simpler than the code it is testing. Just test a few hard coded inputs, and see that they match an expected, hard coded, output. If your macro takes a number, don't run it on all the numbers from 0 to 100. Just choose a few, maybe a positive number, a negative number, and zero, for example. Some redundancy in tests is a good thing, but writing excessively redundant tests most likely is a waste of your time.
  • Don't use excessive copy-paste or confusing style. Just because it's a test doesn't mean you should adopt different practices from usual.
Here are some example unit tests we made. You can find them in /data/test/scenarios/ in the main wesnoth repo.

The macro "GENERIC_UNIT_TEST" is a care-free way to set up a test scenario.
The macro "ASSERT" tests that a condition holds. If it does, then the test continues normally, otherwise the test fails.
The macro "RETURN" tests a condition, and ends the test with victory if it holds, otherwise the test fails.
Spoiler:
Spoiler:
Spoiler:
Spoiler:
Spoiler:
Spoiler:
Spoiler:
Spoiler:
The {ASSERT} and {RETURN} macros are just convenient wrappers for the [endlevel] tag. I wrote them because they are familiar if you are used to C/C++. If you don't like them, you don't have to use them.
The {GENERIC_UNIT_TEST} macro is just to save you work having to specify a map and sides. If you need to control those details for your test, you don't have to use this macro. (See the last test, for example.) The only requirement for a unit test is that it use the [test] tag at scenario level.

These macros, specific to unit tests, are defined here:
https://github.com/wesnoth/wesnoth/blob ... macros.cfg

You can see all of the tests here:
https://github.com/wesnoth/wesnoth/tree ... /scenarios

The unit tests are run using a command-line flag currently available only in the development version of wesnoth. When wesnoth runs a test, it will try to run the test to completion, and report failure if it fails, times out, or results in a broken replay. If anyone can think of other ways to improve the testing mechanism, then we might add those as well.
How to run unit tests yourself
You don't need to be able to run tests yourself to submit them to us -- you can test code samples as usual, and then format as a test and submit for example.

However, if you do want to run unit test scenarios, first ensure that you have the current "trunk" / "master" development version of wesnoth: v1.13.0-dev WesnothRepository

If you run `wesnoth --help`, you should see:

Code: Select all

$ wesnoth -h
Battle for Wesnoth v1.13.0-dev (ca5c64b-Clean)
Started on Sun May 11 16:16:21 2014

Automatically found a possible data directory at /home/chris/wesnoth-src/clone/wesnoth
Usage: wesnoth [<options>] [<data-directory>]

General options:

...

Testing options:
  -t [ --test ] arg     runs the game in a small test scenario. If specified, 
                        scenario <arg> will be used instead.
  -u [ --unit ] arg     runs a unit test scenario. Works like test, except that
                        the exit code of the program reflects the victory / 
                        defeat conditions of the scenario.
                        0 - PASS
                        1 - FAIL
                        2 - FAIL (TIMEOUT)
                        3 - FAIL (INVALID REPLAY)
                        4 - FAIL (ERRORED REPLAY)
  --showgui             don't run headlessly (for debugging a failing test)
  --timeout arg         sets a timeout (milliseconds) for the unit test. If 
                        unused there is no timeout or threading.
  --noreplaycheck       don't try to validate replay of unit test

...
If you test "two_plus_two", using the "-u" flag to signal a unit test, you should see:

Code: Select all

$ wesnoth -u two_plus_two
Battle for Wesnoth v1.13.0-dev (a9274bd-Modified)
Started on Thu May 15 12:29:33 2014

Automatically found a possible data directory at /home/chris/wesnoth-src/clone/wesnoth

Data directory: /home/chris/wesnoth-src/clone/wesnoth
User configuration directory: /home/chris/.config/wesnoth
User data directory: /home/chris/.local/share/wesnoth/1.13
Cache directory: /home/chris/.cache/wesnoth
PASS TEST : two_plus_two

If you test "two_plus_two_fail", you should see:

Code: Select all

$ wesnoth -u two_plus_two_fail
Battle for Wesnoth v1.13.0-dev (a9274bd-Modified)
Started on Thu May 15 12:30:08 2014

Automatically found a possible data directory at /home/chris/wesnoth-src/clone/wesnoth

Data directory: /home/chris/wesnoth-src/clone/wesnoth
User configuration directory: /home/chris/.config/wesnoth
User data directory: /home/chris/.local/share/wesnoth/1.13
Cache directory: /home/chris/.cache/wesnoth
FAIL TEST : two_plus_two_fail
You should run tests with a timeout, in case they don't end.

Code: Select all

$ wesnoth -u empty_test --timeout 5000
Battle for Wesnoth v1.13.0-dev (a9274bd-Modified)
Started on Thu May 15 12:30:40 2014

Automatically found a possible data directory at /home/chris/wesnoth-src/clone/wesnoth

Data directory: /home/chris/wesnoth-src/clone/wesnoth
User configuration directory: /home/chris/.config/wesnoth
User data directory: /home/chris/.local/share/wesnoth/1.13
Cache directory: /home/chris/.cache/wesnoth
Setting timer for 5000 ms.
Error in SemWaitTimeout!
FAIL TEST (TIMEOUT): empty_test
Even when the test passes, the unit testing system will also check whether your WML results in a valid replay:

Code: Select all

$ wesnoth -u break_replay_with_lua_random
Battle for Wesnoth v1.13.0-dev (a9274bd-Modified)
Started on Thu May 15 12:34:54 2014

Automatically found a possible data directory at /home/chris/wesnoth-src/clone/wesnoth

Data directory: /home/chris/wesnoth-src/clone/wesnoth
User configuration directory: /home/chris/.config/wesnoth
User data directory: /home/chris/.local/share/wesnoth/1.13
Cache directory: /home/chris/.cache/wesnoth
20140515 12:34:56 error replay: We called random 0 times, but the original game called random -99 times.
20140515 12:34:56 error replay: checksum = 9edd5r2fs>3Eob14p9INqpEP0ErHez3GvyCq5QOAvxxsLyFxQsyEBBwyRxFXDkCtGCsssUNBstSSztoGbkVTNyMwdQRbFjOBn3ZEdAWotSUxBrNbwxQauvUIT4zwSSOP
random_calls = 

20140515 12:34:56 error replay: found dependent command in replay while is_synced=false
Error while playing the game:  (game::game_error) game_error: The replay is corrupt/out of sync. It might not make much sense to continue. Do you want to save the game?

Error details:

found dependent command in replay while is_synced=false

Observed failure on replay
FAIL TEST (ERRORED REPLAY): break_replay_with_lua_random
If you want to run a bunch of tests at once, it is intended that you will use a simple script. Here's an example bash script. This will only work on systems with a unix shell, like Linux, MacOS.

Code: Select all

#!/bin/bash
set -e
set -o pipefail
./wesnoth -u unit_test_1 --timeout 10000 2> error.log
./wesnoth -u unit_test_2 --timeout 10000 2> error.log
./wesnoth -u unit_test_3 --timeout 10000 2> error.log
./wesnoth -u unit_test_4 --timeout 10000 2> error.log
./wesnoth -u unit_test_5 --timeout 10000 2> error.log
./wesnoth -u unit_test_6 --timeout 10000 2> error.log
rm error.log
This script runs the tests in sequence, sending the errors to error.log. It gives 10 seconds for each test. If a test fails, the script stops, and you can inspect the error log.

Here's an example batch script for windows which Pentarctagon kindly wrote:

Code: Select all

@echo off
set /p testme= Which file(s) should be tested? 

for %%c in (E:\Battle_for_Wesnoth_1.13\data\test\scenarios\%testme%.cfg) do (
  @echo %%c
  wesnoth-debug.exe -u %%c
)

pause
We also have a much fancier bash script that runs on travis, but it's most likely overkill for UMC needs. If you want to use it, though, you are welcome to. It is here.

If you want to inspect a unit test (because it is failing), use the --showgui option, and you will get a normal video display, for the playthrough and the replay.
Spoiler:
---

So, if you have an idea for a test, whether or not it's based on your add-on, please consider submitting it to us. We will continue to write tests ourselves, but to get the full benefit we would like to get some from UMC as well (particularly, some tests for the lua api would be nice to have). You can write here, or you are welcome to make a github pull request with some tests. If you aren't sure how to format it all properly, we can help you with that.
Anonymissimus
Inactive Developer
Posts: 2461
Joined: August 15th, 2008, 8:46 pm
Location: Germany

Re: Call for Code Samples / WML Unit Tests

Post by Anonymissimus »

I was just browsing old fixed bug reports of mine for suitable wml snippets, things that would have been detected by such a unit test. (This one feels appropriate http://gna.org/bugs/index.php?20734) Maybe that's a general strategy to get more tests ?
Unfortunately though, it seems to me that most bugs I reported wouldn't have been found by such a test.
projects (BfW 1.12):
A Simple Campaign: campaign draft for wml startersPlan Your Advancements: mp mod
The Earth's Gut: sp campaignSettlers of Wesnoth: mp scenarioWesnoth Lua Pack: lua tags and utils
updated to 1.8 and handed over: A Gryphon's Tale: sp campaign
User avatar
iceiceice
Posts: 1056
Joined: August 23rd, 2013, 2:10 am

Re: Call for Code Samples / WML Unit Tests

Post by iceiceice »

Yeah I guess its good practice to write a test when you fix a bug, when you can.

Another possible strategy is to mine the wiki code examples. Then at least if the test breaks it either signals a bug, or it's a reminder to update the wiki.

I remember other issues discussed on the forums that might make good tests, for instance, someone was asking about how first-strike interacts with berzerk, and presumably everyone loaded up a test scenario and watched the video slowly to try to make sense of it. A different way is, write a test, see which way makes it pass, then commit that forever -- if it ever changes then we'll know. Besides this, even if the test isn't targeted at a specific bug, it may still catch bugs it wasn't intended for.
User avatar
tekelili
Posts: 1039
Joined: August 19th, 2009, 9:28 pm

Re: Call for Code Samples / WML Unit Tests

Post by tekelili »

If there is not currently a test to check if 2 different events both name=starts are fired in the order they were defined, I could try write one for it :)

Just a shame multiplayer safety is out of unit test range, was what more interested me :|
Be aware English is not my first language and I could have explained bad myself using wrong or just invented words.
World Conquest II
User avatar
Heindal
Posts: 1343
Joined: August 11th, 2011, 9:25 pm
Location: Germany, Karlsruhe
Contact:

Re: Call for Code Samples / WML Unit Tests

Post by Heindal »

Im not 100% sure, if this is something that can be done with a unit test, I will just mention it. While in Wesnoth version 1.11.13+- the game checks the unit and code for spaces and not allowed characters, it does not check the images. Maybe a unit test could not only check all files and the image folder as well.
The future belongs to those, who believe in the beauty of their dreams.
Developer of: Trapped, Five Fates, Strange Legacy, Epical, UR Epic Era
Dungeonmasters of Wesnoth, Wild Peasants vs Devouring Corpses, Dwarf Dwarfson Dwarvenminer
User avatar
iceiceice
Posts: 1056
Joined: August 23rd, 2013, 2:10 am

Re: Call for Code Samples / WML Unit Tests

Post by iceiceice »

tekelili wrote:If there is not currently a test to check if 2 different events both name=starts are fired in the order they were defined, I could try write one for it :)
No, we don't have a test like that right now. That would be a good test :)
tekelili wrote:Just a shame multiplayer safety is out of unit test range, was what more interested me :|
Yeah so I did think about this.

In the majority of cases replay-safety and mp-safety should be the same -- the info that, for example, the server sends to an mp observer, is more or less the same as what wesnoth will load from a replay.
Spoiler:
There's I guess a possibility that WML could pass a replay test but fail in a networked setting. But for most purposes the replay test should get you 90% of the way there IMO.
Spoiler:
Anyways as far as WC tek goes though, I did have in mind that, for example, we could try to test that the recruit mechanism works and is replay safe, using [do_command] to simulate user recruit actions. That's something that's imo complicated enough that it would be worth it to test it and see that we don't break it. (Although, maybe a simplified version, idk I haven't read your code so I don't know how complicated it actually is.)
Heindal wrote:Im not 100% sure, if this is something that can be done with a unit test, I will just mention it. While in Wesnoth version 1.11.13+- the game checks the unit and code for spaces and not allowed characters, it does not check the images. Maybe a unit test could not only check all files and the image folder as well.
Hmm this is interesting, I wonder if this is an oversight on our part.

I searched for the commits that prohibited spaces and such in filenames, I think it's these two:
https://github.com/wesnoth/wesnoth/comm ... 53ac5686ee
https://github.com/wesnoth/wesnoth/comm ... 1386167918

My feeling is that the game should complain for images just the same as the other kinds of files.

But anyways, yeah I think in principle we should be able to test the code that loads images in a unit test.
Spoiler:
Anonymissimus
Inactive Developer
Posts: 2461
Joined: August 15th, 2008, 8:46 pm
Location: Germany

Re: Call for Code Samples / WML Unit Tests

Post by Anonymissimus »

iceiceice wrote:
tekelili wrote:If there is not currently a test to check if 2 different events both name=starts are fired in the order they were defined, I could try write one for it :)
No, we don't have a test like that right now. That would be a good test :)
This makes up for an excellent test. It is something that could easily break, is important for wml authors and hard to test manually.
It should be generalized - "If we have 2 or more events of the same type, do they fire in the order which appears in the scenario file ?" I'm sure it can be macroized to cover all event types or at least all important ones.
projects (BfW 1.12):
A Simple Campaign: campaign draft for wml startersPlan Your Advancements: mp mod
The Earth's Gut: sp campaignSettlers of Wesnoth: mp scenarioWesnoth Lua Pack: lua tags and utils
updated to 1.8 and handed over: A Gryphon's Tale: sp campaign
User avatar
Elvish_Hunter
Posts: 1575
Joined: September 4th, 2009, 2:39 pm
Location: Lintanir Forest...

Re: Call for Code Samples / WML Unit Tests

Post by Elvish_Hunter »

As we discussed elsewhere, here there is my proposed test.
In a scenario, I need to iterate backwards a WML array, because I need to delete items from it. Sure, it's bad practice, but so far my attempts to do it in the right way (iterate the array, then copy only the elements that pass the check in a new array, then replace the old array with the new array) have gone badly, maybe just because I didn't yet have the right idea. So I developed two helper macros, REV_FOREACH and PREVIOUS, that are specular to FOREACH and NEXT:

Code: Select all

# wmlindent: start ignoring
#define REV_FOREACH ARRAY_VALUE VAR
    # last index of an array is always length-1
    {VARIABLE {VAR} "$(${ARRAY_VALUE}.length)-1"}
    [while]
        [variable]
            name={VAR}
            greater_than_equal_to=0
        [/variable]
        [do]
#enddef
#define PREVIOUS VAR
            [set_variable]
                name={VAR}
                sub=1
            [/set_variable]
        [/do]
    [/while]
    {CLEAR_VARIABLE {VAR}}
#enddef
# wmlindent: stop ignoring
I'm using this code in the First Contact scenario in TSoG, starting from line 1132 onwards. I'm attaching it, so you can see its purpose.
Attachments
05_First_Contact.cfg
(39.74 KiB) Downloaded 674 times
Current maintainer of these add-ons, all on 1.16:
The Sojournings of Grog, Children of Dragons, A Rough Life, Wesnoth Lua Pack, The White Troll (co-author)
User avatar
tekelili
Posts: 1039
Joined: August 19th, 2009, 9:28 pm

Re: Call for Code Samples / WML Unit Tests

Post by tekelili »

tekelili wrote:If there is not currently a test to check if 2 different events both name=starts are fired in the order they were defined, I could try write one for it
I have not forgot this, but it is in the waiting room for some months until I manage to get a campaign working in 1.11 :hmm:

however I employ sometime in think how could be the best way to do and what cases should be tested. I discarded [era] events as it looks not having expected behavior currently. There is other issue I had with introducing an event inside a unit that I have no clue if is expected behavior. As code was complicated, here is a brief description:

I had a recruit event that made basically 2 things: Attach a magic item and modify unit xp unstoring to cause leveling if neccessary. In a particular case, one magic item introduced a name=postadvance event into unit to change halo when levelin. My issue was that posadvanced event was not fired in that recruit event after have copyed it into unit. However I had what I wanted attaching magic item in prerecruit and then causing the advance in recruit event. So the event inside unit behavior was: "I dont exist until current event ends"

Is this expected or a bug?
(this was running BfW 1.10.6 in a pc with Windows)

-----EDIT-----
here is my issue resumed in code: I introduced this in hamlets scenario for my test (tested in BfW 1.10.6)

Code: Select all

    [variables]
        [event]
            id=darkness_aura
            name=post advance
            first_time_only=no
            {VARIABLE unit.halo halo/darkens-aura.png}
            [unstore_unit]
                variable=unit
            [/unstore_unit]
        [/event]
    [/variables]

    [event]
        name=recruit
        first_time_only=no
        [set_variables]
            mode=append
            name=unit.event
            [insert_tag]
                name=value
                variable=event
            [/insert_tag]
        [/set_variables]

        {VARIABLE_OP unit.experience add 50}
        [unstore_unit]
            variable=unit
            fire_event=yes
        [/unstore_unit]
    [/event]
This will cause the very first nuit recruited advance 1 level, but wont show any aura as (imho) is the expected behavior. The second unit recruit and any other one will have an halo, due to event being already writed in scenario (wich proves first unit has it written).

To have desired behavior I used this code

Code: Select all

    [event]
        name=prerecruit
        first_time_only=no
        [set_variables]
            mode=append
            name=unit.event
            [insert_tag]
                name=value
                variable=event
            [/insert_tag]
        [/set_variables]
        [unstore_unit]
            variable=unit
            fire_event=yes
        [/unstore_unit]
    [/event]

    [event]
        name=recruit
        first_time_only=no
        [store_unit]
            variable=unit
            [filter]
                x,y=$unit.x,$unit.y
            [/filter]
        [/store_unit]
        {VARIABLE_OP unit.experience add 50}
        [unstore_unit]
            variable=unit
            fire_event=yes
        [/unstore_unit]
    [/event]
to check additionals store/unstore didnt change anything, this other way to write case 1 also dont work for first unit

Code: Select all

    [variables]
        [event]
            id=darkness_aura
            name=post advance
            first_time_only=no
            {VARIABLE unit.halo halo/darkens-aura.png}
            [unstore_unit]
                variable=unit
            [/unstore_unit]
        [/event]
    [/variables]

    [event]
        name=recruit
        first_time_only=no
        [store_unit]
            variable=unit
            [filter]
                x,y=$unit.x,$unit.y
            [/filter]
        [/store_unit]
        [set_variables]
            mode=append
            name=unit.event
            [insert_tag]
                name=value
                variable=event
            [/insert_tag]
        [/set_variables]
        {VARIABLE_OP unit.experience add 50}
        [unstore_unit]
            variable=unit
            fire_event=yes
        [/unstore_unit]
    [/event]
Be aware English is not my first language and I could have explained bad myself using wrong or just invented words.
World Conquest II
User avatar
iceiceice
Posts: 1056
Joined: August 23rd, 2013, 2:10 am

Re: Call for Code Samples / WML Unit Tests

Post by iceiceice »

tekelili:

I'm not sure exactly what behavior was intended, but looking at your code I think it's not a bug and it's what I would expect. (Someone else may disagree though.)

Here's my understanding:

For every event name, there is a list. When an event is declared, it gets added to the list for that name. When an event name is fired, all of the events fire in the order that they were declared.

However, when we fire an event, if it generates more events, those new events don't go on the list until we are done firing. The mere act of adding events to the list cannot cause events to fire, so this guarantees that the process will stop.

I guess you could imagine that it would be otherwise, and that events should be added to the list immediately but then it would be easy to cause infinite loops.

Edit:

I did some code inspection,

the code that adds the new events to the queue is here I think:

https://github.com/wesnoth/wesnoth/blob ... p.cpp#L137

When this guy is created, it swaps out the event queue,

https://github.com/wesnoth/wesnoth/blob ... p.cpp#L505

and restores it in its destructor. However, I don't understand why there's the check for "!done()" there, it sounds like its for exception handling, but I think it should be putting the newly registered events in the queue regardless...

Anyways I think it's pretty clear that the intended behavior is, "don't think about any new events that got registered until after we are done firing all events." So in your case, if you register the post_advance event at the same time as you unstore the unit, it won't fire the first time, only at subsequent advances.
User avatar
tekelili
Posts: 1039
Joined: August 19th, 2009, 9:28 pm

Re: Call for Code Samples / WML Unit Tests

Post by tekelili »

@iceiceice: If it is expected behavior, then there is no a bug, is fine :)

But I think this should be explained in wiki at [event]. Before WML I had only experience with primitive coding languages and I asumed that [events] were kinda subroutines... and is not intuitive be unable to create a subroutine in memory :?
Be aware English is not my first language and I could have explained bad myself using wrong or just invented words.
World Conquest II
User avatar
iceiceice
Posts: 1056
Joined: August 23rd, 2013, 2:10 am

Re: Call for Code Samples / WML Unit Tests

Post by iceiceice »

tekelili:

Actually when I wrote I didn't completely understand the code, so I went back and ran some actual tests:

Here's the tests I wrote. All of them pass except for the 4th:

Code: Select all

{GENERIC_UNIT_TEST "events_in_events_1" (
    [event]
        name=test
        {VARIABLE pass_test 1}
    [/event]
    [event]
        name=start
        {VARIABLE pass_test 0}
        [fire_event]
            name=test
        [/fire_event]
        {RETURN ({VARIABLE_CONDITIONAL pass_test equals 1})}
    [/event]
)}

{GENERIC_UNIT_TEST "events_in_events_2" (
    [event]
        name=start
        {VARIABLE pass_test 0}
        [event]
            name=test
            {VARIABLE pass_test 1}
        [/event]

        [fire_event]
            name=test
        [/fire_event]
        {RETURN ({VARIABLE_CONDITIONAL pass_test equals 1})}
    [/event]
)}

{GENERIC_UNIT_TEST "events_in_events_3" (
    [event]
        name=start
        [event]
            name=test
            {VARIABLE pass_test 1}
        [/event]
    [/event]        
    [event]
        name=start
        {VARIABLE pass_test 0}

        [fire_event]
            name=test
        [/fire_event]
        {RETURN ({VARIABLE_CONDITIONAL pass_test equals 1})}
    [/event]
)}

{GENERIC_UNIT_TEST "events_in_events_4" (
    [event]
        name=start
        {VARIABLE pass_test 0}

        [fire_event]
            name=test
        [/fire_event]
        {RETURN ({VARIABLE_CONDITIONAL pass_test equals 1})}
    [/event]
    [event]
        name=start
        [event]
            name=test
            {VARIABLE pass_test 1}
        [/event]
    [/event]
)}
So actually the thing I was saying is not accurate, I don't have any idea why your example wasn't working.

(In particular, your example is roughly like test #2, so I would think the event handler in your case would fire if that test is passing.)

It might be a bug, more info to come hopefully...

(Note: Tests run in version 1.13.0 (but I don't think any of the event handler code has changed))


EDIT:

Okay, here are the subsequent tests that I wrote. All of these pass. I think that there might just be something funky going on with your insert_tag syntax?

Code: Select all

{GENERIC_UNIT_TEST "events_in_events_5" (
    [event]
        name=start
        {VARIABLE pass_test 0}
        {UNIT 1 "Orcish Grunt" 1 1 ()}
        [store_unit]
            [filter]
                x=1
                y=1
            [/filter]
            variable=my_unit
            kill=yes
        [/store_unit]
        [event]
            name=post_advance
            {VARIABLE pass_test 1}
        [/event]

        {VARIABLE_OP my_unit.experience add 50}
        [unstore_unit]
            variable=my_unit
            fire_event=yes
        [/unstore_unit]
    [/event]
    [event]
        name=start
        {RETURN ({VARIABLE_CONDITIONAL pass_test equals 1})}
    [/event]
)}

{GENERIC_UNIT_TEST "events_in_events_6" (
    [event]
        name=start
        {VARIABLE pass_test 0}
        {UNIT 1 "Orcish Grunt" 1 1 ()}
        [store_unit]
            [filter]
                x=1
                y=1
            [/filter]
            variable=my_unit
            kill=yes
        [/store_unit]
        [set_variables]
            name=ev0
            [value]
                 name=post_advance
                 {VARIABLE pass_test 1}
            [/value]
        [/set_variables]
        [insert_tag]
            name=event
            variable=ev0
        [/insert_tag]
        [fire_event]
            name=test
        [/fire_event]

        {VARIABLE_OP my_unit.experience add 50}
        [unstore_unit]
            variable=my_unit
            fire_event=yes
        [/unstore_unit]
    [/event]
    [event]
        name=start
        {RETURN ({VARIABLE_CONDITIONAL pass_test equals 1})}
    [/event]
)}

{GENERIC_UNIT_TEST "events_in_events_7" (
    [event]
        name=start
        {VARIABLE pass_test 0}
        {UNIT 1 "Orcish Grunt" 1 1 ()}
        [store_unit]
            [filter]
                x=1
                y=1
            [/filter]
            variable=my_unit
            kill=yes
        [/store_unit]
        [set_variables]
            name=ev0
            [value]
                 name=post_advance
                 {VARIABLE pass_test 1}
            [/value]
        [/set_variables]
        [event]
            name=test
            [insert_tag]
                name=event
                variable=ev0
            [/insert_tag]
        [/event]
        [fire_event]
            name=test
        [/fire_event]

        {VARIABLE_OP my_unit.experience add 50}
        [unstore_unit]
            variable=my_unit
            fire_event=yes
        [/unstore_unit]
    [/event]
    [event]
        name=start
        {RETURN ({VARIABLE_CONDITIONAL pass_test equals 1})}
    [/event]
)}

{GENERIC_UNIT_TEST "events_in_events_8" (
    [event]
        name=start
        {VARIABLE pass_test 0}
        {UNIT 1 "Orcish Grunt" 1 1 ([variables]
                                        [my_event]
                                            name=post_advance
                                            {VARIABLE pass_test 1}
                                        [/my_event]
                                    [/variables])}
        [store_unit]
            [filter]
                x=1
                y=1
            [/filter]
            variable=my_unit
            kill=yes
        [/store_unit]
        [insert_tag]
            name=event
            variable=my_unit.variables.my_event
        [/insert_tag]

        {VARIABLE_OP my_unit.experience add 50}
        [unstore_unit]
            variable=my_unit
            fire_event=yes
        [/unstore_unit]
    [/event]
    [event]
        name=start
        {RETURN ({VARIABLE_CONDITIONAL pass_test equals 1})}
    [/event]
)}
Note: These tests have now been committed to the project.
User avatar
tekelili
Posts: 1039
Joined: August 19th, 2009, 9:28 pm

Re: Call for Code Samples / WML Unit Tests

Post by tekelili »

iceiceice wrote:Okay, here are the subsequent tests that I wrote. All of these pass. I think that there might just be something funky going on with your insert_tag syntax?
My syntax is really funky... but it was for a good reason :P
In my scenario it had totally sense, as I had lot of complicated stuff in my magic items and I had some delayed variable substitution not working until I employed it (I think problem was with feeding event) :eng:

Btw, sorry if I am wrong... but it looked to me that you are writing the post_advance event in the scenario instead inside the unit?

Edit: It is off topic, but just for clarification that was only syntax I found to have this working
Spoiler:
Last edited by tekelili on June 21st, 2014, 9:30 pm, edited 3 times in total.
Be aware English is not my first language and I could have explained bad myself using wrong or just invented words.
World Conquest II
User avatar
Sapient
Inactive Developer
Posts: 4453
Joined: November 26th, 2005, 7:41 am
Contact:

Re: Call for Code Samples / WML Unit Tests

Post by Sapient »

Just wanted to say: this is a good idea. The previous way was to add an example to the scenario-test.cfg. I've added some tests there myself that would be good candidates for inclusion.

- "Start Dynamic Events!" menu item, tests recursive insert_tag behavior including infinite insert recursion prevention

- [filter_radius] test, captures connected villages
http://www.wesnoth.org/wiki/User:Sapient... "Looks like your skills saved us again. Uh, well at least, they saved Soarin's apple pie."
User avatar
iceiceice
Posts: 1056
Joined: August 23rd, 2013, 2:10 am

Re: Call for Code Samples / WML Unit Tests

Post by iceiceice »

tekelili wrote: Btw, sorry if I am wrong... but it looked to me that you are writing the post_advance event in the scenario instead inside the unit?
Yeah so I worked slowly to the hard case, I only put the event in the unit in test 8 above. In that case it still works though.

I will write again after I look at this feeding code...
Post Reply