Iterate over all variables

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

Moderator: Forum Moderators

User avatar
iceiceice
Posts: 1056
Joined: August 23rd, 2013, 2:10 am

Re: Iterate over all variables

Post by iceiceice »

:hmm:

You know, I wonder if the current implementation of the game gives memory leaks if "check_victory" is called during the execution of a lua api function, or any time that "end_turn_exception" or "end_level_exception" (or "network_error") is thrown and propogates through the embedded lua code. Because the "lua stack" does not have a destructor, and neither does any of that code?

Edit: For instance, there is a bug report here which reports other problems when this happens:

https://gna.org/bugs/?22173

But I wonder if things aren't already FUBAR by that point anyways.
gfgtdf
Developer
Posts: 1432
Joined: February 10th, 2013, 2:25 pm

Re: Iterate over all variables

Post by gfgtdf »

No we compile the luaapi as C++ and we added support for exceptions in there:

https://github.com/wesnoth/wesnoth/blob ... do.cpp#L61

https://github.com/wesnoth/wesnoth/comm ... 4183d03470

But reardless of whether luakernel supports exceptions, my plan was still catch those 'inviliad_variablename' exceptions when we use the function.

Elvish_Hunter wrote:While for a Lua coder may make sense, for a WML coder having a variable called _G will be something like Moon Logic: completely unobvious, it'll start making sense only if and when our UMC author starts learning Lua.
not more unobvious than 'length', '__cfg' or similar. Also it could make learning lua easier for people that know wml.

iceiceice wrote: Edit: For instance, there is a bug report here which reports other problems when this happens:

https://gna.org/bugs/?22173
this bug is not related to exceptions.
Scenario with Robots SP scenario (1.11/1.12), allows you to build your units with components, PYR No preperation turn 1.12 mp-mod that allows you to select your units immideately after the game begins.
User avatar
iceiceice
Posts: 1056
Joined: August 23rd, 2013, 2:10 am

Re: Iterate over all variables

Post by iceiceice »

gfgtdf wrote:No we compile the luaapi as C++ and we added support for exceptions in there:
So to be completely clear, you believe it is safe for the lua api functions in src/scripting/lua.cpp to throw exceptions? (Edit: Not just that there is some code _longjmp etc. but that it won't cause memory leaks in the lua stack implementation etc.) This is something we really should know and should document in comments in that file. It is far from obvious at least to me and I assume most devs who "casually observe". The vast majority of those functions are no throw...
gfgtdf wrote:
Elvish_Hunter wrote:While for a Lua coder may make sense, for a WML coder having a variable called _G will be something like Moon Logic: completely unobvious, it'll start making sense only if and when our UMC author starts learning Lua.
not more unobvious than 'length', '__cfg' or similar. Also it could make learning lua easier for people that know wml.
I agree with Elvish_Hunter. Length is pretty obvious. Double underscore things like "__cfg" are never supposed to be used in wml. Lua is not required reading for people who just want to write a little WML.
gfgtdf wrote:this bug is not related to exceptions.
I didn't say that it was, but obviously if you want to call "check_victory" from a codepath that runs through the embedded lua kernel, then the exceptions will propogate back through the kernel...
gfgtdf
Developer
Posts: 1432
Joined: February 10th, 2013, 2:25 pm

Re: Iterate over all variables

Post by gfgtdf »

iceiceice wrote:
So to be completely clear, you believe it is safe for the lua api functions in src/scripting/lua.cpp to throw exceptions? This is something we really should know and should document in comments in that file. It is far from obvious at least to me and I assume most devs who "casually observe". The vast majority of those functions are no throw...
i didnt write that code and i didn't veryfy it. but thats what the code was made for.
iceiceice wrote: I agree with Elvish_Hunter. Length is pretty obvious. Double underscore things like "__cfg" are never supposed to be used in wml. Lua is not required reading for people who just want to write a little WML.
i don't force people to use '_G', just like i don't force people to use lua.
iceiceice wrote:
gfgtdf wrote:this bug is not related to exceptions.
I didn't say that it was, but obviously if you want to call "check_victory" from a codepath that runs through the embedded lua kernel, then the exceptions will propogate back through the kernel...
the kernel catches all exepctions in the code i showed above. And stores those that derive from 'lua_jailbreak_exception' in an internal buffer

then when leaving lua it rethrows those that were stored in the buffer.
Scenario with Robots SP scenario (1.11/1.12), allows you to build your units with components, PYR No preperation turn 1.12 mp-mod that allows you to select your units immideately after the game begins.
Anonymissimus
Inactive Developer
Posts: 2461
Joined: August 15th, 2008, 8:46 pm
Location: Germany

Re: Iterate over all variables

Post by Anonymissimus »

iceiceice wrote:You know, I wonder if the current implementation of the game gives memory leaks if "check_victory" is called during the execution of a lua api function, or any time that "end_turn_exception" or "end_level_exception" (or "network_error") is thrown and propogates through the embedded lua code. Because the "lua stack" does not have a destructor, and neither does any of that code?
Quite possible.
I always considered that function a pain in the ass which should better have been implemented without exceptions, by unwinding the stack with returns and conditions for instance. Even alone for the fact that the MSVC debugger seems to choke at such points in that it doesn't automatically continue at the point the exception is caught. You have to figure it out manually.
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: Iterate over all variables

Post by iceiceice »

:hmm: I thought about this briefly when I was working on this branch: https://github.com/wesnoth/wesnoth/pull/191

I agree there's no good reason that function should throw. But the issue is, check_victory is called in tons of places. For instance when the play controller calls "play_slice()" (just to poll for mouse and keyboard input!) exceptions can get thrown.

In 1.12 and master, gfgtdf has made it so that check_victory is called at the end of every "synchronized" context. So I believe this means that any time you click to attack and kill the enemy leader, there is going to be an exception that goes from inside the synced context, back through the mouse handler code, back through place_slice(), up to the handler somewhere in play_controller.

I don't think we can realistically rewrite all the hotkey / mouse handler code to pass back return values instead. There's surely a correct way to write this that doesn't involve so many long range exceptions being thrown around routinely, if you have any ideas I think it would be good to fix this.
gfgtdf
Developer
Posts: 1432
Joined: February 10th, 2013, 2:25 pm

Re: Iterate over all variables

Post by gfgtdf »

iceiceice wrote: So I believe this means that any time you click to attack and kill the enemy leader, there is going to be an exception that goes from inside the synced context, back through the mouse handler code, back through place_slice(), up to the handler somewhere in play_controller.
this has been like that even before my pr.
https://github.com/wesnoth/wesnoth/blob ... .cpp#L1154
Scenario with Robots SP scenario (1.11/1.12), allows you to build your units with components, PYR No preperation turn 1.12 mp-mod that allows you to select your units immideately after the game begins.
User avatar
iceiceice
Posts: 1056
Joined: August 23rd, 2013, 2:10 am

Re: Iterate over all variables

Post by iceiceice »

gfgtdf wrote:
iceiceice wrote: So I believe this means that any time you click to attack and kill the enemy leader, there is going to be an exception that goes from inside the synced context, back through the mouse handler code, back through place_slice(), up to the handler somewhere in play_controller.
this has been like that even before my pr.
https://github.com/wesnoth/wesnoth/blob ... .cpp#L1154
Yes, I'm sure it was, I didn't mean to imply otherwise.
gfgtdf
Developer
Posts: 1432
Joined: February 10th, 2013, 2:25 pm

Re: Iterate over all variables

Post by gfgtdf »

ok i tried to refactor the variable_info code:
https://github.com/wesnoth/wesnoth/pull/231
@Sapient
Since you seem to be the one who knows about the previous implementation
do you know if there are any special cases i have to watch out for ?
Scenario with Robots SP scenario (1.11/1.12), allows you to build your units with components, PYR No preperation turn 1.12 mp-mod that allows you to select your units immideately after the game begins.
User avatar
Sapient
Inactive Developer
Posts: 4453
Joined: November 26th, 2005, 7:41 am
Contact:

Re: Iterate over all variables

Post by Sapient »

gfgtdf wrote:ok i tried to refactor the variable_info code:
https://github.com/wesnoth/wesnoth/pull/231
@Sapient
Since you seem to be the one who knows about the previous implementation
do you know if there are any special cases i have to watch out for ?
I don't think I'll be able to review it sorry. I'm not sure what is the motivation behind those changes, but in my humble opinion it seems needlessly complex. I may be biased though.
http://www.wesnoth.org/wiki/User:Sapient... "Looks like your skills saved us again. Uh, well at least, they saved Soarin's apple pie."
gfgtdf
Developer
Posts: 1432
Joined: February 10th, 2013, 2:25 pm

Re: Iterate over all variables

Post by gfgtdf »

after reading http://gna.org/bugs/?22268 i investigated more and it seems like even without 'our extensions' the original lua code uses c++ exceptions internaly if compiled as c++ code.
see http://www.lua.org/source/5.2/ldo.c.html
so i am sure that it can handle c++ execptions correctly.
Scenario with Robots SP scenario (1.11/1.12), allows you to build your units with components, PYR No preperation turn 1.12 mp-mod that allows you to select your units immideately after the game begins.
Post Reply