--- Log opened Wed Oct 19 00:00:41 2016
20161019 00:13:44-!- skeletenchi [enchilado@defocus/yummy/enchilado] has quit [Ping timeout: 250 seconds]
20161019 00:15:28-!- skeletenchi [enchilado@defocus/yummy/enchilado] has joined #wesnoth-dev
20161019 00:22:31-!- mattsc [~mattsc@wesnoth/developer/mattsc] has quit [Quit: So long and thanks for all the fish.]
20161019 00:26:41-!- mattsc [~mattsc@wesnoth/developer/mattsc] has joined #wesnoth-dev
20161019 00:39:25-!- Greg-Boggs [~greg_bogg@173.240.241.83] has joined #wesnoth-dev
20161019 00:43:54-!- Greg-Boggs [~greg_bogg@173.240.241.83] has quit [Ping timeout: 250 seconds]
20161019 00:51:37< iceiceice> tad_carlucci: it's a bad idea to throw exceptions from the lua panic function
20161019 00:52:07< iceiceice> lua is not exception safe
20161019 00:52:20< iceiceice> if you catch the exception your lua state will be trashed
20161019 00:52:33< iceiceice> it is better to just call abort
20161019 00:53:03< iceiceice> and don't cause panics
20161019 00:53:32< iceiceice> it doesn't really make sene to try to recover from panics, it makes more sense to code the program properly so that they don't happen
20161019 00:54:10< iceiceice> tad_carlucci, wesnoth developers have been working on these issues for years...
20161019 00:54:36< iceiceice> we have a whole exception type lua_jailbreak_exception which we hacked into lua in order to try to work around this issue
20161019 00:55:23< iceiceice> please don't commit a panic function which throws exceptions
20161019 00:55:37< tad_carlucci> NP
20161019 00:56:22< iceiceice> thx
20161019 00:57:00< tad_carlucci> celticminstrel will just have to accept a panic calls abort(). The code is easy, but I've been staring at the lua_State and worried that it would be unusable and worried about just that.
20161019 00:57:02< celticminstrel> iceiceice: Then what do you propose instead? Calling abort() is unacceptable behaviour. We need to protect the AI code somehow.
20161019 00:57:43< celticminstrel> If the problem is only that the Lua state would be trashed, then that's no problem if it boots you to titlescreen.
20161019 00:58:19< celticminstrel> Because in that case the Lua state is destroyed anyway.
20161019 00:58:32< iceiceice> i guess you could test that
20161019 00:58:37< iceiceice> i dont know how badly trashed it gets
20161019 00:58:49< iceiceice> presumably you still want to call lua_close(L) on it?
20161019 00:58:52< iceiceice> or do you just leak it?
20161019 00:58:59< celticminstrel> Probably you'd want to close it, yes...
20161019 00:59:12< celticminstrel> Is there a better way to simply protect the AI code from unexpected errors in Lua C API calls?
20161019 00:59:19< tad_carlucci> iceiceice, It will have stack items and garbage to be collected.
20161019 00:59:23< celticminstrel> Something like pcall(), but calling a C function from C.
20161019 00:59:30< celticminstrel> For example.
20161019 00:59:37< iceiceice> celticminstrel, there was a thing called cpcall in lua 5.1
20161019 00:59:45< iceiceice> and they removed it because they said it was easier to just use pcall for that
20161019 00:59:52 * tad_carlucci nods
20161019 00:59:58< celticminstrel> Hmm.
20161019 00:59:59< iceiceice> i have a library called lua-primer
20161019 01:00:04< iceiceice> i made a thing called cpp_pcall
20161019 01:00:07< celticminstrel> So maybe we can actually do this with luaW_pcall after all...
20161019 01:00:08< iceiceice> which is kind of like what you want i think
20161019 01:00:14< iceiceice> and i have some links in the documentation
20161019 01:00:17< celticminstrel> Just need to figure out where to do it.
20161019 01:00:18< iceiceice> into the lua users wiki
20161019 01:00:39< iceiceice> https://cbeck88.github.io/lua-primer/lua_primer/reference/miscellaneous/cpppcall.html
20161019 01:00:54< tad_carlucci> The problem is that the panic today was completely inside Lua. No C++ involved at all.
20161019 01:01:12< celticminstrel> That doesn't seem to align with what I was experiencing though...
20161019 01:01:27< celticminstrel> As I said before, it was crashing when the C++ code called lua_getfield.
20161019 01:02:01< iceiceice> hmmm
20161019 01:02:16< iceiceice> i guess i dont know if you can catch e.g. lua stack discipline errors
20161019 01:02:17< tad_carlucci> So if you expect that to happen check that the table exisst and is a table before you getfield
20161019 01:02:50< iceiceice> yah i think if you use things like getfield illegally you probably just segfault
20161019 01:03:10< iceiceice> if it panics instead i guess that's a bonus
20161019 01:03:15< tad_carlucci> But that's in C++. And it won't stop panic abort()
20161019 01:03:18< celticminstrel> It seemed about equivalent to the error you'd get when trying to access a field from nil.
20161019 01:03:21< celticminstrel> In Lua code.
20161019 01:03:47< celticminstrel> Well, I'll try to do some sanity testing when the AI handler is initialized, first.
20161019 01:03:50< iceiceice> i think the C api doesn't actually raise lua errors
20161019 01:03:57< iceiceice> when a lua error is raised it is supposed to have like a stack trace and such
20161019 01:04:04< iceiceice> and a line number of lua code associated to it
20161019 01:04:09< celticminstrel> If that doesn't work out I'll try to run it in a protected context somehow.
20161019 01:04:20< iceiceice> idk what their debug mechanism would do if the error is caused by lua_getfield
20161019 01:07:11< iceiceice> bbl
20161019 01:07:14-!- iceiceice [~chris@unaffiliated/iceiceice] has quit [Quit: Ex-Chat]
20161019 01:22:27-!- stikonas [~gentoo@wesnoth/translator/stikonas] has quit [Quit: Konversation terminated!]
20161019 01:30:33< gfgtdf> tad_carlucci: why shodul i not do "require("something").somefun()" ? i'd think that the code that calls the scipt was executed with pcall whaever and thut thats error handler is called.
20161019 01:30:51< tad_carlucci> Wrong.
20161019 01:31:09< tad_carlucci> It panics when the '.' part executes.
20161019 01:33:14< tad_carlucci> The point being the panic was because you're "dereferencing the null pointer" (sic) and the message is "index a function"
20161019 01:35:02< tad_carlucci> It's not that you should not do it, per se. It's fine, jsut be aware it can cause lua to panic if the "something" script has a syntax error.
20161019 01:36:56< tad_carlucci> And, this was in an AI .. they are not called from the C++ using pcall. They're executed from on-the-fly scripts. So the assumption that all Lua is in a protected environment is false, too.
20161019 01:38:07< gfgtdf> tad_carlucci: well, we quite often use things like "mylib = wesnoth.require("mylib.lua") ..     ... mylib.sonefunc()" and it never gave a panic before when mylib contained  a syntx error.
20161019 01:38:45< tad_carlucci> Maybe it's just AI then, since it's not called using pcall.
20161019 01:39:35< gfgtdf> tad_carlucci: you know where exactly it is called without pcall ?
20161019 01:39:52< celticminstrel> gfgtdf: src/ai/lua/engine_lua.cpp in several places
20161019 01:40:05< celticminstrel> Also possibly src/ai/lua/core.cpp in lua_ai_handler::handle.
20161019 01:40:15< tad_carlucci> Actuall 1 place, 2 branches, 2 options
20161019 01:40:22< celticminstrel> Though that does use luaW_pcall... but I was still getting a panic before it even got there...
20161019 01:40:29< celticminstrel> tad_carlucci: Uh, there should be at least four places...
20161019 01:40:39< celticminstrel> Maybe one is in a different file though.
20161019 01:40:45< celticminstrel> Goals might be in a different file...
20161019 01:40:51< gfgtdf> someone has a stacktrace of that panic ?
20161019 01:41:41< tad_carlucci> How do you think I started?
20161019 01:43:16< tad_carlucci> If you want to see the program change to correct the panic, I can roll back and find my pastebin link, again.
20161019 01:43:50< tad_carlucci> The c++ call site which took us into Lua has the same problem, but was not involved.
20161019 01:44:23< gfgtdf> tad_carlucci: you posted the stacktrace here today ?
20161019 01:46:47< tad_carlucci> Probably not. And it'll take me a while to recreate it because I don't have a debug build right now. The repo is easy. Start MP Local, The Freelands and set player 1 to the experimental AI.
20161019 01:47:17< tad_carlucci> To cause the crash, put a syntax error in the general recruiting ai lua
20161019 01:47:28-!- gfgtdf_ [~chatzilla@x4e36ac6b.dyn.telefonica.de] has joined #wesnoth-dev
20161019 01:49:00< gfgtdf_> tad_carlucci: i cannot debug here locally
20161019 01:49:20-!- gfgtdf [~chatzilla@x4e3697cd.dyn.telefonica.de] has quit [Ping timeout: 250 seconds]
20161019 01:49:23-!- gfgtdf_ is now known as gfgtdf
20161019 01:49:40< gfgtdf> tad_carlucci: just in the file generic_recruit_engine ?
20161019 01:50:18< tad_carlucci> That I know of. When I fix the Lua to stop that panic another occurs which I've not tracked down.
20161019 01:51:35< gfgtdf> tad_carlucci: i tried that on 1.12.5 and couldnt reproduce (don't have a 1.13 build here)
20161019 01:53:04< tad_carlucci> https://github.com/wesnoth/wesnoth/blob/master/src/ai/lua/engine_lua.cpp#L163-L173
20161019 01:54:26< tad_carlucci> The function which called the Lua which tried to load the general recruit AI with an error and paniced when it could not use it.
20161019 01:54:55< celticminstrel> ^panicked
20161019 01:55:17< tad_carlucci> Well Pan ate it and said, "Ick!"
20161019 01:59:11< celticminstrel> Ha ha.
20161019 01:59:34< celticminstrel> Okay, so if the code fails to load, then no handler is created.
20161019 01:59:41< celticminstrel> So this isn't an issue of the code failing to load.
20161019 02:00:32< tad_carlucci> The code loads and eventually gets to the Lua I changed in that pastebin earlier. That's where the panic occurs.
20161019 02:02:06< celticminstrel> So wesnoth.require is raising an error, rather than returning nil?
20161019 02:03:32< tad_carlucci> Don't know what the Lua data type is. Lua says it's a function and it cannot be indexed.
20161019 02:05:51< celticminstrel> Hmm...
20161019 02:06:48< celticminstrel> Meaning wesnoth.require returned a function?
20161019 02:07:10< tad_carlucci> Trust, but not verified
20161019 02:07:15< celticminstrel> ???
20161019 02:07:29< tad_carlucci> Lua says it, I've not seen it.
20161019 02:23:58< tad_carlucci> At it's heart, wesnoth.require is basically lua_load which makes sense. And there is a protected call forthe LOAD (initialization) but is NOT called for the AI "evaluation" and "execution" functions because they're called by Lua unprotected AFTER wesnoth.require has completed
20161019 02:25:16< vultraz> gfgtdf: iceiceice: crash is gone
20161019 02:25:24< celticminstrel> But... aren't the evaluation and execution functions part of the code loaded for the lua_ai_handler?
20161019 02:25:33< celticminstrel> Which uses luaW_pcall to call the function?
20161019 02:26:18< mattsc> gfgtdf, tad_carlucci: confirmed, the panic does not happen in 1.12.6.  It also does not happen in 1.13.5.  But it does in current master.
20161019 02:26:19< tad_carlucci> Can't use luaW_pcall for those. Must use lua_loadstring
20161019 02:26:35< celticminstrel> This is after it's loaded...
20161019 02:26:43< tad_carlucci> And it does is I roll back prior to Lua 5.3.3 going in.
20161019 02:27:01< mattsc> tad_carlucci: haven’t done that yet ...
20161019 02:27:06< tad_carlucci> So, I guess the thing to do is bisect
20161019 02:27:12< gfgtdf> mattsc: can you try to generate a a stacktrace for the panic 
20161019 02:27:14< gfgtdf> ?
20161019 02:27:15< tad_carlucci> I did roll back .. first thing.
20161019 02:27:31< gfgtdf> mattsc: at curret master i mean
20161019 02:27:55< gfgtdf> mattsc: a c++ stacktrace i meant.
20161019 02:33:04< mattsc> tad_carlucci: yeah, just confirmed that too, it was present before the Lua 5.3.3 merge
20161019 02:36:02< mattsc> gfgtdf: recompiling for current master, will get you the stacktrace afterward
20161019 02:36:32< mattsc> I think ...
20161019 02:38:30< gfgtdf> mattsc: thx
20161019 02:41:05< mattsc> gfgtdf: hmm, does this contain what you need?  http://pastebin.com/EWtfmwH8
20161019 02:41:33< gfgtdf> mattsc: yes exactly what i asked for thx
20161019 02:41:50< mattsc> okay, that’s what I thought — and then I confused myself  :P
20161019 02:43:18< gfgtdf> tad_carlucci: the stacktrace sys teh erro comes from https://github.com/wesnoth/wesnoth/blob/master/src/ai/lua/core.cpp#L1147
20161019 02:44:31< celticminstrel> That's where I was getting the crash, too.
20161019 02:44:48< gfgtdf> celticminstrel: you know why we dont use rawget there ?
20161019 02:45:23< celticminstrel> Probably just because that requires pushing the key first, so it's more verbose.
20161019 02:45:49< celticminstrel> Besides, these are internal tables we're working with, so there are no metatables involved.
20161019 02:46:00< celticminstrel> (Well, unless someone adds one via debug_ai()...)
20161019 02:55:04-!- iceiceice [~chris@unaffiliated/iceiceice] has joined #wesnoth-dev
20161019 02:56:35< gfgtdf> tad_carlucci: ok i dont really understand that code, but my approach would be to add some assert(lua_istable(L, ...)), assert(lua_isfunction(L, ...)) ...inside lua_ai_action_handler::handle to check that the assumnptions there about the lua stack are true.
20161019 02:58:44< gfgtdf> really have to go now
20161019 02:58:44-!- gfgtdf [~chatzilla@x4e36ac6b.dyn.telefonica.de] has quit [Quit: ChatZilla 0.9.92 [Firefox 49.0.1/20160922113459]]
20161019 02:59:21< tad_carlucci> I stepped the code in gdb and got to that point and realized the failure was previously. Working back gets to that change to the lua I put up on pastebin earlier.
20161019 03:00:12< tad_carlucci> But (a) if workds on 1.13.5 (b) it does not work prior to the Lua 5.3.3 merge so (c) something changed in between. My vote is wesnoth.require someplace.
20161019 03:01:03< tad_carlucci> I'm setting up a bisect because I want to know what and when it changed so we can make a rational decision on how to fix it.
20161019 03:03:48-!- bumbadadabum [~bumbadada@wesnoth/developer/bumbadadabum] has quit [Ping timeout: 245 seconds]
20161019 03:03:50< irker634> wesnoth: Celtic Minstrel wesnoth:master 200b57e72190 / src/scripting/lua_kernel_base.cpp: Improve Lua console tab completion https://github.com/wesnoth/wesnoth/commit/200b57e72190f6aadd668c8ea97af8692335b70c
20161019 03:04:07 * celticminstrel was looking at the AI stuff, saw something, and got distracted. >_>
20161019 03:04:23-!- JyrkiVesterinen [~JyrkiVest@87-100-190-14.bb.dnainternet.fi] has joined #wesnoth-dev
20161019 03:08:06< tad_carlucci> celticminstrel, NP I think everyone is distracted because it's a hard bug to nail down. The failure site and the crash site are wide apart.
20161019 03:16:22< tad_carlucci> celticminstrel, Want to take a bet that we don't see the 'the build has passed' message on that commit? We've missed it on the last three ...
20161019 03:27:32< iceiceice> +1 to gfgtdf's approach
20161019 03:27:51< iceiceice> also one thing you can do,
20161019 03:28:01< iceiceice> since 5.2 (?) lua_getfield actually returns an integer
20161019 03:28:03< vultraz> iceiceice: confirmed crash fix in fonts
20161019 03:28:10< iceiceice> which indicates the type of what was gotten
20161019 03:28:21< iceiceice> or LUA_TNONE or LUA_TNIL if it was nothing
20161019 03:28:35< iceiceice> (actually NONE is to indicate past the end of lua stack)
20161019 03:29:00< iceiceice> so you could do things like `assert(LUA_TFUNCTION == lua_getfield(L, ... , ...), "expected a function");`
20161019 03:30:45< tad_carlucci> iceiceice, The question is: what changed since 1.13.5 was tagged? It does not cause a crash there. And it crashes if you roll back the lua 5.3.3 commit. So it's not the upgrade but it's somewhere in between.
20161019 03:31:16< tad_carlucci> I have a feeling if I can find the change, the fix will become apparent.
20161019 03:31:54< iceiceice> idk, you could try git bisect?
20161019 03:32:02< iceiceice> i didnt realize there was a change
20161019 03:32:06< tad_carlucci> runing that now.
20161019 03:32:11< iceiceice> gl
20161019 03:32:20 * tad_carlucci chuckles.
20161019 03:32:22< tad_carlucci> I know.
20161019 03:36:18 * Aginor stirs
20161019 03:36:45< vultraz> :o
20161019 03:41:42< tad_carlucci> Interesting idea: https://github.com/iboB/mtime_cache
20161019 03:48:11< iceiceice> tad_carlucci, so the mtime only really matters for make files iirc
20161019 03:48:20< iceiceice> another way to do this is install ccache on travis
20161019 03:48:27< iceiceice> and make travis ccache the ccache directory
20161019 03:48:49 * tad_carlucci shrugs
20161019 03:49:14< iceiceice> idk i think travis-ci has become too complicated
20161019 03:49:21< tad_carlucci> Its something to think about if anyone decides to do some work on the travis builds.
20161019 03:49:33< tad_carlucci> I sort of agree on the complexity.
20161019 03:51:06-!- vultraz [~chatzilla@wesnoth/developer/vultraz] has quit [Ping timeout: 250 seconds]
20161019 03:51:58-!- ancestral [~ancestral@75-168-189-115.mpls.qwest.net] has joined #wesnoth-dev
20161019 03:52:11< JyrkiVesterinen> iceiceice: shadowm attempted to enable ccache in the weekend.
20161019 03:52:24< JyrkiVesterinen> He quickly backed it out because it caused builds to fail.
20161019 03:52:42< iceiceice> hmm too bad
20161019 03:56:24-!- vultraz [~chatzilla@wesnoth/developer/vultraz] has joined #wesnoth-dev
20161019 03:57:08-!- JyrkiVesterinen [~JyrkiVest@87-100-190-14.bb.dnainternet.fi] has quit [Quit: Going to work]
20161019 03:59:37< irker634> wesnoth: Wedge009 wesnoth:master 45b6914d16ee / src/font/sdl_ttf.cpp: No need to re-define ellipsis variable. https://github.com/wesnoth/wesnoth/commit/45b6914d16ee13315ad272d77224e333764d6fd7
20161019 04:01:04-!- vultraz [~chatzilla@wesnoth/developer/vultraz] has quit [Ping timeout: 250 seconds]
20161019 04:03:04-!- Bonobo [~Bonobo@2001:44b8:254:3200:4844:6f8a:a55a:e173] has joined #wesnoth-dev
20161019 04:04:08< celticminstrel> Aginor: I don't suppose you'd have any idea why the flg dialog hangs when joining an MP game?
20161019 04:04:59< celticminstrel> It hangs in the dialog::show loop, and the screen just remains black.
20161019 04:05:06< celticminstrel> Well, loading screen I guess.
20161019 04:05:25< Aginor> celticminstrel: no, I might have time to look tonight
20161019 04:05:34< celticminstrel> Not sure if it's actually still in the loading screen or if that's just a remnant that hasn't been undrawn yet...
20161019 04:05:35< Aginor> but is events being pumped?
20161019 04:05:39< celticminstrel> Yeah.
20161019 04:05:45< Aginor> s/is/are/
20161019 04:06:08< Aginor> then it's probably not hanging, it's "just" doing the wrong thing
20161019 04:06:25< celticminstrel> Maybe. I'm not sure.
20161019 04:06:54< celticminstrel> It's being called from another dialog's post_build (and same effect if moved to pre_show), but no idea if that's really relevant...
20161019 04:09:07-!- vultraz [~chatzilla@wesnoth/developer/vultraz] has joined #wesnoth-dev
20161019 04:09:49< Aginor> I have no idea how the interaction there goes
20161019 04:09:58< Aginor> are dialogs allowed to spawn new dialogs?
20161019 04:10:11< celticminstrel> Generally speaking, yes.
20161019 04:10:24< celticminstrel> Lots of dialogs spawn dialogs.
20161019 04:10:26< Aginor> or is the problem that the code expects control to returned to it when a new dialog is spawned?
20161019 04:10:41< celticminstrel> I'm not sure.
20161019 04:10:53< Aginor> you should easily enough figure out what's going on there by attaching a debugger though
20161019 04:11:04< celticminstrel> It might be unusual for a dialog to spawn another dialog before it itself is shown.
20161019 04:11:21< celticminstrel> All I got from attaching a debugger was that it's definitely in the dialog::show loop.
20161019 04:11:34< celticminstrel> Maybe I didn't look as closely as I could've, but...
20161019 04:11:53< Aginor> you probably need to step a bit to figure it out
20161019 04:12:19< celticminstrel> Probably.
20161019 04:12:35< celticminstrel> Stepping through events::pump is not fun, though.
20161019 04:12:55< celticminstrel> And there's not much else to step through, I think...
20161019 04:13:05< vultraz> celticminstrel: can confirm again moving it to preshow doesnt work
20161019 04:14:07< celticminstrel> I thought of moving it before even calling show().
20161019 04:14:15< Aginor> do you have to step through that one? you should be able to just move on instead
20161019 04:14:19< celticminstrel> But I wasn't sure what exactly would need to be moved for that to work.
20161019 04:14:31< celticminstrel> Aginor: I don't know whether I have to step through events::pump.
20161019 04:14:40< Aginor> try not to at first
20161019 04:14:46< celticminstrel> Makes sense. :P
20161019 04:14:51< Aginor> :D
20161019 04:16:42 * Aginor is full of sagelike advice
20161019 04:18:37-!- Greg-Boggs [~greg_bogg@173.240.241.83] has joined #wesnoth-dev
20161019 04:20:17< vultraz> try or try not, there is no do :P
20161019 04:20:45< celticminstrel> Somehow that Yoda-ism seems a little... distorted. :P
20161019 04:23:04-!- Greg-Boggs [~greg_bogg@173.240.241.83] has quit [Ping timeout: 260 seconds]
20161019 04:35:00< irker634> wesnoth: Wedge009 wesnoth:master e80347dae0ab / src/fake_unit_manager.cpp: Don't try to remove anything if fake_units queue is already empty (bugs #24022/2 https://github.com/wesnoth/wesnoth/commit/e80347dae0ab7dc1a9b5f9017b27395bba7bd019
20161019 04:35:02< irker634> wesnoth: Wedge009 wesnoth:master 6701854a2cff / changelog: Update change log. https://github.com/wesnoth/wesnoth/commit/6701854a2cff110f8ddaf373a3454efc7f24a9f9
20161019 04:40:31-!- travis-ci [~travis-ci@ec2-54-197-70-163.compute-1.amazonaws.com] has joined #wesnoth-dev
20161019 04:40:32< travis-ci> wesnoth/wesnoth#11659 (master - 45b6914 : Wedge009): The build has errored.
20161019 04:40:33< travis-ci> Build details : https://travis-ci.org/wesnoth/wesnoth/builds/168817092
20161019 04:40:33-!- travis-ci [~travis-ci@ec2-54-197-70-163.compute-1.amazonaws.com] has left #wesnoth-dev []
20161019 04:45:28< celticminstrel> vultraz, zookeeper: Is PR 817 serviceable yet?
20161019 04:46:07< vultraz> serviceable, but I'm waiting on him to finish the gates.
20161019 04:46:11< vultraz> do not merge, please.
20161019 04:46:21< celticminstrel> Gates in the same PR?
20161019 04:46:30< vultraz> yes
20161019 04:46:43< celticminstrel> Since that PR is kinda messed up, might it not be better to merge it and start a new one for gates?
20161019 04:46:57< vultraz> gates are already in there
20161019 04:47:02< vultraz> just waiting on revisions
20161019 04:47:03< celticminstrel> Ah, okay then.
20161019 04:47:05< vultraz> so leave it, please.
20161019 04:47:17< celticminstrel> Are you okay to merge it when it's ready, or do you want me to?
20161019 04:47:27< vultraz> ill deal 
20161019 04:47:30< celticminstrel> 'kay
20161019 04:48:26< celticminstrel> Okay, so does anyone have an opinion on PR 833?
20161019 04:48:47< vultraz> travis passes, just merge
20161019 04:48:53< celticminstrel> I'm kinda glad that XCode and MSVC don't need to support out-of-tree builds.
20161019 04:49:07< vultraz> matthiaskrgr_: will confirm if it works
20161019 04:49:11< celticminstrel> Alright. If they still don't work, someone can fix it later.
20161019 04:49:12< vultraz> (what even is an out-of-tree build)
20161019 04:49:33< tad_carlucci> I can't confirm it works as intended, only that it doesn't break for me. I do in-tree builds
20161019 04:49:48< celticminstrel> I know what it is but it's somewhat hard for me to describe it...
20161019 04:49:53< vultraz> I  wish you would finish 798
20161019 04:49:58< vultraz> (the menu refactor stuff)
20161019 04:50:01< celticminstrel> Oh.
20161019 04:50:11< celticminstrel> I don't remember what was wrong with that...
20161019 04:50:18< vultraz> some items not showing
20161019 04:50:24< celticminstrel> Something about editor menus working but in-game ones not, I think?
20161019 04:50:33< celticminstrel> Were there items not showing?
20161019 04:50:39< vultraz> editor
20161019 04:51:11< celticminstrel> If matthiaskrgr_ is going to confirm that 833 works, I'll wait for that before merging.
20161019 04:51:17< celticminstrel> Has he said he'll confirm?
20161019 04:51:34-!- ancestral [~ancestral@75-168-189-115.mpls.qwest.net] has quit [Quit: i go nstuf kthxbai]
20161019 04:55:54< tad_carlucci> Not that I've seen.
20161019 04:56:59< vultraz> just mergeeee
20161019 04:57:21< vultraz> celticminstrel: will you finish 798?
20161019 04:57:47< celticminstrel> Probably.
20161019 04:58:03< celticminstrel> If I can get sufficiently un-distracted.
20161019 05:04:42< tad_carlucci> That's easy. See that little "X" in the top-right corner?  Click it.
20161019 05:13:02-!- JyrkiVesterinen [~JyrkiVest@nblzone-242-23.nblnetworks.fi] has joined #wesnoth-dev
20161019 05:21:20< vultraz> i wonder if i should move the faction select stuff outside join game
20161019 05:30:54< vultraz> JyrkiVesterinen: what's next on your todo list?
20161019 05:31:12< JyrkiVesterinen> The script to simulate lobby activity.
20161019 05:32:16< vultraz> ah
20161019 05:32:34< vultraz> im not sure that's really needed now that we can connect to the 1.12 server and see standard load
20161019 05:33:58< JyrkiVesterinen> Well, I'd like to finish the tasks related to the lobby tests before moving on to other stuff.
20161019 05:34:12< vultraz> ok
20161019 05:34:15< JyrkiVesterinen> I like to process my tasks in FIFO order if possible.
20161019 05:34:24< vultraz> first in first out?
20161019 05:34:40< JyrkiVesterinen> Exactly.
20161019 05:38:02< vultraz> I'm thinking of tackling the tooltips issue i pointed out to you
20161019 05:38:45< JyrkiVesterinen> What tooltip issue?
20161019 05:39:25< vultraz> https://gna.org/bugs/index.php?22176
20161019 05:40:36< JyrkiVesterinen> Oh, right. I misunderstood your earlier message and interpreted that you planned to assign another bug to me.
20161019 05:40:53< JyrkiVesterinen> Feel free to fix the bug yourself.
20161019 05:41:05< vultraz> I'm still pondering how to do so
20161019 05:41:11< vultraz> you mentioned RAII?
20161019 05:42:05< vultraz> I was thinking something like
20161019 05:42:28< JyrkiVesterinen> Yes. In short, the game could have a data structure about the dialogs which are being shown. For example, a handle for each dialog.
20161019 05:42:43< vultraz> static std::vector<dialog_open_helper> where dialog_open_helper is a struct that pushes a new member to the vector in its ctor and removes itself in its dtor.
20161019 05:42:55< vultraz> then we make is_in_dialog check !vec.empty
20161019 05:43:25< JyrkiVesterinen> Yes, that would work.
20161019 05:48:16< vultraz> celticminstrel: confirmed your cvideo thing for prepreshow dialogs works
20161019 05:49:56< vultraz> hm..
20161019 05:50:35< JyrkiVesterinen> Hmm... looking at my own commit history ( https://github.com/wesnoth/wesnoth/commits/master?after=ZwGFSiz%2FEQ%2BN2vNzo0VO%2FH8kqfkrMzQ%3D&author=jyrkive ), I see that my progress with MP tests has been extremely slow.
20161019 05:50:54< JyrkiVesterinen> I made the first MP test fix commit in September 3, and I'm still not done in that area.
20161019 05:51:20< JyrkiVesterinen> Sigh. I have kept being distracted with higher-priority stuff such as build failures.
20161019 05:52:04< vultraz> hm... actually, wouldn't the dialogs have to be in charge for removing their helper struct entries from the vector
20161019 05:52:08< vultraz> i think so..
20161019 05:52:25< vultraz> i can't make it remove itself in the dtor since there's nothing to actually destroy it..
20161019 05:52:42< JyrkiVesterinen> Isn't post_show() called when the dialog disappears?
20161019 05:52:50< vultraz> yes
20161019 05:53:00< vultraz> I guess I can wire this into the dialog loop
20161019 05:53:01< JyrkiVesterinen> It would be the right time to remove the helper struct.
20161019 05:54:00-!- Kwandulin [~Miranda@p5DDD2B8F.dip0.t-ipconnect.de] has joined #wesnoth-dev
20161019 05:54:19< JyrkiVesterinen> They are used to determine visibility of other items (map labels, chat backlog lines), so they themselves should also be tied to *visibility* and not *existence* of dialogs.
20161019 05:55:04< vultraz> ah
20161019 05:55:17< vultraz> ok, so add/remove directly before/after show
20161019 05:56:16< vultraz> JyrkiVesterinen: how's this look? http://pastebin.com/s7Jsn5id I'm not sure if the static vector needs to be before or after the struct... 
20161019 05:57:11< JyrkiVesterinen> What do you use helper_pos_ for?
20161019 05:57:43< vultraz> I was thinking something like..
20161019 05:57:44< JyrkiVesterinen> I don't recall the C++ rule about if the vector needs to be after the struct or if either order would work.
20161019 05:57:57< JyrkiVesterinen> I'd place the vector after the struct anyway (like you already do).
20161019 05:58:30< vultraz> vec.emplace()... dialog_stack_helper& temp = vec.back()... dialog.show()... vec.erase(temp.helper_pos_);
20161019 05:59:07< vultraz> now that i write that down it seems messy :/
20161019 05:59:27< JyrkiVesterinen> Is it possible to close dialogs in arbitrary order?
20161019 05:59:35< vultraz> no
20161019 06:00:11< JyrkiVesterinen> In that case you can just call vec.pop_back() to remove the latest struct.
20161019 06:00:29< vultraz> hmm
20161019 06:00:36< vultraz> yes, good point
20161019 06:00:41< vultraz> now that I think about it
20161019 06:01:14< JyrkiVesterinen> Mind you, it might still be useful to have some identification for the structs in case some kind of bug causes dialogs to be removed in a wrong order, or not removed at all.
20161019 06:01:28< JyrkiVesterinen> It's up to you.
20161019 06:01:30-!- ancestral [~ancestral@75-168-189-115.mpls.qwest.net] has joined #wesnoth-dev
20161019 06:02:04< vultraz> what about something like 'dialog_stack_helper* temp  = new dialog_stack_helper()... dialog.show() .. delete temp' and the struct ctor does vec.push_back(this) and its dtor vec.pop_back()? 
20161019 06:02:36< JyrkiVesterinen> I prefer direct push_back() and pop_back().
20161019 06:02:47< JyrkiVesterinen> new and delete are code smells in modern C++.
20161019 06:03:17< vultraz> so essentially the struct would have nothing?
20161019 06:03:21< vultraz> in it, that is
20161019 06:03:54< JyrkiVesterinen> I'm not sure if empty structs are possible in C++.
20161019 06:04:09< JyrkiVesterinen> If not, you can add a dummy integer or something to it.
20161019 06:04:12< vultraz> in that case, why not just have the vector be a stack of tdialog* pointers
20161019 06:04:38< vultraz> and in the dialog loop just have vec.push_back(this)... show.. vec.pop_back
20161019 06:04:49< JyrkiVesterinen> Yes, it would work, and also potentially serve as a debugging aid if something goes wrong.
20161019 06:05:24< vultraz> since that seems the simplest solution I'll do that
20161019 06:05:35< JyrkiVesterinen> Sounds good. :)
20161019 06:06:51-!- Greg-Boggs [~greg_bogg@173.240.241.83] has joined #wesnoth-dev
20161019 06:08:03< vultraz> eh...
20161019 06:08:05< vultraz> hm
20161019 06:08:10< vultraz> might've hit a snag..
20161019 06:08:22< vultraz> im not sure tpopup (tooltips) goes through tdialog::show
20161019 06:08:30< vultraz> it seems to implement its own show method..
20161019 06:09:21< JyrkiVesterinen> Hmm, quoting your own message that I posted to the bug tracker:
20161019 06:09:23< JyrkiVesterinen> [06:47:10] <vultraz> JyrkiVesterinen: https://gna.org/bugs/index.php?22176 AFAIK this is still an issue. Aginor said it's because tooltips are displayed in popup 'windows' and as such set the is_in_dialog flag. Need some way to not set is_in_dialog for tooltips and for dropdown menus, but I can't figure out a good way. 
20161019 06:09:23< vultraz> hmm
20161019 06:09:43< JyrkiVesterinen> How do the tooltips set is_in_dialog if they don't go through tdialog::show()?
20161019 06:10:14< vultraz> is_in_dialog was just checking if there were dispatchers..
20161019 06:10:32< vultraz> but looking at the code again, I think I might have gotten the issue backwards ..
20161019 06:11:06< vultraz> because in display.cpp we have this line
20161019 06:11:08< vultraz> 	if (!gui::in_dialog()) {
20161019 06:11:10< vultraz> 		labels().recalculate_labels();
20161019 06:11:11< vultraz> 	}
20161019 06:11:20-!- Greg-Boggs [~greg_bogg@173.240.241.83] has quit [Ping timeout: 260 seconds]
20161019 06:11:47< vultraz> so i think maybe the problem is actually that tooltips were NOT setting is_in_dialog
20161019 06:11:56< vultraz> so I think..
20161019 06:12:12< vultraz> ok, so twindow has 3 show* functions
20161019 06:12:20< vultraz> maybe i'll move this to that
20161019 06:12:36< vultraz> ie, instead if push/pop in tdialog::show, we deal with it in twindow::show*
20161019 06:37:07< iceiceice> JyrkiVesterinen, empty structs are okay in C++
20161019 06:37:14< iceiceice> ok to define, ok to instantiate
20161019 06:37:28< iceiceice> the main wierdness is, sizeof(T) will still be 1 and not 0
20161019 06:37:53< iceiceice> but they generally get optimized out so it doesn't really matter
20161019 06:37:56< JyrkiVesterinen> I figured something like that would be necessary.
20161019 06:38:03< vultraz> blah my fix doesn't work! :(
20161019 06:38:19< JyrkiVesterinen> Otherwise stuff like putting empty structs in a vector would be problematic.
20161019 06:38:30< iceiceice> yea
20161019 06:39:05< vultraz> dammiitt
20161019 06:39:08< vultraz> why doesn't this work :|
20161019 06:39:44< vultraz> ...hm
20161019 06:39:48< vultraz> let me see here..
20161019 06:39:50< vultraz> 	return is_in_dialog || gui2::is_in_dialog();
20161019 06:39:58< vultraz> so it checks gui1 and gui2..
20161019 06:40:39< vultraz> what would false OR true be...
20161019 06:41:13< vultraz> false AND true would be... false
20161019 06:41:22< vultraz> but what i false OR true.
20161019 06:41:30< vultraz> is*
20161019 06:42:23< vultraz> I would say... true?
20161019 06:42:51< vultraz> JyrkiVesterinen?
20161019 06:43:06< JyrkiVesterinen> Yeah, obviously. false || true == true.
20161019 06:43:17< JyrkiVesterinen> I'm surprised that you even need to ask. :/
20161019 06:43:26< vultraz> just checking
20161019 06:43:50< vultraz> to make sure im not making a stupid mistake
20161019 06:43:53< vultraz> :P
20161019 06:44:41< vultraz> ok so the logic is correct
20161019 06:45:32< vultraz> (for the record, im not stupid, i just make really dumb mistakes sometimes so im just making sure i don't keep working on this and come back 2 hours later to find out 'wait, i'm an idiot, the logic is off :| )
20161019 06:48:00< vultraz> ..huh
20161019 06:49:00< vultraz> this is interesting
20161019 06:49:04< vultraz> the vector is always empty..
20161019 06:49:28< vultraz> can you not add members to a static vector?
20161019 06:49:41< vultraz> s/vector/container
20161019 06:49:48< JyrkiVesterinen> You can add members to it.
20161019 06:50:04< vultraz> so why is this always empty...
20161019 06:50:06< vultraz> hmmmmmmmmm
20161019 06:50:19< JyrkiVesterinen> And if you couldn't, the code wouldn't even compile. C++ performs checks in compile time when possible.
20161019 06:50:40< vultraz> well this is odd, then
20161019 06:51:10< JyrkiVesterinen> The only reasons why the vector can be empty is that the code that adds an item to it isn't called, or the item is removed before you check the item count.
20161019 06:51:52< vultraz> neither seem possible...
20161019 07:07:05< vultraz> actually, what's odd is that i was testing this from within a dialog...
20161019 07:07:09< vultraz> which was still open
20161019 07:07:19< vultraz> so the vector should not be empty :/
20161019 07:14:43< celticminstrel> vultraz: tpopup goes through tpopup::show. :P
20161019 07:14:55< vultraz> yes
20161019 07:15:02< vultraz> but im handling this in twindow::show*
20161019 07:15:10< celticminstrel> tpopup and tdialog are sort of "sibling" classes, though there's no actual inheritance involved.
20161019 07:15:26< celticminstrel> Are tooltips really tpopup though? Wouldn't they be ttip?
20161019 07:15:27< vultraz> i assume you mean they bot handle different types of dialogs
20161019 07:15:48< celticminstrel> tpopup does not call twindow::show. It calls twindow::show_non_modal IIRC
20161019 07:16:00< vultraz> yes
20161019 07:16:02< celticminstrel> And ttip should call twindow::show_tooltip or something.
20161019 07:16:19< vultraz> ttip is a tpopup inheritor...
20161019 07:16:21< vultraz> hm
20161019 07:16:23< vultraz> interesting..
20161019 07:16:40< celticminstrel> Ah, ttip inherits from tpopup? I see. I hadn't actually looked at ttip, only tpopup.
20161019 07:16:43< vultraz> so we could make popup dialogs by inheriting from tpopup instead of tdialog
20161019 07:16:51< celticminstrel> Well yes, that's the idae.
20161019 07:16:53< celticminstrel> ^idea
20161019 07:16:56< vultraz> interesting
20161019 07:17:02< celticminstrel> The floating textbox is going to inherit from tpopup.
20161019 07:17:11< vultraz> instead of adding new tooltip definitions,
20161019 07:17:16< celticminstrel> However, it's on hold until the main game UI is compatible with GUI2.
20161019 07:18:14< celticminstrel> (Might also need some modifications to tpopup to support the text field.)
20161019 07:18:34< vultraz> i assume dropdown inherits from tdialog is so it pauses the gam
20161019 07:18:35< vultraz> e
20161019 07:18:40< vultraz> s/is//
20161019 07:18:53< celticminstrel> Yes.
20161019 07:19:32< celticminstrel> It could theoretically be changed, but in that case I think I'd want a pause option.
20161019 07:20:00< celticminstrel> (It probably wouldn't need a new dialog, you could just use show_transient_message.)
20161019 07:21:46< vultraz> sigh
20161019 07:21:55< vultraz> vector is STILL empty :|
20161019 07:24:48-!- celticminstrel [~celmin@unaffiliated/celticminstrel] has quit [Quit: And lo! The computer falls into a deep sleep, to awake again some other day!]
20161019 07:34:17< vultraz> or..
20161019 07:34:19< vultraz> is it?
20161019 07:34:22< vultraz> i don't understand this..
20161019 07:34:32< vultraz> if i measure the size immediately after a push_back...
20161019 07:34:35< vultraz> it's 1
20161019 07:34:53< vultraz> displaying a tooltip...still 1...
20161019 07:34:56< vultraz> what's going on :/
20161019 07:36:30-!- boucman_work [~boucman@fw-alt.idf.smile.fr] has joined #wesnoth-dev
20161019 07:36:44< vultraz> this doesn't make any sense
20161019 07:54:11< vultraz> even if i never pop_back, it doesn't work
20161019 07:55:10-!- Greg-Boggs [~greg_bogg@173.240.241.83] has joined #wesnoth-dev
20161019 07:59:36-!- Greg-Boggs [~greg_bogg@173.240.241.83] has quit [Ping timeout: 260 seconds]
20161019 08:11:19< irker634> wesnoth: Nils Kneuper wesnoth:master a094a5fe61f5 / data/core/about.cfg: updated list of Finnish translators https://github.com/wesnoth/wesnoth/commit/a094a5fe61f5ae6f5ac8779365249a9906965ce8
20161019 08:11:22< irker634> wesnoth: Nils Kneuper wesnoth:1.12 311a021b777c / data/core/about.cfg: updated list of Finnish translators https://github.com/wesnoth/wesnoth/commit/311a021b777c44af4c226dc80932a0aa437bf93d
20161019 08:18:02-!- boucman_work [~boucman@fw-alt.idf.smile.fr] has quit [Ping timeout: 250 seconds]
20161019 08:25:14-!- zookeeper [~lmsnie@wesnoth/developer/zookeeper] has joined #wesnoth-dev
20161019 08:39:15-!- tad_carlucci [~lundberg@173.217.65.103] has quit [Quit: Off to resolve a merge conflict between the wife and husband branches of my real life.]
20161019 08:42:23-!- ancestral [~ancestral@75-168-189-115.mpls.qwest.net] has quit [Quit: i go nstuf kthxbai]
20161019 08:51:30-!- ancestral [~ancestral@75-168-189-115.mpls.qwest.net] has joined #wesnoth-dev
20161019 08:52:24-!- iceiceice [~chris@unaffiliated/iceiceice] has quit [Ping timeout: 252 seconds]
20161019 08:57:52-!- boucman_work [~boucman@gre92-5-82-237-199-7.fbx.proxad.net] has joined #wesnoth-dev
20161019 09:09:21-!- JyrkiVesterinen [~JyrkiVest@nblzone-242-23.nblnetworks.fi] has quit [Quit: .]
20161019 09:22:25-!- horrowind [~Icedove@2a02:810a:8380:10a8:21b:fcff:fee3:c3ff] has joined #wesnoth-dev
20161019 09:27:49-!- JyrkiVesterinen [~JyrkiVest@nblzone-242-23.nblnetworks.fi] has joined #wesnoth-dev
20161019 09:32:02-!- boucman_work [~boucman@gre92-5-82-237-199-7.fbx.proxad.net] has quit [Ping timeout: 244 seconds]
20161019 09:39:46-!- bumbadadabum [~bumbadada@wesnoth/developer/bumbadadabum] has joined #wesnoth-dev
20161019 09:43:17-!- ancestral [~ancestral@75-168-189-115.mpls.qwest.net] has quit [Quit: i go nstuf kthxbai]
20161019 09:43:25-!- Greg-Boggs [~greg_bogg@173.240.241.83] has joined #wesnoth-dev
20161019 09:47:52-!- Greg-Boggs [~greg_bogg@173.240.241.83] has quit [Ping timeout: 260 seconds]
20161019 09:52:41-!- Kwandulin [~Miranda@p5DDD2B8F.dip0.t-ipconnect.de] has quit [Read error: Connection reset by peer]
20161019 09:58:27-!- Kwandulin [~Miranda@p5DDD2B8F.dip0.t-ipconnect.de] has joined #wesnoth-dev
20161019 09:58:34-!- boucman_work [~boucman@fw-alt.idf.smile.fr] has joined #wesnoth-dev
20161019 10:24:40< vultraz> JyrkiVesterinen: https://github.com/Vultraz/wesnoth/commit/b68a4cee48a2a16db61001bc4cd4e4a9c752900e am i doing something obviously wrong :/ the vector isn't empty when it get added to, but when its queried it returns empty! 
20161019 10:31:23< JyrkiVesterinen> I see one thing that looks wrong to me: the fact that gui2::open_window_stack is defined in a header file.
20161019 10:31:30< JyrkiVesterinen> I'm not sure if it should even compile.
20161019 10:31:45< JyrkiVesterinen> Try declaring the vector as extern in the header...
20161019 10:31:49< JyrkiVesterinen> extern static std::vector<twindow*> open_window_stack;
20161019 10:32:04< JyrkiVesterinen> ...and moving the definition to handler.cpp.
20161019 10:32:26< JyrkiVesterinen> Everything else looks correct.
20161019 10:45:49< vultraz> hm
20161019 10:45:54< vultraz> extern static doesn't build..
20161019 10:46:41< vultraz> "conflicting specifiers in declaration"
20161019 10:46:43-!- bumbadadabum [~bumbadada@wesnoth/developer/bumbadadabum] has quit [Ping timeout: 245 seconds]
20161019 10:51:02< JyrkiVesterinen> You can actually drop the "static" qualifier. It's a freestanding (i.e. global) variable anyway.
20161019 10:51:18-!- Bonobo [~Bonobo@2001:44b8:254:3200:4844:6f8a:a55a:e173] has quit [Quit: Leaving]
20161019 10:51:22< JyrkiVesterinen> (My memory about this is a bit rusty, as I rarely use global variables.)
20161019 11:00:39< vultraz> ok at least now the vector isn't registering empty
20161019 11:00:44< vultraz> but the bug is still present
20161019 11:01:11< vultraz> for the record, what was the issue with it being in the header?
20161019 11:02:01< JyrkiVesterinen> Violation of the One Definition Rule. A variable can only be defined in one so-called compilation unit.
20161019 11:02:28< JyrkiVesterinen> Each .cpp file that #includes handler.h ends up having its own definition of the vector.
20161019 11:02:53< vultraz> huh
20161019 11:03:03< vultraz> I see
20161019 11:03:11< JyrkiVesterinen> Which the linker should notice and give an error, which is why I don't know why you even got the code to build.
20161019 11:03:25< vultraz> a lot of things build for me and then error for other people :P
20161019 11:05:40< vultraz> hmmmmm
20161019 11:07:14< vultraz> well seems ive actuallybeen looking in the wrong place for this bug..
20161019 11:11:08< vultraz> it's not that recalc label call..
20161019 11:11:22 * vultraz pings Aginor
20161019 11:11:33-!- irker634 [~irker@uruz.ai0867.net] has quit [Quit: transmission timeout]
20161019 11:22:19< vultraz> AH
20161019 11:22:21< vultraz> I think i got it :D
20161019 11:27:36-!- irker416 [~irker@uruz.ai0867.net] has joined #wesnoth-dev
20161019 11:27:37< irker416> wesnoth: Charles Dang wesnoth:master c3eb503c6ec4 / src/gui/ (5 files in 3 dirs): GUI2: fixed floating labels drawing over dialogs when tooltips are displayed (bu https://github.com/wesnoth/wesnoth/commit/c3eb503c6ec4b4b8aa96d41fda012590fb8260b1
20161019 11:27:54< vultraz> Aginor: ^ right fix, I hope
20161019 11:27:59< vultraz> JyrkiVesterinen: thanks for the help
20161019 11:28:13< JyrkiVesterinen> You're welcome.:)
20161019 11:28:13< vultraz> and now you have one less think on your TODO :)
20161019 11:31:41-!- Greg-Boggs [~greg_bogg@173.240.241.83] has joined #wesnoth-dev
20161019 11:32:51< vultraz> (and sorry for the stupid bool question earlier :| )
20161019 11:32:58-!- stikonas [~gentoo@wesnoth/translator/stikonas] has joined #wesnoth-dev
20161019 11:36:08-!- Greg-Boggs [~greg_bogg@173.240.241.83] has quit [Ping timeout: 260 seconds]
20161019 11:37:17< irker416> wesnoth: Charles Dang wesnoth:master 339b4f3ee1a0 / data/gui/window/ (chat_log.cfg gamestate_inspector.cfg lua_interpreter.cfg): Enabled some commented out tooltips now that bug #22176 is fixed https://github.com/wesnoth/wesnoth/commit/339b4f3ee1a0743ed18fc71357ea5a333918c75d
20161019 11:40:03-!- Shiki [~Shiki@141.57.62.169] has joined #wesnoth-dev
20161019 11:40:23-!- Shiki is now known as shiki|sevu
20161019 11:41:22-!- ToBeCloud [uid51591@wikimedia/ToBeFree] has joined #wesnoth-dev
20161019 11:57:29-!- Duthlet [~Duthlet@dslb-188-104-253-155.188.104.pools.vodafone-ip.de] has joined #wesnoth-dev
20161019 12:20:56-!- horrowind [~Icedove@2a02:810a:8380:10a8:21b:fcff:fee3:c3ff] has quit [Quit: horrowind]
20161019 12:26:07-!- esr [~esr@wesnoth/developer/esr] has quit [Ping timeout: 252 seconds]
20161019 12:28:26-!- Kwandulin [~Miranda@p5DDD2B8F.dip0.t-ipconnect.de] has quit [Read error: Connection reset by peer]
20161019 12:51:08-!- stikonas [~gentoo@wesnoth/translator/stikonas] has quit [Quit: Konversation terminated!]
20161019 12:52:12-!- bumbadadabum [~bumbadada@wesnoth/developer/bumbadadabum] has joined #wesnoth-dev
20161019 12:52:21-!- stikonas [~gentoo@wesnoth/translator/stikonas] has joined #wesnoth-dev
20161019 13:02:58-!- bumbadadabum [~bumbadada@wesnoth/developer/bumbadadabum] has quit [Ping timeout: 245 seconds]
20161019 13:03:42-!- JyrkiVesterinen [~JyrkiVest@nblzone-242-23.nblnetworks.fi] has quit [Quit: .]
20161019 13:04:51-!- mattsc [~mattsc@wesnoth/developer/mattsc] has quit [Quit: mattsc]
20161019 13:06:16-!- stikonas [~gentoo@wesnoth/translator/stikonas] has quit [Quit: Konversation terminated!]
20161019 13:07:49-!- stikonas [~gentoo@wesnoth/translator/stikonas] has joined #wesnoth-dev
20161019 13:08:26-!- Appleman1234_ [~Appleman1@KD106154015144.au-net.ne.jp] has joined #wesnoth-dev
20161019 13:10:57-!- Appleman1234 [~Appleman1@KD106154001160.au-net.ne.jp] has quit [Ping timeout: 260 seconds]
20161019 13:11:13-!- Appleman1234_ is now known as Appleman1234
20161019 13:20:25-!- mattsc [~mattsc@wesnoth/developer/mattsc] has joined #wesnoth-dev
20161019 13:34:40-!- shiki|sevu [~Shiki@141.57.62.169] has quit [Quit: Verlassend]
20161019 13:43:44-!- Kwandulin [~Miranda@p5DDD2B8F.dip0.t-ipconnect.de] has joined #wesnoth-dev
20161019 13:58:18-!- bumbadadabum [~bumbadada@wesnoth/developer/bumbadadabum] has joined #wesnoth-dev
20161019 14:00:53-!- tad_carlucci [~lundberg@173.217.65.103] has joined #wesnoth-dev
20161019 14:11:55-!- stikonas [~gentoo@wesnoth/translator/stikonas] has quit [Quit: Konversation terminated!]
20161019 14:12:49-!- stikonas [~gentoo@wesnoth/translator/stikonas] has joined #wesnoth-dev
20161019 14:19:41-!- stikonas [~gentoo@wesnoth/translator/stikonas] has quit [Read error: Connection reset by peer]
20161019 14:20:06-!- stikonas [~gentoo@wesnoth/translator/stikonas] has joined #wesnoth-dev
20161019 14:37:42-!- irker416 [~irker@uruz.ai0867.net] has quit [Quit: transmission timeout]
20161019 14:37:48-!- irker265 [~irker@uruz.ai0867.net] has joined #wesnoth-dev
20161019 14:37:48< irker265> wesnoth: mattsc wesnoth:master f2406ac8292d / data/ai/lua/ (generic_recruit_engine.lua generic_rush_engine.lua retreat.lua): Experimental AI: correctly deal with hidden and petrified units https://github.com/wesnoth/wesnoth/commit/f2406ac8292daee890b3053db5fe4161e8f85841
20161019 14:52:21-!- mattsc [~mattsc@wesnoth/developer/mattsc] has quit [Quit: mattsc]
20161019 15:10:57-!- ToBeCloud [uid51591@wikimedia/ToBeFree] has quit [Quit: Connection closed for inactivity]
20161019 15:16:15-!- mattsc [~mattsc@wesnoth/developer/mattsc] has joined #wesnoth-dev
20161019 15:23:14< tad_carlucci> mattsc, Found the source of your crash. When you see it, it makes perfectly good sense it's the source: $ git bisect good
20161019 15:23:14< tad_carlucci> 797613f760d0c84afefa39b305bb512f4a574479 is the first bad commit
20161019 15:25:57< mattsc> tad_carlucci: Nice.  Thanks for doing that!
20161019 15:26:43< mattsc> … and I thought I was just making a quick flippant comment on the side there yesterday  :P
20161019 15:26:55< tad_carlucci> Now I need to figure out the fix.
20161019 15:26:55< tad_carlucci> bbl
20161019 15:26:55-!- tad_carlucci [~lundberg@173.217.65.103] has quit [Quit: Off to resolve a merge conflict between the wife and husband branches of my real life.]
20161019 15:27:27-!- clavii is now known as clavi
20161019 15:37:47-!- Kwandulin [~Miranda@p5DDD2B8F.dip0.t-ipconnect.de] has quit [Read error: Connection reset by peer]
20161019 15:54:20< vultraz> 76 bugs awaiting closure following the next release :D
20161019 16:04:47-!- heirecka [~heirecka@exherbo/developer/heirecka] has quit [Quit: Bye]
20161019 16:06:17-!- Greg-Boggs [~greg_bogg@173.240.241.83] has joined #wesnoth-dev
20161019 16:06:19-!- Greg-Boggs [~greg_bogg@173.240.241.83] has quit [Remote host closed the connection]
20161019 16:06:25-!- Greg-Boggs [~greg_bogg@173.240.241.83] has joined #wesnoth-dev
20161019 16:18:37-!- celticminstrel [~celmin@unaffiliated/celticminstrel] has joined #wesnoth-dev
20161019 16:19:24-!- boucman_work [~boucman@fw-alt.idf.smile.fr] has quit [Ping timeout: 260 seconds]
20161019 16:23:35-!- ChipmunkV [~vova@static-89-94-113-91.axione.abo.bbox.fr] has joined #wesnoth-dev
20161019 16:24:00-!- tad_carlucci [~lundberg@173.217.65.103] has joined #wesnoth-dev
20161019 16:27:51-!- Kwandulin [~Miranda@p5DDD2B8F.dip0.t-ipconnect.de] has joined #wesnoth-dev
20161019 16:30:22-!- iceiceice [~chris@pool-173-61-153-221.cmdnnj.fios.verizon.net] has joined #wesnoth-dev
20161019 16:47:02< celticminstrel> tad_carlucci: Any news from the bisect?
20161019 16:53:21< tad_carlucci> Found the bad commit. Scroll back a bit to find it ^. Working on fixing it.
20161019 16:55:37< celticminstrel> Scrolling won't help. :P
20161019 16:55:48< celticminstrel> I could check the logs, but if you're fixing it, I dunno if I need to.
20161019 17:03:46-!- boucman [~rosen@wesnoth/developer/boucman] has joined #wesnoth-dev
20161019 17:06:12< tad_carlucci> Basically, the commit was trying to reduce code duplication with the panic functions for pcall and broke something. One of yours, from Aug 10th.
20161019 17:21:47-!- mordante [~mordante@wesnoth/developer/mordante] has joined #wesnoth-dev
20161019 17:22:00< mordante> servus
20161019 17:38:27-!- irker265 [~irker@uruz.ai0867.net] has quit [Quit: transmission timeout]
20161019 17:44:39-!- Duthlet [~Duthlet@dslb-188-104-253-155.188.104.pools.vodafone-ip.de] has quit [Quit: leaving]
20161019 17:56:22-!- esr [~esr@wesnoth/developer/esr] has joined #wesnoth-dev
20161019 17:56:44-!- bumbadadabum [~bumbadada@wesnoth/developer/bumbadadabum] has quit [Ping timeout: 245 seconds]
20161019 17:57:09< iceiceice> mordante, i was looking at some of the code that talks to pango recently,
20161019 17:57:17< iceiceice> the class ttext object
20161019 17:57:48< iceiceice> i have a theory that there is a bug in how we are calculating the width and height from pango api
20161019 17:57:53< iceiceice> code lives here now: https://github.com/wesnoth/wesnoth/blob/master/src/font/text.cpp
20161019 17:58:14< iceiceice> at lines like this: https://github.com/wesnoth/wesnoth/blob/master/src/font/text.cpp#L578
20161019 17:58:37< iceiceice> code mostly assume sthat width = `rect_.x + rect_.width` for instance
20161019 17:58:50< iceiceice> my theory is that it maybe should be `-rect_.x + rect_.width`
20161019 17:59:44< iceiceice> i dont really know pango enough to be sure, i was thinking of tweaking many of the calculations in a branch under that assumption, and seeing how it looks
20161019 18:00:04< iceiceice> i know this code is pretty old, but if you have any thoughts on this theory i would appreciate
20161019 18:02:18-!- bumbadadabum [~bumbadada@wesnoth/developer/bumbadadabum] has joined #wesnoth-dev
20161019 18:08:03-!- Kwandulin [~Miranda@p5DDD2B8F.dip0.t-ipconnect.de] has quit [Read error: Connection reset by peer]
20161019 18:24:41-!- heirecka [~heirecka@exherbo/developer/heirecka] has joined #wesnoth-dev
20161019 18:26:04-!- JyrkiVesterinen [~JyrkiVest@89-166-119-48.bb.dnainternet.fi] has joined #wesnoth-dev
20161019 18:53:18< mordante> iceiceice, what is the issue you encounter?
20161019 18:54:53< tad_carlucci> celticminstrel, You're going to have to fix the crash. It's beyond my ken.
20161019 18:55:59< iceiceice> mordante, no specific issue, i just tried to refactor this code to be more easily understandable
20161019 18:56:13< iceiceice> if there's an issue it's indicated in this code comment
20161019 18:56:13< iceiceice> https://github.com/wesnoth/wesnoth/blob/master/src/font/text.cpp#L545
20161019 19:00:24< Aginor> vultraz: I think you should take that fix further to make wiggle-room for the future
20161019 19:00:32< tad_carlucci> vultraz, iceiceice How important is it that the game be able to survive a random Lua error such as the one mattsc created a couple days ago?
20161019 19:01:10< iceiceice> what's the trade-off ?
20161019 19:01:43< tad_carlucci> Coredump on a typo in UMC or mainline.
20161019 19:02:17< iceiceice> i mean like, that's bad, but whats the cost of what you want to do to fix it?
20161019 19:02:42< Aginor> that'll make it very frustrating for someone to figure out what they did wrong though, making for a bad UMC dev experience
20161019 19:02:54< Aginor> if it literally is "coredump on typo"
20161019 19:03:23< tad_carlucci> Declare commit 797613f760d0c84afefa39b305bb512f4a574479 broken and 1.13.6 bloked until it can be successfully reverted. I can't see how to do that.
20161019 19:04:08< mordante> iceiceice, that bug is also ancient and I think it was an issue in pango itself
20161019 19:04:16< tad_carlucci> Aginor, I'm not sure any syntax error will do it. But mattsc's sure does.
20161019 19:04:19< mordante> it may be it is already fixed
20161019 19:04:44< mordante> did you test your theory?
20161019 19:05:00< iceiceice> no, i thought i would ask you about it first
20161019 19:05:33< iceiceice> i might play around with it later, but if you think this is fixed then maybe not worth
20161019 19:06:40< tad_carlucci> iceiceice, Aginor The issue with that commit is it appears to break our exception fix for Lua and our handling of lua errors. At this point I can't say how far-reaching a problem it is. I would not trust any error handling coming out of Lua doing the right thing.
20161019 19:06:55-!- stikonas [~gentoo@wesnoth/translator/stikonas] has quit [Quit: Konversation terminated!]
20161019 19:07:00< iceiceice> celticminstrel, can you comment on what you were trying to do with that commit?
20161019 19:07:23< iceiceice> file doesn't exist anymore i guess
20161019 19:07:29-!- stikonas [~gentoo@wesnoth/translator/stikonas] has joined #wesnoth-dev
20161019 19:07:35< tad_carlucci> The message was pretty clear: reduce duplicated code.
20161019 19:07:38-!- stikonas [~gentoo@wesnoth/translator/stikonas] has quit [Client Quit]
20161019 19:07:51< mordante> IIRC the issue was that pango did not honour the width setting and had to be manually reduced by a small bit to properly honour it
20161019 19:08:14< mordante> the variable has been called hack for a reason ;-)
20161019 19:08:35< tad_carlucci> I put the function back in, from that file, and removed the replacement. It changes the crash message to: error scripting/lua: error in error handling
20161019 19:08:55< iceiceice> tad_carlucci, so i wrote the code that he was moving out of lua kernel base,
20161019 19:09:15< iceiceice> the reason that it makes sense to put it in lua kernel base, is that lua kernel base is responsible for setting up the error handler
20161019 19:09:22 * tad_carlucci shrugs. I didn't look at who did what until I found that commit.
20161019 19:09:50< iceiceice> off hand, if you move it out of there it seems like a bad idea
20161019 19:09:51< tad_carlucci> iceiceice, Actually, there appear to be two areas which do it. And do it differently.
20161019 19:10:08< tad_carlucci> So there's a lot of confusion going on.
20161019 19:10:29< iceiceice> i think my goal was to replace all the luaW_pcall with lua_kernel::protected_call
20161019 19:10:36< iceiceice> at one point
20161019 19:10:45< tad_carlucci> That's the problem.
20161019 19:11:05< iceiceice> well, actually i think the problem is using luaW_pcall wihtout setting up a proper error handler first
20161019 19:11:18< iceiceice> since it can crash the game
20161019 19:12:11< tad_carlucci> Well, how about I step away and let you and celticminstrel work out who does what? I would probably take too long because I don't know what it did, should do, or was supposed to be doing.
20161019 19:13:17< tad_carlucci> All I can say for sure is it used to reasonably recover and let the author see the error and now it coredumps, and bisect proves that commit is where it went wrong.
20161019 19:14:06< iceiceice> i think it's celmin's bug
20161019 19:14:21< iceiceice> bbl
20161019 19:14:24-!- iceiceice [~chris@pool-173-61-153-221.cmdnnj.fios.verizon.net] has quit [Quit: Ex-Chat]
20161019 19:14:31-!- stikonas [~gentoo@wesnoth/translator/stikonas] has joined #wesnoth-dev
20161019 19:18:47-!- mordante [~mordante@wesnoth/developer/mordante] has quit [Quit: Leaving]
20161019 19:20:19-!- stikonas [~gentoo@wesnoth/translator/stikonas] has quit [Remote host closed the connection]
20161019 19:21:07-!- stikonas [~gentoo@wesnoth/translator/stikonas] has joined #wesnoth-dev
20161019 19:26:38-!- JyrkiVesterinen [~JyrkiVest@89-166-119-48.bb.dnainternet.fi] has quit [Quit: .]
20161019 19:28:38-!- stikonas [~gentoo@wesnoth/translator/stikonas] has quit [Quit: Konversation terminated!]
20161019 19:31:13-!- mjs-de [~mjs-de@x5ce33e33.dyn.telefonica.de] has joined #wesnoth-dev
20161019 19:40:04< celticminstrel> tad_carlucci: I wouldn't consider the Lua crash a blocker for a development release. I'd consider it a blocker for a stable release though. However, in this case we could just revert the problem commit, since I don't think there are any advantages other than the reduced code duplication.
20161019 19:41:02< celticminstrel> Mind you, I'd prefer not to revert it, so if anyone can figure out what's really the problem, that'd be better.
20161019 19:41:13< tad_carlucci> celticminstrel, Revert won't take. Missing files. I mocked them to get a clean compile and removed the duplicated function but it just crashes differently
20161019 19:41:16-!- ancestral [~ancestral@8.42.164.20] has joined #wesnoth-dev
20161019 19:42:10< tad_carlucci> I'm giving it another go. But it may be too much for me to handle since I don't know what else you changed which was not part of the problem commit. Something, surely, but what?
20161019 19:42:29< celticminstrel> Wait, it's not just that commit?
20161019 19:45:45< celticminstrel> I can't see anything obviously problematic about that commit. It doesn't look like the flow of code has changed at all, except I think for a moved lua_remove in one case.
20161019 19:46:19< celticminstrel> I suppose I'd have to trace through it more carefully to be sure, but...
20161019 19:46:47< celticminstrel> So it crashes with that commit but does not crash immediately before it?
20161019 19:49:09< tad_carlucci> Correct.
20161019 19:49:35< tad_carlucci> But there is something downstream which prevents it being reverted. Missing files, moved functions. 
20161019 19:50:06< tad_carlucci> I'm working on a min change revert and I guess I'll run gdb to see what's up with the error in error handling.
20161019 19:50:51< celticminstrel> ...why did my SMB connection suddenly die.
20161019 19:51:09< tad_carlucci> ?
20161019 19:51:15-!- stikonas [~gentoo@wesnoth/translator/stikonas] has joined #wesnoth-dev
20161019 19:52:02< celticminstrel> Nothing important. It's just how I pull from Windows to Mac.
20161019 19:52:02< celticminstrel> Connect through SMB, which mounts a directory under /Volumes, and then pull from that path.
20161019 19:52:21< celticminstrel> Because I don't have an sshd set up on the Windows computer.
20161019 19:53:39< tad_carlucci> Oh. I get that all often, too. Reboot the big Windows 10 box and all my Windows shares start working, even the ones which don't involve that box. Hey, it's Microsoft, so who knows.
20161019 19:54:33< celticminstrel> The only thing I can guess is that the computer went into... not sure if it's sleep more or just screen-saving mode, but...
20161019 19:56:58< celticminstrel> ^mode
20161019 20:12:19-!- Greg-Boggs [~greg_bogg@173.240.241.83] has quit []
20161019 20:13:32-!- Greg-Boggs [~greg_bogg@173.240.241.83] has joined #wesnoth-dev
20161019 20:17:37-!- ancestral [~ancestral@8.42.164.20] has quit [Quit: i go nstuf kthxbai]
20161019 20:26:27< tad_carlucci> celticminstrel, I'm trying something really off-the-wall: reflecting luaW_pcall over to lua_kernel_base::protected_call ...
20161019 20:26:35< mattsc> Aginor, iceiceice, tad_carlucci, celticminstrel: just FYI, with the error I encountered, it still gives you a Lua error message before triggering the panic.  For example:
20161019 20:26:38< mattsc> error scripting/lua: ai/lua/generic_rush_engine.lua:346: ')' expected (to close '(' at line 345) near 'if'
20161019 20:27:07 * celticminstrel wonders why you included Aginor in that...
20161019 20:27:18< mattsc> So you do get the information what happened and it’s easy to fix, it’s just annoying that you have to restart Wesnoth (which, I agree, should ideally not happen)
20161019 20:27:37< tad_carlucci> mattsc, That's right. The error appears correctly. It's the down-stream handling which goes wrong. A panic function for the pcall isn't being set or used or something.
20161019 20:27:51< mattsc> celticminstrel: because of this:  “20161019 19:02:42< Aginor> that'll make it very frustrating for someone to figure out what they did wrong though, making for a bad UMC dev experience”
20161019 20:28:07 * tad_carlucci shrugs
20161019 20:28:15< celticminstrel> Ah.
20161019 20:28:26 * mattsc joins tad_carlucci in shrugging
20161019 20:28:31< celticminstrel> He's right though, a panic means that most addon devs won't even see the error message.
20161019 20:28:56< mattsc> And more than that, it’s not just a panic, it’s a PANIC
20161019 20:29:27< celticminstrel> I don't quite understand why iceiceice says that throwing an exception from the atpanic function is bad though...
20161019 20:29:28< tad_carlucci> Yea. It's sorta yells to get your attention, doesn't it?
20161019 20:29:45< celticminstrel> The Lua documentation explicitly suggests doing a longjmp to recover.
20161019 20:29:54< celticminstrel> And I'm not sure how that's different from an exception?
20161019 20:29:58< tad_carlucci> celticminstrel, That's because it's written for C.
20161019 20:30:01< mattsc> celticminstrel: I am not arguing at all that it shouldn’t be fixed.  I’m just providing additional information (which admittedly might not be useful to the problem at hand)
20161019 20:30:07< tad_carlucci> celticminstrel, Stack unwinding
20161019 20:30:46< celticminstrel> But why does stack unwinding cause a problem?
20161019 20:30:58< tad_carlucci> mattsc, Actually, if I get the crash fixed, your typo causes a spam on your screen. 30 or so Lua messages as fast as they can display.
20161019 20:31:29< celticminstrel> Spam is fine as long as you can at least see the latest one or two.
20161019 20:31:39< mattsc> tad_carlucci: that’s no problem for me as I always start Wesnoth dev versions from the terminal
20161019 20:31:39< tad_carlucci> celticminstrel, Because if you longjmp you don't do it, and if you don't do it, destructors don't get called, and that means massive memory leaks.
20161019 20:31:59< celticminstrel> I think you're answering the opposite of what I asked...
20161019 20:32:09< mattsc> celticminstrel: actually, in my experience it’s usually the first that is most important
20161019 20:32:13< celticminstrel> I mean why would unwinding the stack result in a trashed Lua state.
20161019 20:32:18< mattsc> in my experience with Lua AIs
20161019 20:32:37< celticminstrel> mattsc: I would think knowing any of them would suffice... then fix it and try again...
20161019 20:32:50< tad_carlucci> mattsc, generally true for most errors. c++ can be very verbose but the first is usually the one to fix.
20161019 20:33:03< celticminstrel> Though you're probably right that the first is the most likely to be the root cause.
20161019 20:33:31< mattsc> yeah; in any case, for me it’s not a problem and that happens with other types of errors as well
20161019 20:34:08< celticminstrel> tad_carlucci: I don't suppose you have any idea why unwinding the stack would trash the Lua state as iceiceice seems to have been implying?
20161019 20:34:20< celticminstrel> (Assuming it's not unwound past the point where the state was constructed.)
20161019 20:34:21< tad_carlucci> celticminstrel, The problem is an exception bypasses all the Lua cleanup, and a longjmp bypasses all the C++ cleanup. The only workable (sic) solution I've found is to clsoe the lua environment and start afresh.
20161019 20:35:08< celticminstrel> Except that in Wesnoth an exception does not bypass the Lua cleanup (since Lua catches exceptions instead of using longjmp).
20161019 20:35:30< tad_carlucci> And I can see that now ... mattsc: Why does a typo cause all the AI to disapear and break all the WML?
20161019 20:36:02< celticminstrel> I wonder if we should make wesnoth.require error instead of returning nil.
20161019 20:36:07< celticminstrel> (And wesnoth.dofile)
20161019 20:36:26< tad_carlucci> celticminstrel, Well, yes and no. Lua cleans up what it know of. What it has from C++ which needs cleanup it can't do a thing about.
20161019 20:38:54< mattsc> tad_carlucci: umm, I don’t know. I just figured that’s part of the error handling built into the C++ code.
20161019 20:39:19< mattsc> If I can’t figure out what to do, don’t do anything.  That sort of thing.
20161019 20:41:03< tad_carlucci> Consider what would happen if I call Lua and Lua, first, calls C++ to lock the database and blank the screen, which stores the lock object on the Lua stack. Then, later, Lua calls some other function and it throws an exception.
20161019 20:41:54< tad_carlucci> So we unwind the stack back to Lua, convery to a longjmp, panic, convert to an exception, and rethrow. Notice we didn't release the lock and un-blank the screen? Because Lua doesn't know it needs to be done.
20161019 20:44:47< tad_carlucci> So, the way to fix it is to call lua_close, which will release all those objects and the one with the lock will do it's thing. But then we don't have Lua any more so we retart it.
20161019 20:45:34-!- gfgtdf [~chatzilla@x4e36ac6b.dyn.telefonica.de] has joined #wesnoth-dev
20161019 20:45:35< tad_carlucci> And .. bang, no add-ons, no ai, no wml extensions. Just a nice clean Lua because we don't know what we need to do to restore it
20161019 20:48:22< tad_carlucci> What we have works, sorta. But it depends upon (1) ALWAYS using the SAME protocol for pcall and (2) NEVER storing anything in Lua. The current issue is that we violated (1) back in August and are just now realizing it.
20161019 21:05:06-!- mjs-de [~mjs-de@x5ce33e33.dyn.telefonica.de] has quit [Remote host closed the connection]
20161019 21:08:05-!- vultraz [~chatzilla@wesnoth/developer/vultraz] has quit [Ping timeout: 250 seconds]
20161019 21:17:49-!- horrowind [~Icedove@2a02:810a:8380:10a8:21b:fcff:fee3:c3ff] has joined #wesnoth-dev
20161019 21:29:54-!- zookeeper [~lmsnie@wesnoth/developer/zookeeper] has quit [Ping timeout: 250 seconds]
20161019 21:40:30-!- horrowind [~Icedove@2a02:810a:8380:10a8:21b:fcff:fee3:c3ff] has quit [Quit: horrowind]
20161019 21:51:10-!- mattsc [~mattsc@wesnoth/developer/mattsc] has quit [Quit: So long and thanks for all the fish.]
20161019 22:01:43-!- iceiceice [~chris@ip-64-134-98-88.public.wayport.net] has joined #wesnoth-dev
20161019 22:01:43-!- iceiceice [~chris@ip-64-134-98-88.public.wayport.net] has quit [Changing host]
20161019 22:01:43-!- iceiceice [~chris@unaffiliated/iceiceice] has joined #wesnoth-dev
20161019 22:02:06< iceiceice> celticminstrel, i didn't know that the manual suggests longjmp
20161019 22:02:24< iceiceice> i read through mailing list a bit, this thread is informative:
20161019 22:02:27< iceiceice> http://lua-users.org/lists/lua-l/2011-12/msg00124.html
20161019 22:02:48< iceiceice> i guess that you are probably right
20161019 22:02:58< iceiceice> we should ahve like a "lua_panic_exception" or something
20161019 22:02:59< iceiceice> and catch it at title screen or something
20161019 22:03:17< iceiceice> and throw it from panic function
20161019 22:03:37< iceiceice> most of the time you cannot throw from a function that you give to lua
20161019 22:03:47< iceiceice> because it has counters that keep track of when your functions return and such
20161019 22:03:57< iceiceice> and other kinds of things that will get messed up
20161019 22:04:33-!- ChipmunkV [~vova@static-89-94-113-91.axione.abo.bbox.fr] has quit [Quit: ChipmunkV]
20161019 22:04:36< iceiceice> but i guess if we throw from panic function, and the dtor of lua_kernel calls lua_close then it's probably ok if the docs say that longjmp is ok, and roberto says lua_close after panic is ok
20161019 22:05:20< tad_carlucci> I didmn
20161019 22:07:22< tad_carlucci> 'I didn't see that lua_close, specifically, was stated to be safe after a panic. But I might have missed something. Roberto ONLY speaks to C, as far as I cant tell, btw.
20161019 22:07:48< iceiceice> tad_carlucci, there are like 5 emails following that one
20161019 22:07:52< iceiceice> if you click on "follow ups"
20161019 22:08:03< tad_carlucci> hang on
20161019 22:08:29< iceiceice> http://lua-users.org/lists/lua-l/2011-12/msg00164.html
20161019 22:08:37< iceiceice> http://lua-users.org/lists/lua-l/2011-12/msg00173.html
20161019 22:08:49< iceiceice> http://lua-users.org/lists/lua-l/2011-12/msg00167.html
20161019 22:10:07< tad_carlucci> Ah. I see. Yes, the manual says that. I didn't realize it was actually in reference to lua_close. What I did find was a couple game developer blogs about the issues they faced following a lua_close.
20161019 22:10:30< iceiceice> yah in general if you close a corrupted lua state that can be what triggers the segfault ultimately
20161019 22:10:39< tad_carlucci> Basically, you're left in the state of "don't know, best go back to square one"
20161019 22:10:43-!- Bonobo [~Bonobo@2001:44b8:254:3200:3591:b28:85fa:df47] has joined #wesnoth-dev
20161019 22:10:44< iceiceice> since closing actually involves like, tracking down all these objects and calling __gc
20161019 22:10:55< tad_carlucci> Yes.
20161019 22:11:54< tad_carlucci> Anyway: I don't see much difference between crashing to the titlescreen and crashing to the environment other than hiding the messages on the console and preventing a coredump.
20161019 22:12:20< iceiceice> i mean we could try to put the error message in the exception
20161019 22:12:24< iceiceice> and show it in a nice little dialog
20161019 22:12:45< iceiceice> it would be nicer still if we didn't panic and the scenario can continue
20161019 22:12:52< iceiceice> but yah its better than hard crash to desktop
20161019 22:13:06< tad_carlucci> Well, that was the process before the commit in August broke it.
20161019 22:13:22< iceiceice> yah i think making a panic function doesn't fix whatever that problem is
20161019 22:13:25< iceiceice> that's just a separate project
20161019 22:13:30 * tad_carlucci nods.
20161019 22:15:07< tad_carlucci> Which is why I've not done it to date. I only started yesterday because celmin was so upset about ANY crash. But there are things we can't handle. And I don't think we can even be sure catching at the titlescreen will actually clean up
20161019 22:15:49< tad_carlucci> I can envision the report now: It kept crashing to the titlescreen and I kept going back to play, until my computer froze and I had to reboot. HELP.
20161019 22:16:44< iceiceice> i mean that could be said for any exception that we catch at title screen
20161019 22:16:44< iceiceice> it's true though
20161019 22:17:08< iceiceice> i remember at one point we had a bug where, if the theme.cfg indicated a button image that didn't exist, then when you booted to game you crash with uncauhgt exception
20161019 22:17:16< iceiceice> because it throws a gui::button_exception or osmething
20161019 22:17:23< iceiceice> we now catch that at title screen
20161019 22:17:36< iceiceice> but if the *titlescreen* button images aren't found idk what we do, maybe infinite loop
20161019 22:18:24< iceiceice> tad_carlucci, we could try to put some kind of counter so that if a panic happens twice or three times, we kill the program
20161019 22:18:28< tad_carlucci> The way I handled it, way back when, was to have the main daemon fork so it could be in a known-good state at all times. Basically a crash-to-desktop where my program was the desktop.
20161019 22:18:51< tad_carlucci> In our terms: the titlescreen becomes a shell to run everything else.
20161019 22:19:39< iceiceice> what do you do if we can't launch the title screen though?
20161019 22:23:48< tad_carlucci> If there were a portable way to do it, I'd consider catching in main(), launch a replacement and die. And, yes, use some sort of command-line switch to limit the number of times it can happen.
20161019 22:29:32< celticminstrel> [Oct 19@6:12:20pm] iceiceice: i mean we could try to put the error message in the exception
20161019 22:29:33< celticminstrel> [Oct 19@6:12:24pm] iceiceice: and show it in a nice little dialog
20161019 22:29:34< celticminstrel> This. If Lua causes a boot to titlescreen, we need to show the error message, possibly in a similar manner to how WML errors are already shown. But it's definitely better to avoid the panic in the first place using pcall, and while a panic function is good, it's true that it doesn't really fix the underlying issue here.
20161019 22:30:01< celticminstrel> BTW, currently if there's an error before reaching the titlescreen, the game just quits. If it's an error loading GUI2, you don't even get any error message (except on the console of course).
20161019 22:30:17< iceiceice> celticminstrel, so, one of the things that i tried to do when i made the lua_kernel_base and all that,
20161019 22:30:25< iceiceice> i wanted to like, encapsulate the creation of lua state
20161019 22:30:29< iceiceice> and establish invariants related to its setup
20161019 22:30:42< iceiceice> my view is that like, luaW_pcall is sort of unsafe
20161019 22:30:51< iceiceice> because it requires you to assume that setup already happened
20161019 22:31:09< iceiceice> what i wanted to do was like, move most of our lua related functions to be methods of some kernel
20161019 22:31:25< iceiceice> any time that you are doing pcall, the error handler is specific to how the lua state is set up
20161019 22:31:28< iceiceice> if its the game lua state
20161019 22:31:30< iceiceice> if its a different lua state
20161019 22:31:35< iceiceice> if the lua interpreter dialog is attached
20161019 22:31:45< iceiceice> all the errors are handled slightly differently, and it uses inheritance to figure out which one
20161019 22:31:59< iceiceice> but i didn't really get to the point of refacotirng the AI code
20161019 22:32:44-!- skeletenchi [enchilado@defocus/yummy/enchilado] has quit [Ping timeout: 250 seconds]
20161019 22:32:53< iceiceice> actually it wasn't at all clear to me how it should be done
20161019 22:32:59< iceiceice> what i think i wanted to do was make like a new lua kernel for the ai
20161019 22:33:06< iceiceice> but mattsc tells me that would break everything
20161019 22:33:18< iceiceice> and that its essential that the scenario lua and the ai lua can talk to eachother
20161019 22:33:54< iceiceice> if i had continued with the lua state refactor, what i would have tried to do is move the ai code into the game_lua_kernel as methods probly
20161019 22:34:07< iceiceice> and get rid of luaW_pcall
20161019 22:34:23< iceiceice> but i did not open that can of worms, its up to you guys to approach it how you want
20161019 22:36:28< celticminstrel> Well, I suppose we could get rid of luaW_pcall...
20161019 22:36:34< tad_carlucci> My approach is to figure out what changed and why it broke things and then do the minimum to get it back so mattsc's typo does what it used to do: spam the screen with cascading failures.
20161019 22:37:28< celticminstrel> I wonder if it would really break things to have a separate Lua state for AI...
20161019 22:37:49< iceiceice> it would be cleaner i think
20161019 22:37:53< iceiceice> if u wanted to do like simulation ais
20161019 22:37:59< tad_carlucci> celticminstrel, I just tried having luaW_pcall forward the work to lua_kernel_base::protected_call but that caused a segfault. So I'm back to looking for other approaches.
20161019 22:38:00< celticminstrel> I guess you'd need a way to choose which state to open in the console.
20161019 22:38:08< iceiceice> but i think what i concluded is that wesnoth engine can't really do simulation ais
20161019 22:38:53< celticminstrel> It can do simulation with --nogui. Though that simulation means running everything just like a normal game, so maybe it doesn't quite count.
20161019 22:39:04< tad_carlucci> Frankly, I'd have a different state for a lot of parts. Mainline/core Lua separate from Campaign/Scenarion, for instance.
20161019 22:39:26< celticminstrel> That ... doesn't really sound workable?
20161019 22:40:17< tad_carlucci> It's a major design change. Won't work with the design we have now. But nothing prevents it from working otherwise.
20161019 22:41:01-!- skeletenchi [enchilado@defocus/yummy/enchilado] has joined #wesnoth-dev
20161019 22:51:14< tad_carlucci> And that is why the gdb backtrace is misleading: the AI being loaded is not the AI causing the crash. The error is in generic_recruit_engine. The lua being loaded is generic_rush_engine.
20161019 22:51:46< celticminstrel> gdb backtrace? Not Lua backtrace?
20161019 22:53:28< tad_carlucci> The Lua backtrace does not tell us which file triggered the panic. The message about the missing ')' comes from recruit loading INSIDE rush. So I'm about to load rush (no errors because it's fine) and am setting a breakpoint to catch when rush loads recruit to walk down the failure chain and see if I can see where it goes awry
20161019 22:56:43< tad_carlucci> And there is the first problem. Our C++ was just told there's an error and we're ignoring it completely.
20161019 22:56:43-!- bumbadadabum [~bumbadada@wesnoth/developer/bumbadadabum] has quit [Ping timeout: 245 seconds]
20161019 23:03:20-!- boucman [~rosen@wesnoth/developer/boucman] has quit [Remote host closed the connection]
20161019 23:06:09< tad_carlucci> Stack corruption. The error message is supposed to be on the top but something else is. So we cause a panic fetching the message, and since we're already in the exception handling, the pcall protected area is done and we're about to head toward PANIC and abort()
20161019 23:06:19< tad_carlucci> So .. to find the Lua stack corruption.
20161019 23:10:39< tad_carlucci> And that's why the backtrace is meaningless. It's showing the PANIC caused by the improperly handed panic.
20161019 23:14:44-!- irker253 [~irker@uruz.ai0867.net] has joined #wesnoth-dev
20161019 23:14:45< irker253> wesnoth: Celtic Minstrel wesnoth:master 166809e8f735 / src/units/attack_type.hpp: Fix leak of attack types https://github.com/wesnoth/wesnoth/commit/166809e8f73542982f30d6057cc5820c96fa22fc
20161019 23:17:52< iceiceice> tad_carlucci, one thing which i developed in primer which i found extremely useful for tracking down stack discipline issues
20161019 23:18:03< iceiceice> is an object "assert_stack_neutral"
20161019 23:18:23< iceiceice> i can send a link, one sec
20161019 23:18:32-!- bumbadadabum [~bumbadada@wesnoth/developer/bumbadadabum] has joined #wesnoth-dev
20161019 23:19:04< iceiceice> https://github.com/cbeck88/lua-primer/blob/master/include/primer/support/asserts.hpp#L74
20161019 23:19:39< iceiceice> so it defines this object "stack_neutrality_assertion"
20161019 23:19:51< iceiceice> which keeps track of the `lua_gettop` value when it is created
20161019 23:20:00< iceiceice> and in the dtor, it checks if `lua_gettop` is the same
20161019 23:20:07< iceiceice> if not it asserts false, and reports the file and line number
20161019 23:20:20< celticminstrel> I wonder why it's an assertion rather than RAII.
20161019 23:20:23< iceiceice> you put these little lines `ASSERT_STACK_NEUTRAL(L);` where L is the lua state
20161019 23:20:46< iceiceice> when you build in debug mode,  then it constantly checks the stack discipline
20161019 23:20:54< iceiceice> in release mode, the macros just go away
20161019 23:21:02< celticminstrel> Callback functions do not need to be stack neutral though.
20161019 23:21:13< iceiceice> yah sometimes you have to get creative
20161019 23:21:16< celticminstrel> And some functions must not be stack neutral.
20161019 23:21:17< tad_carlucci> What we have here is an unprotected call where we assume we're protected, a mishandled error condition, and a stack corruption. Probably fixing any one will fix a lot of problems.
20161019 23:21:19< iceiceice> like, you make a custom scope for it
20161019 23:21:42< tad_carlucci> Oh, and the unprotected call? lua_load()
20161019 23:21:57< iceiceice> if you want you could make a version that asserts that the stack changes by a fixed amount
20161019 23:22:01< iceiceice> but i didn't find that necessary
20161019 23:22:01< celticminstrel> Where is the unprotected call? src/ai/engine_lua.cpp?
20161019 23:22:21< tad_carlucci> fileops 
20161019 23:22:31< celticminstrel> Huh?
20161019 23:22:35< tad_carlucci> lua_load
20161019 23:22:39< celticminstrel> In the definition of wesnoth.require/dofile?
20161019 23:22:52< iceiceice> celticminstrel, it's not really an RAII object, it's not managing a resource
20161019 23:23:06< iceiceice> idk what you call it
20161019 23:23:20< celticminstrel> Wait, hold on, lua_load isn't supposed to panic on errors, is it? It returns a status code.
20161019 23:25:19< tad_carlucci> It's an unprotected call and we mishandle the return code.  Error #3 from lua_load gets ignored and trying to recover we cause a LUA error #2 when we go for the string which isn't there.
20161019 23:27:08< tad_carlucci> lua_load did the right thing, gave a string "missing )" and error 3 .. we're crashing because the error string isn't there when we go to get it for error #2 (3 and 2 are lua constants)
20161019 23:27:58-!- bumbadadabum [~bumbadada@wesnoth/developer/bumbadadabum] has quit [Ping timeout: 245 seconds]
20161019 23:27:59< tad_carlucci> I'm resetting to take a closer look at what we do about lua_load returning an error. Seemed fishy.
20161019 23:28:35-!- bumbadadabum [~bumbadada@wesnoth/developer/bumbadadabum] has joined #wesnoth-dev
20161019 23:28:54< celticminstrel> Okay, so it sounds like the status code of lua_load needs to be handled properly?
20161019 23:29:32< iceiceice> celticminstrel, why did you insert this line?
20161019 23:29:33< iceiceice> https://github.com/wesnoth/wesnoth/commit/797613f760d0c84afefa39b305bb512f4a574479#diff-a59a7933beaab823a69cf286a14d4a8bR85
20161019 23:29:43< iceiceice> between pcall and rethrow?
20161019 23:30:24< celticminstrel> Well, it was there in the original, though in a different place.
20161019 23:30:37< celticminstrel> I didn't add anything completely new in that commit...
20161019 23:30:50< iceiceice> it's not a refactor if you change the behavior
20161019 23:31:10< celticminstrel> Any behaviour change is unintended though.
20161019 23:31:33< celticminstrel> So basically, what you seem to be saying is that it should go after the rethrow?
20161019 23:31:40< tad_carlucci> What does this mean? I jsut noticed it stepping into the MP scenario
20161019 23:31:43< tad_carlucci> 20161019 18:30:19 error desktop: Failed to send visual notification: The name org.freedesktop.Notifications was not provided by any .service files
20161019 23:32:00< iceiceice> i mean yah i dont know for a fact that it matters but it definitely changes the behavior
20161019 23:32:12< iceiceice> it means you remove an extra entry in cases when you end up throwing
20161019 23:33:48-!- bumbadadabum [~bumbadada@wesnoth/developer/bumbadadabum] has quit [Ping timeout: 245 seconds]
20161019 23:34:59< iceiceice> i guess could test if changing the order of those back fixes the crash?
20161019 23:35:02< loonycyborg> tad_carlucci: desktop notifications for when it's your turn and wesnoth isn't active app
20161019 23:35:09< loonycyborg> and stuff like that
20161019 23:35:26< loonycyborg> there are multiple protocols that we support
20161019 23:35:43< tad_carlucci> loonycyborg, Ah. Well now I know.
20161019 23:35:57-!- iceiceice [~chris@unaffiliated/iceiceice] has quit [Quit: Ex-Chat]
20161019 23:37:36-!- mattsc [~mattsc@wesnoth/developer/mattsc] has joined #wesnoth-dev
20161019 23:40:31-!- bumbadadabum [~bumbadada@wesnoth/developer/bumbadadabum] has joined #wesnoth-dev
20161019 23:42:35< tad_carlucci> Wait a minute .. I continued and it's loading more Lua .. just like 1.13.5 .. it's not until it gets to loading and calling :move_to_enemy_exec() that we crash. talk about being far from the cause.
20161019 23:52:02< tad_carlucci> Ah. It was running core/macros/ai.cfg and loading all the candidate actions .. that was jsut the last one loaded.
--- Log closed Thu Oct 20 00:00:47 2016