Wesnoth Lua Pack: Development Thread

Discussion of Lua and LuaWML support, development, and ideas.

Moderator: Forum Moderators

Post Reply
User avatar
8680
Moderator Emeritus
Posts: 742
Joined: March 20th, 2011, 11:45 pm
Location: The past

Re: Wesnoth Lua Pack: Development Thread

Post by 8680 »

I hereby propose the inclusion into this Lua Pack of my [modify_unit_attacks] tag. This tag is an easy yet powerful solution to the years-old problem of editing the properties of a unit’s attacks in a manner that is both easily done and easily undone. See my linked post for an example of its usage.

I hope I have conducted this marketing pitch submission proposal correctly.
User avatar
Elvish_Hunter
Posts: 1575
Joined: September 4th, 2009, 2:39 pm
Location: Lintanir Forest...

Re: Wesnoth Lua Pack: Development Thread

Post by Elvish_Hunter »

8680 wrote:I hope I have conducted this marketing pitch submission proposal correctly.
Sure you did. However, I opened the Lua script and did a quick check. I noticed one thing at the start:

Code: Select all

local load, pairs, tonumber, tostring, type, helper =
	loadstring or load, pairs, tonumber, tostring, type,
	wesnoth.require "lua/helper.lua"
Why are you overriding (not really, because you're overriding them with copies of themselves) the standard functions load, pairs, tonumber, tostring and type?

Code: Select all

local attackHasDamage = attackHasDamage
Also, once that you declared a function as local, you don't need to assign it to a local variable. You can, of course, assign its return value to a local variable. Remeber that in Lua functions are already variables, and a proof of this can be obtained by iterating through the global _G table.

Code: Select all

	description="thrown ?"
	range=ranged
	damage="?-1"
	number="math.floor(?/2)"
My opinion is that such a non-standard syntax in the WML construct will become confusing in the long run. Maybe a better option will be to add support for a hypothetical $this_attack, like in [modify_unit] and [harm_unit] $this_unit is supported, thus allowing use of FormulaAI syntax:

Code: Select all

damage="$($this_attack.damage-1)"
In conclusion, the tag itself looks promising; however, I can't help but notice that there are still several rough edges that need to be smoothed.
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
8680
Moderator Emeritus
Posts: 742
Joined: March 20th, 2011, 11:45 pm
Location: The past

Re: Wesnoth Lua Pack: Development Thread

Post by 8680 »

I know Lua well; I know it has first-class functions. I copy pointers to global functions to local variables for efficiency:
Lua-Users wiki wrote:Local variables are very quick, since they are accessed by index. If possible, make global variables local (weird, eh?). Seriously, it works great and indexed access is always going to be faster than a hash lookup.
I’ve implemented $this_attack (I can see how it would be useful, for accessing keys other than the one currently being processed), but I haven’t tested it.

I assume cfg.__parsed etc. evaluate $(formulas) as well as interpolating $variables, because I don’t know of any functions specifically for formula evaluation, so formulas should already work.
Attachments
modify_unit_attacks.lua.gz
(1.69 KiB) Downloaded 1018 times
Luther
Posts: 128
Joined: July 28th, 2007, 5:19 pm
Location: USA

Re: Wesnoth Lua Pack: Development Thread

Post by Luther »

8680 wrote:I know Lua well; I know it has first-class functions. I copy pointers to global functions to local variables for efficiency:
Lua-Users wiki wrote:Local variables are very quick, since they are accessed by index. If possible, make global variables local (weird, eh?). Seriously, it works great and indexed access is always going to be faster than a hash lookup.
Premature optimization has a significant cost in readability. From http://lua-users.org/wiki/LocalsVsGlobals:
Performance: The above point implies that locals can be a bit faster (OptimisingUsingLocalVariables). Still, table indexing is a fairly efficient operation in Lua (e.g. strings are interned with precomputed hash), so this point can often be ignored unless profiling indicates optimization is needed.
User avatar
8680
Moderator Emeritus
Posts: 742
Joined: March 20th, 2011, 11:45 pm
Location: The past

Re: Wesnoth Lua Pack: Development Thread

Post by 8680 »

Any optimization may cost readability. “Premature” optimization has no greater cost in that regard than other kinds. For me, assigning global function pointers to local variables allows me to give the functions names that I find more immediately comprehensible. I do recognize that this practice may have an adverse effect upon the script’s maintainability for others; however, I am accustomed to working alone, and battlestar, for whom I originally wrote the script, uses it as a black box, so this was not a concern. I see that it now is a concern, but is it enough of one to be a blocking issue?
User avatar
Elvish_Hunter
Posts: 1575
Joined: September 4th, 2009, 2:39 pm
Location: Lintanir Forest...

Re: Wesnoth Lua Pack: Development Thread

Post by Elvish_Hunter »

8680 wrote:I copy pointers to global functions to local variables for efficiency:
Usually, I read that global variables should be converted to local for one main reason: if two functions use the same variable name, and said variable is global, then some unpredictable disasters (bugs) may happen. That said, it's always good practice making variables local instead of global in every programming language.
What you quoted from the wiki may be true for data types passed by value (like numbers); but as you say, functions are pointers:

Code: Select all

function test3()
	print( debug.getinfo(print).func )
	local blah=print
	print( debug.getinfo(blah).func )
end

> test3()
function: 01E3EF60
function: 01E3EF60
The hex number is the memory address where currently is the print() function. It's the same even for the local variable.
Then I decided to make a quick benchmark:

Code: Select all

function test()
	local start = os.clock()
	local i=0
	while i<1000 do
		print('test')
		i=i+1
	end
	print('Test 1 time elapsed: ' .. os.clock()-start )
end

function test2()
	local print=print
	local start = os.clock()
	local i=0
	while i<1000 do
		print('test')
		i=i+1
	end
	print('Test 2 time elapsed: ' .. os.clock()-start )
end
And these are the outputs that I collected. Since we don't have the BBCode for tables, have an image:
lua benchmark.png
lua benchmark.png (6.34 KiB) Viewed 88547 times
The first value (0.405) is the one that I obtained after opening the Lua interpreter, as I noticed that the first command is always slower. So, if we remove the first pair of results, we can see that the time spent is the same. In other words: since functions are passed by reference, there is no performance gain assigning standard functions to local variables, but only the chance to introduce obscure bugs.
8680 wrote:I assume cfg.__parsed etc. evaluate $(formulas) as well as interpolating $variables, because I don’t know of any functions specifically for formula evaluation, so formulas should already work.
Yes, that's right. On the other hand, cfg.__literal does not perform such substitution, that can be done with wesnoth.tovconfig().
8680 wrote:I see that it now is a concern, but is it enough of one to be a blocking issue?
I'd say yes. Not only this is a coding style completely different from both mainline wml-tags.lua and WLP, but so far I haven't seen any Lua script that reassigns standard library functions to local variables.
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
8680
Moderator Emeritus
Posts: 742
Joined: March 20th, 2011, 11:45 pm
Location: The past

Re: Wesnoth Lua Pack: Development Thread

Post by 8680 »

Firstly, I was already convinced. Secondly, kiloiterations are nothing; back when I benchmarked Lua I did so for gigaiterations. Thirdly, you have I/O in your loops, making performance I/O bound, making the performance gain from the local variable even more negligible.
Elvish_Hunter wrote:What you quoted from the wiki may be true for data types passed by value (like numbers); but as you say, functions are pointers [...]
And those pointers are passed by value, and they are the same size as numbers.
Elvish_Hunter wrote:I'd say yes.
Very well. I withdraw my proposal.

Edit:
pauxlo wrote:If it is only slightly faster with the local declaration, it isn't worth the loss in readability.
I concur, but for me, it is a gain in readability, and I didn’t originally expect anyone but me to read that script.
User avatar
pauxlo
Posts: 1047
Joined: September 19th, 2006, 8:54 pm

Re: Wesnoth Lua Pack: Development Thread

Post by pauxlo »

When deciding if it is useful to use local variables instead of global ones when accessing functions, there is no point in doing benchmarks either with print or with loops of millions of iterations of some function calls never used.
Do benchmarks with the actual function/tag being used by Wesnoth.

If it is only slightly faster with the local declaration, it isn't worth the loss in readability.
CIB
Code Contributor
Posts: 625
Joined: November 24th, 2006, 11:26 pm

Re: Wesnoth Lua Pack: Development Thread

Post by CIB »

Maybe someone else would find this useful as well? Getting a unit to "fake move toward a destination with X movepoints" isn't as trivial as it might be.

Code: Select all

function move_toward(unit, x, y, turns)
	local path = wesnoth.find_path(unit, x, y)
	unit.moves = turns
	
	local furthest = nil
	for i, tile in ipairs(path) do
		local check_path = wesnoth.find_path(unit, tile[1], tile[2], {max_cost=turns})
		if #check_path > 0 then furthest = check_path end
	end
	
	local destination = furthest[#furthest]
	if not destination then return end
	
	helper.move_unit_fake({id = unit.id}, destination[1], destination[2])
end
User avatar
Elvish_Hunter
Posts: 1575
Joined: September 4th, 2009, 2:39 pm
Location: Lintanir Forest...

Re: Wesnoth Lua Pack: Development Thread

Post by Elvish_Hunter »

After a long time, I finally uploaded the 1.7.0 version of the Wesnoth Lua Pack on the 1.12 add-ons server. As usual, there are several changes in this version. Have a look at the changelog:

Code: Select all

Version 1.7.0
   * Removed file ai_helper.lua.
   * Rewritten [get_numerical_minimum] and [get_numerical_maximum]
   * Completely reworked layout and code of HTML readme
   * Enabled Pango markup in [alert], [confirm], [prompt] and [item_dialog]
   * Added [loot] tag, by Elvish_Hunter
   * Added [choice_box] tag, by Elvish_Hunter
   * Bumped Wesnoth version requirement to 1.11.9
   * Fixed a bug in color selection that made [show_side_debug] not working
   * Better indentation of the gui-tags.lua file
The hardest thing to do was updating the HTML file: I found out that the tutorial that I followed back then taught me several obsolete practices :evil: , and so I ended up reading more tutorials and doing a lot of testing. It wasn't hard in itself, but reading and testing was very time consuming.
Anyway, have fun using it and don't forget to report any bug that you may find! ;)
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)
AI
Developer
Posts: 2396
Joined: January 31st, 2008, 8:38 pm

Re: Wesnoth Lua Pack: Development Thread

Post by AI »

Wescamp.py gave me the following output:

Code: Select all

[WARNING 2014-04-11 13:08:13,143]
pot-update in addon Wesnoth_Lua_Pack:
expected closed node 'command' got '{TAG}' at ./Wesnoth_Lua_Pack/utils.cfg:64 at /home/ai/bin/wmlxgettext line 179, <FILE> line 64.
non-empty node stack at end of ./Wesnoth_Lua_Pack/utils.cfg at /home/ai/bin/wmlxgettext line 203, <FILE> line 122.
WML seems invalid for ./Wesnoth_Lua_Pack/utils.cfg, node info extraction forfeited past the error point at /home/ai/bin/wmlxgettext line 210.
This actually seems to be a bug in wmlxgettext, and I'll see if I can fix that.
Anonymissimus
Inactive Developer
Posts: 2461
Joined: August 15th, 2008, 8:46 pm
Location: Germany

Re: Wesnoth Lua Pack: Development Thread

Post by Anonymissimus »

AI wrote:This actually seems to be a bug in wmlxgettext, and I'll see if I can fix that.
Gnihihihi.
I had written that file and tested it recently (umc dev rev 19589).
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
Celtic_Minstrel
Developer
Posts: 2158
Joined: August 3rd, 2012, 11:26 pm
Location: Canada
Contact:

Re: Wesnoth Lua Pack: Development Thread

Post by Celtic_Minstrel »

The [scatter_units] tag in WLP 1.7 sometimes raises an error when placing units. The text of the error is "bad argument #1 to 'put_unit' (invalid location)". Any idea what would cause this? I'm not quite sure if it's a bug in my scenario or in the WLP, though I can't see anything wrong with the location filters I'm using. Maybe it could happen if the location filter doesn't match any locations? (In that case, I'd expect the tag to just do nothing.)
Author of The Black Cross of Aleron campaign and Default++ era.
Former maintainer of Steelhive.
User avatar
Celtic_Minstrel
Developer
Posts: 2158
Joined: August 3rd, 2012, 11:26 pm
Location: Canada
Contact:

Re: Wesnoth Lua Pack: Development Thread

Post by Celtic_Minstrel »

I think I figured out the cause of this - I believe it happens if [scatter_units] happens to choose a location on the map edge for placement.
Author of The Black Cross of Aleron campaign and Default++ era.
Former maintainer of Steelhive.
User avatar
Elvish_Hunter
Posts: 1575
Joined: September 4th, 2009, 2:39 pm
Location: Lintanir Forest...

Re: Wesnoth Lua Pack: Development Thread

Post by Elvish_Hunter »

Celtic_Minstrel wrote:I think I figured out the cause of this - I believe it happens if [scatter_units] happens to choose a location on the map edge for placement.
Do you have a WML test case available? If so, can you please attach it? If not, I can make one easily, but if you can this'll make my fix faster ;)
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)
Post Reply