bumbadadabum's WML tags

[Astoria]

I've decided to put my (limited) knowledge of Lua to use to make some WML tags. So far, I've finished one, but I'll (hopefully) finish the other ideas I have soon.

Tags:

[flash_image]:

This tag blinks an image, like HIGHLIGHT_IMAGE does in core, but in lua, is more compact, and has more options.
Usage:

Code: Select all

[flash_image]
image= --the image to be displayed
x,y= --the location where it will be displayed
delay= --the time the image stays displayed (when blinking more than once, this is also the time between blinks
times= --the amount of times the image is blinked
stays_on_map= (default=yes) --Whether the image stays on the map afterwards
[/flash_image]

coding:

Code: Select all

function wml_actions.flash_image(cfg)

	local items = wesnoth.require "lua/wml/items.lua"

	local delay = cfg.delay or 500
	local image = cfg.image or helper.wml_error("[flash_image] missing required image= attribute.")
	local x = cfg.x or helper.wml_error("[flash_image] missing required x= attribute.")
	local y = cfg.y or helper.wml_error("[flash_image] missing required y= attribute.")
        local times = cfg.times or 1
        local stays_on_map = cfg.stays_on_map or true

        for i = 1, times do
		wesnoth.add_tile_overlay(x, y, {image = image })
		wml_actions.redraw {}
		wesnoth.delay(delay)
		wesnoth.remove_tile_overlay(x, y, image)
		wml_actions.redraw {}
		if  i ~= times then
			wesnoth.delay(delay)
		end
	end

	if stays_on_map then
		wesnoth.delay(delay)
		items.place_image(x, y, image)
	end
end

[Elvish_Hunter]

First of all, let me say that this tag looks like a solid candidate at least for the WLP. So, keep up the good work.
However, there are two things that ought to be fixed or improved:
1) The x, y keys:

Code: Select all

x,y= --the location where it will be displayed

It'll be better if, instead of x and y, you modify the tag to accept a StandardLocationFilter, without the [filter_location] sub-tag.
2) Booleans:

Code: Select all

local stays_on_map = cfg.stays_on_map or true

As you may already know, Lua has only two values equivalent to a boolean false: false and nil. Everything else, including zeros, empty strings and empty tables, is always true. Your intention was to have a "if stays_on_map is not supplied, then it's true", right? Then please consider for a moment what will happen in both situations:
stays_on_map is missing -> missing value is translated by WML as nil -> nil is false -> (false OR true) = true
stays_on_map = no -> no is translated by WML as false -> (false OR true) -> true
Considering that every other value is true, and (true OR true) = true, you have a bug. To solve it, I'd suggest to change the line this way:

Code: Select all

local stays_on_map
if cfg.stays_on_map == false then
    stays_on_map = false
else
    stays_on_map = true
end

[Astoria]

First of all, let me say that this tag looks like a solid candidate at least for the WLP. So, keep up the good work.
However, there are two things that ought to be fixed or improved:
1) The x, y keys:

Code: Select all

x,y= --the location where it will be displayed

It'll be better if, instead of x and y, you modify the tag to accept a StandardLocationFilter, without the [filter_location] sub-tag.
2) Booleans:

Code: Select all

local stays_on_map = cfg.stays_on_map or true

As you may already know, Lua has only two values equivalent to a boolean false: false and nil. Everything else, including zeros, empty strings and empty tables, is always true. Your intention was to have a "if stays_on_map is not supplied, then it's true", right? Then please consider for a moment what will happen in both situations:
stays_on_map is missing -> missing value is translated by WML as nil -> nil is false -> (false OR true) = true
stays_on_map = no -> no is translated by WML as false -> (false OR true) -> true
Considering that every other value is true, and (true OR true) = true, you have a bug. To solve it, I'd suggest to change the line this way:

Code: Select all

local stays_on_map
if cfg.stays_on_map == false then
    stays_on_map = false
else
    stays_on_map = true
end

Thanks for the tips, I'll change it as soon as I have access to a computer.