--- Log opened Thu Jul 14 00:00:13 2016
--- Day changed Thu Jul 14 2016
20160714 00:00:13< celmin> Is delete_handler_index only ever called in that one file?
20160714 00:00:56< vultraz> yes
20160714 00:01:02< celmin> Hmm.
20160714 00:01:10< celmin> So it's only called by remove_handler.
20160714 00:01:43< celmin> Maybe remove_handler should be changed to take an iterator… how many places is that called from?
20160714 00:01:45< vultraz> and once in has_focus
20160714 00:02:22< celmin> The definition of remove_handler implies a linear search, which is quite silly.
20160714 00:02:40< vultraz> remove_handler is called twice 
20160714 00:02:42< celmin> If you have an iterator to a list element, then you don't need to search for it.
20160714 00:02:50< vultraz> in leave and leave_globa
20160714 00:02:51< vultraz> l
20160714 00:03:01< celmin> I see.
20160714 00:03:07< celmin> So again, only in that file.
20160714 00:03:28< celmin> Hmm, it's called with this...
20160714 00:03:28-!- Greg-Boggs [~greg_bogg@173.240.241.83] has joined #wesnoth-dev
20160714 00:03:33-!- RatArmy [~RatArmy@133.15.175.65] has quit [Ping timeout: 240 seconds]
20160714 00:03:35< celmin> How annoying...
20160714 00:04:23< vultraz> for some reason this code is making my brain hurt
20160714 00:06:00< vultraz> celmin: so what do you recommend, here
20160714 00:06:08< vultraz> I've never worked with std::list
20160714 00:07:08< celmin> I'm not sure, but vaguely thinking about changing join() in some way to obtain an iterator that can be used in leave()...
20160714 00:08:34< celmin> But I suppose for now it could be sufficient to delete the delete_handler_index function and push its logic into remove_handler somehow...
20160714 00:08:40-!- prkc [~prkc@catv-80-98-46-199.catv.broadband.hu] has quit [Ping timeout: 250 seconds]
20160714 00:09:09< vultraz> shouldn't the behavior of remove_handler be different with a list instead of a vector
20160714 00:09:30< celmin> Possibly.
20160714 00:10:12< celmin> However, the way it's defined currently requires a search for the handler to remove, regardless of what type of container is being used.
20160714 00:10:12-!- bumbadadabum [~bumbadada@wesnoth/developer/bumbadadabum] has joined #wesnoth-dev
20160714 00:10:13< vultraz> shouldn't iterating through the list and deleting it if it matches ptr be enough?
20160714 00:11:04< celmin> I think so? Basically handlers.erase(i) should do the trick. You probably also need to update focused_handler if it's equal to i.
20160714 00:11:20< celmin> (Assuming you've made it an iterator.)
20160714 00:11:51< celmin> The separate handling for when you're deleting the last element is good though.
20160714 00:12:11< vultraz> is a list ordered?
20160714 00:12:14< celmin> Since it's probably common to be deleting the last element?
20160714 00:12:21< celmin> Yes a list is ordered.
20160714 00:15:13< vultraz> something like handlers.erase(handlers.back()); for the last element?
20160714 00:15:36< celmin> Yeah, but before that you probably want to decrement focused_handler.
20160714 00:15:44< celmin> If it's equal, that is.
20160714 00:15:49< vultraz> (I swear, one of these days I'll take an actual class on this and really, properly learn iterators and lists and all this stuff)
20160714 00:16:06< celmin> ie, if *focused_handler == handlers.back()
20160714 00:16:33< vultraz> so wait, we don't want to check if(handlers.back() == ptr) ?
20160714 00:17:05< celmin> Oh, that's true, you could instead compare *focused_handler == ptr - might be marginally more efficient.
20160714 00:17:15< celmin> (To actually answer your question, yes of course you want to check that.)
20160714 00:17:52< vultraz> ok, so deref focused_handler and then decrease it?
20160714 00:18:06< vultraz> wait, no
20160714 00:18:18< vultraz> delete the back and reassign it?
20160714 00:18:35< celmin> Basically you need to make sure focused_handler is a valid iterator, so if it happens to be pointing to the handler that will be deleted, then you need to make focused_handler point to a different handler (or to end()) before deleting it.
20160714 00:19:05< vultraz> ohh
20160714 00:19:07< vultraz> I see
20160714 00:19:10< vultraz> back returns the elemtnt
20160714 00:19:14< celmin> So if focused_handler is pointing to back(), decrement it before deleting back().
20160714 00:19:14< vultraz> end returns an iterator to the end
20160714 00:19:19< vultraz> last element*
20160714 00:19:35< vultraz> wait
20160714 00:19:37< vultraz> focused_handler is an int :|
20160714 00:19:38< celmin> Yes, but end() is not the iterator to the last element - it's the iterator after the last element.
20160714 00:19:48< celmin> Yes, I already said you should make focused_handler an iterator.
20160714 00:19:51< vultraz> ok
20160714 00:20:09< vultraz> so :
20160714 00:20:11< vultraz> 	std::list<sdl_handler*> handlers;
20160714 00:20:13< vultraz> 	handlers::iterator focused_handler;
20160714 00:20:14< vultraz> right?
20160714 00:20:30< celmin> No, handlers is not a type name so you can't do that.
20160714 00:20:38< celmin> You need std::list<sdl_handler*>::iterator
20160714 00:20:53< vultraz> unless I typedef
20160714 00:20:56< vultraz> ?
20160714 00:21:02< celmin> You can do a typedef/using if you don't like the duplication, yes.
20160714 00:21:35< vultraz> using?
20160714 00:21:49-!- prkc [~prkc@46.166.138.150] has joined #wesnoth-dev
20160714 00:21:59< celmin> It's the new C++ way of making typedefs. Can be easier to read in complex cases.
20160714 00:22:16< vultraz> can I have an example?
20160714 00:22:20< celmin> using T = int; is equivalent to typedef int T;
20160714 00:22:29< vultraz> huh
20160714 00:22:48< vultraz> okk
20160714 00:22:49< celmin> And using T = int(*)(int) is equivalent to typedef int(* T)(int)
20160714 00:23:00< vultraz> if I change focued_handler to an iterator that means I have to change a bunch of code
20160714 00:23:00< celmin> Which is an example of the aforementioned complex cases.
20160714 00:23:21-!- stikonas [~gentoo@wesnoth/translator/stikonas] has quit [Quit: Konversation terminated!]
20160714 00:23:25< celmin> Yeah.
20160714 00:23:49< celmin> I would expect it to be a fairly straightforward change, since handlers[i]. could be replaced with i->
20160714 00:24:03< vultraz> ok, so..
20160714 00:24:05< vultraz> 	if(*focused_handler == ptr) {
20160714 00:24:06< vultraz> 		focused_handler = handlers.end();
20160714 00:24:08< vultraz> 		handlers.erase(handlers.back());
20160714 00:24:09< vultraz> is that what you meant?
20160714 00:24:35< celmin> Not quite?
20160714 00:24:43< vultraz> :(
20160714 00:25:15< celmin> First of all, there should be a closing brace after the second line. (I assume this is all within the if(handlers.back() == ptr) condition.)
20160714 00:25:30< vultraz> yes, this is just a snippet
20160714 00:25:37< celmin> And it's better to do --focused_handler rather than assigning end().
20160714 00:25:43< vultraz> oh
20160714 00:25:44-!- Greg-Boggs [~greg_bogg@173.240.241.83] has quit [Remote host closed the connection]
20160714 00:25:46< vultraz> wait, you can do that?
20160714 00:25:48< celmin> The erasure should happen regardless of the value of focused_handler.
20160714 00:25:52-!- Greg-Boggs [~greg_bogg@173.240.241.83] has joined #wesnoth-dev
20160714 00:26:03< celmin> Yes, it's an iterator so that means it supports ++ and --.
20160714 00:26:10< vultraz> blagh
20160714 00:26:14< vultraz> I need to learn this stuff
20160714 00:26:18< celmin> All iterators support ++; some don't support --, but a list iterator does.
20160714 00:26:21< vultraz> I thought that was for numbers
20160714 00:26:29< celmin> It's for numbers and iterators.
20160714 00:26:36< celmin> You've never seen people using it on pointers?
20160714 00:26:45< vultraz> I don't know
20160714 00:27:07< celmin> The most basic iterator type is one that supports *x and ++x
20160714 00:27:33< celmin> List iterators are bidirectional, so they also support --x
20160714 00:27:47< vultraz> I see
20160714 00:27:53< celmin> Vector iterators are random-access, so they also support x+n and x-x
20160714 00:29:21< celmin> Anyway, it's best to make sure that focused_handler is always a valid iterator, so it should only be equal to end() if there are no elements at all.
20160714 00:29:27< celmin> I think.
20160714 00:29:41< celmin> Unless it's possible to have a case where there are handlers but none of them are focused
20160714 00:29:50< vultraz> ok, how's this look? http://pastebin.com/4AF6JRXN
20160714 00:29:53< celmin> In that case I guess it would also be equal to end().
20160714 00:30:13< celmin> Uh, what happend to if(handlers.back() == ptr)?
20160714 00:30:15< celmin> ^happened
20160714 00:30:25< vultraz> blah, 
20160714 00:30:37< vultraz> I thought you said to change that
20160714 00:30:42< celmin> That's the main condition, the test for focused_handler only covers the decrement.
20160714 00:30:55< celmin> No, you need both of them.
20160714 00:31:09< vultraz> ok
20160714 00:31:27< celmin> At line 7 of the paste you also need a test for focused_handler
20160714 00:31:43< celmin> Something like if(focused_handler == i) {--focused_handler}
20160714 00:32:07< celmin> Except that if focused_handler==begin() then you should increment instead of decrement (but only if it's also equal to i)
20160714 00:32:09< vultraz> updated paste
20160714 00:32:27-!- Greg-Boggs [~greg_bogg@173.240.241.83] has quit [Remote host closed the connection]
20160714 00:32:28 * vultraz groans
20160714 00:33:01< celmin> I wonder if end() remains the same as elements are added and removed.
20160714 00:33:18< celmin> I guess it would make sense.
20160714 00:33:46-!- ChipmunkV [~vova@d0017-2-88-172-31-68.fbx.proxad.net] has quit [Quit: ChipmunkV]
20160714 00:34:34< vultraz> updated paste again
20160714 00:34:55< vultraz> dat nesting
20160714 00:35:20< celmin> Wait a second, what happened to the find() call?
20160714 00:35:42< celmin> …wait, what?
20160714 00:35:50 * vultraz faceplants
20160714 00:36:15< celmin> Oh, for(handler_list h: handlers) won't compile, right?
20160714 00:36:38< celmin> You don't need a for-loop, just keep the find call and replace h with i in what you have in the paste.
20160714 00:36:39< vultraz> I don't know, I can't build anything right now becuase I haven't finished updating focused_handler's usecases
20160714 00:37:23< celmin> If you want to reduce nesting a little you can if(i == end()) {return false;}
20160714 00:40:17< vultraz> updated paste
20160714 00:40:39< vultraz> blagh I was using ~= and not !=
20160714 00:40:42< celmin> …well, you can put the return on its own line at least.
20160714 00:40:55< celmin> Uh, is that what that was meant to be? I was wondering.
20160714 00:41:23< celmin> Yeah okay sure whatever. Not the way I would do it, but I guess it's not the worst use of the ternary operator.
20160714 00:41:41< celmin> I actually assumed ~= was meant to be some sort of assignment operator.
20160714 00:41:56< vultraz> nah, i'm just in the lua mentality
20160714 00:41:58< vultraz> :P
20160714 00:42:49< vultraz> ok, now what is the depth stuff
20160714 00:43:16< vultraz> also I assume focused_handler = -1 is now supposed to be handlers.end()?
20160714 00:43:26< celmin> That seems reasonable.
20160714 00:43:33< celmin> Depth stuff?
20160714 00:44:11< vultraz> 	static int depth = 0;
20160714 00:44:13< vultraz> 	++depth;
20160714 00:44:20< vultraz> 	--depth;
20160714 00:44:22< vultraz> 	if(depth == 0) {
20160714 00:44:23< vultraz> 		cycle_focus();
20160714 00:44:30< celmin> I dunno, ask aginor?
20160714 00:44:42< vultraz> hm
20160714 00:44:58< vultraz> looking at cycle_focus it seems to deal with setting focused_handler
20160714 00:45:13< celmin> I would assume that from the name.
20160714 00:45:31< vultraz> it looks through handlers()
20160714 00:46:43< vultraz> well, this will have to change
20160714 00:48:12< celmin> You could store a copy of focused_handler, and just increment it; when it's equal to end(), set it to begin().
20160714 00:48:22< celmin> If it's equal to the copy, break.
20160714 00:48:41< vultraz> a copy?
20160714 00:48:43< vultraz> where
20160714 00:49:20< celmin> Before the loop, store a copy of the focused_handler in a local variable. You can even use auto if you don't want to type out the iterator's type name in full.
20160714 00:49:29< Aginor> you may also need to worry about what happens to that value when the element it pointed to is deleted
20160714 00:49:41< Aginor> although I'm not entirely convinced that code is actually used...
20160714 00:50:19< celmin> Aginor: I think that's what we were worrying about just now.
20160714 00:50:46< vultraz> as far as I can tell, since depth is static it's cumulative... and cycle_focus happens only if there's only one context
20160714 00:50:49< vultraz> so it's odd
20160714 00:50:57< celmin> vultraz: The logic of cycle_focus doesn't really change much with focused_handler being an iterator.
20160714 00:51:19< vultraz> celmin: I'm trying to figure out the logic of it being there in the first plac
20160714 00:51:21< vultraz> e
20160714 00:51:28 * celmin shrugs.
20160714 00:51:42< vultraz> the function is only called in this one place
20160714 00:52:04< vultraz> and doesn't seem to do much of anything besides set focused_handler
20160714 00:52:14< vultraz> maybe Aginor can elaborate
20160714 00:52:17< vultraz> im going to get lunch
20160714 00:53:31-!- Ravana_ [~Ravana@unaffiliated/ravana/x-2327071] has quit [Read error: Connection reset by peer]
20160714 01:07:46< vultraz> celmin, Aginor: there's something about the depth logic that makes me feel the cycle code is unnecessary 
20160714 01:12:42< vultraz> as far as I can tell
20160714 01:12:50< vultraz> unless you try to remove a handler that doesn't exist
20160714 01:12:56< vultraz> depth will always be 0
20160714 01:13:09< vultraz> brb restarting irc
20160714 01:13:12-!- vultraz [~chatzilla@wesnoth/developer/vultraz] has quit [Quit: ChatZilla 0.9.92-rdmsoft [XULRunner 35.0.1/20150122214805]]
20160714 01:14:00-!- vultraz [~chatzilla@wesnoth/developer/vultraz] has joined #wesnoth-dev
20160714 01:15:35< vultraz> since i find it unlikely that depth is ever not 0, then cycle_focus is always called..
20160714 01:15:36< vultraz> in remove_handler, we just either decremented or incremented focused_handler
20160714 01:15:43< vultraz> then in cycle_focus, we increase it again
20160714 01:16:26< celmin> That's not quite what cycle_focus does.
20160714 01:16:46< celmin> cycle_focus could change the focused_handler to any valid iterator from the list.
20160714 01:23:35< vultraz> celmin: how this http://pastebin.com/uTaKL7Fu
20160714 01:23:37< vultraz> hows
20160714 01:23:59< celmin> That's a different logic.
20160714 01:24:30< celmin> It probably won't have the intended effect.
20160714 01:24:53< celmin> The intended logic is to begin at the current focused_handler and loop through until the next handler requiring focus is reached.
20160714 01:25:08< celmin> (If the current one requires focus it might even stop right away.)
20160714 01:25:54< celmin> That's why I said to store a copy of focused_handler, since when you're looping around like that it's your only way to tell that you've checked all of them.
20160714 01:26:35< celmin> …why does cycle_focus both set focused_handler and return it? The return value isn't used, either...
20160714 01:26:40-!- RatArmy [~RatArmy@133.15.175.65] has joined #wesnoth-dev
20160714 01:27:20< vultraz> there's a namespace-only void cycle_focus() that's not implemented
20160714 01:27:39< vultraz> dunno why the context class version returns the focused handler
20160714 01:28:51< vultraz> GAH
20160714 01:29:00< vultraz> I cannot seem to figure out the logic you describe in iterators
20160714 01:29:41< celmin> It's almost the same as it was with indices though.
20160714 01:30:04< celmin> If the iterator == end(), set it to begin()
20160714 01:30:52< celmin> The only other quirk is to avoid an infinite loop (which, thinking about it, that existing code doesn't seem to actually do?).
20160714 01:31:00-!- gfgtdf [~chatzilla@x4e36a56a.dyn.telefonica.de] has quit [Quit: ChatZilla 0.9.92 [Firefox 47.0/20160604131506]]
20160714 01:31:23< vultraz> celmin: http://pastebin.com/uTaKL7Fu
20160714 01:32:15< vultraz> wait
20160714 01:32:18< celmin> Um… you can't use a for-range to implement this sort of logic. You need the for-with-iterators version, because you're starting (and ending) iteration in the middle.
20160714 01:32:39< vultraz> right, just realized
20160714 01:33:05< celmin> Also, focus_handler implies to me "a handler that is triggered by a focus change", rather than the correct implication of "the current handler that is focused".
20160714 01:33:44< Aginor> I can't elaborate too much while at work, sorry :)
20160714 01:33:56< vultraz> wait, why...
20160714 01:34:02< vultraz> celmin: ok you just confused me
20160714 01:34:15< vultraz> you're never using the i value
20160714 01:34:20< celmin> I'm complaining about your choice of name "temp_focus_handler".
20160714 01:34:21< celmin> Wait, what?
20160714 01:34:31< Aginor> but if you want to drop it, commit it to a branch and we can look
20160714 01:34:32< celmin> What are you talking about?
20160714 01:34:48< vultraz> you've confused me about what type of for() loop to use
20160714 01:35:03< vultraz> and why it need be so
20160714 01:35:04< celmin> A regular one where the loop variable is an iterator.
20160714 01:35:28< celmin> The basic form being for(auto iter = handlers.begin(); iter != handlers.end(); ++iter)
20160714 01:35:28< vultraz> but why, we never use the loop value!
20160714 01:35:38< celmin> But a bit different here.
20160714 01:35:57-!- RatArmy [~RatArmy@133.15.175.65] has quit [Ping timeout: 260 seconds]
20160714 01:36:17< celmin> Because you want to start at focused_handler rather than at begin(), and the ending condition is comparing to temp_focused_handler instead of end().
20160714 01:36:37< celmin> And within the loop you need to set it to begin() if it ends up being equal to end().
20160714 01:37:02< celmin> I think your confusion is stemming from the assumption that the loop variable in a for-loop is always an index, but that's not the case at all.
20160714 01:37:25< vultraz> wait, this doesn't make sense
20160714 01:37:31< vultraz> we don't touch anything from the loop
20160714 01:37:39< vultraz> so how is temp_focus_handler ever increased?
20160714 01:37:47< vultraz> whether as index or iterator
20160714 01:38:00< celmin> The whole point is that it's not increased. It's your sentinel value.
20160714 01:38:29< celmin> In your last paste, keep line 3 but drop line 4.
20160714 01:38:55< celmin> The loop can look like this: for(++focused_handler; focused_handler != temp_focused_handler; ++focused_handler)
20160714 01:38:59< celmin> ^loop header
20160714 01:39:20< celmin> The contents of the loop are basically right but should be testing on focused_handler.
20160714 01:39:32< vultraz> we make a copy of focused_handler,  then every loop cycle it looks whether the copy need focus
20160714 01:39:35< vultraz> but the copy never changes!
20160714 01:39:46< vultraz> unless the copy was already the end of the loop
20160714 01:39:57< celmin> Bleh.
20160714 01:40:00< celmin> Okay, reverse it then.
20160714 01:40:09< vultraz> so basically, the copy can only be one of two values
20160714 01:40:12< celmin> Use focused_handler as your sentinel.
20160714 01:40:14< vultraz> begin, or not end
20160714 01:40:48< celmin> Again, keep line 3 of your paste.
20160714 01:41:13< celmin> Loop header can look like for(++temp_focused_handler; temp_focused_handler != focused_handler; ++temp_focused_handler)
20160714 01:41:23< celmin> Then the contents of the loop should be fine without change, I think.
20160714 01:41:30< celmin> Yeah.
20160714 01:41:31< vultraz> ...wha??
20160714 01:41:34 * vultraz reads that line
20160714 01:41:57< celmin> If you prefer you can keep line 4 of the paste you showed me and omit the first instance of that from the loop header.
20160714 01:42:05< vultraz> so, initial statement is to bump the copy... and increase the copy as long as it's not the focu
20160714 01:42:07< vultraz> focus
20160714 01:42:27< vultraz> since inside the loop, the copy is then reset to the beginning if it reaches the end, it will loop all the way around
20160714 01:42:30< vultraz> is that right?
20160714 01:42:30< celmin> Then at line 16 assign focused_handler = temp_focused_handler.
20160714 01:42:34< celmin> Yeah.
20160714 01:42:43< vultraz> see now this makes sense
20160714 01:42:50< vultraz> now we're doing something with the value in the loop
20160714 01:42:56< vultraz> you understand why I was lost before?
20160714 01:43:07< celmin> Yeah.
20160714 01:43:32< celmin> My initial thought basically involved swapping every instance of focused_handler and temp_focused_handler in that code.
20160714 01:43:39< celmin> Except for the first line.
20160714 01:44:01< celmin> But somehow that didn't get through to you, so I reversed it.
20160714 01:44:07< vultraz> [12:42:29]	celmin	Then at line 16 assign focused_handler = temp_focused_handler.
20160714 01:44:17< celmin> Right, because the end goal is to update focused_handler.
20160714 01:44:20< vultraz> how come? in case nothing needs focus?
20160714 01:44:34< vultraz> because it's already set at line 12 if something does need focus
20160714 01:44:44< celmin> Ah, you're right, silly me.
20160714 01:45:08< vultraz> I don't think we need to remember the temp otherwise
20160714 01:45:19< celmin> If nothing needs focus then it probably doesn't matter which handler is considered to be focused.
20160714 01:45:35< celmin> So yeah, I guess you can ignore the line 16 comment.
20160714 01:46:47< vultraz> ok http://pastebin.com/uTaKL7Fu
20160714 01:46:54< vultraz> is this all you meant
20160714 01:47:33< celmin> That look fine.
20160714 01:48:17< vultraz> ok, now to fix the rest of the focused_handler cases
20160714 01:51:15< vultraz> is there a difference between const  handler_list::iterator and handler_list::const_iterator
20160714 01:52:34< celmin> Yes.
20160714 01:52:45< vultraz> celmin: I should be able to remove delete_handler_index, right?
20160714 01:52:54< celmin> It's the same as the difference between const handler_list* and handler_list*const
20160714 01:52:57< celmin> Yes.
20160714 01:53:02< vultraz> im pondering the use in has_focus
20160714 01:53:24< celmin> Though I don't quite recall which of those corresponds to which...
20160714 01:53:56< celmin> Oh, yeah, I think const handler_list* corresponds to const_iterator - it's an iterator that cannot mutate what it points to.
20160714 01:54:08< vultraz> i guess I should change that to remove_handler..? or just comment it out
20160714 01:54:23< celmin> Then handler_list*const corresponds to const iterator - an iterator that can't be changed but can mutate what it points too.
20160714 01:54:31-!- Greg-Boggs [~greg_bogg@173.240.241.83] has joined #wesnoth-dev
20160714 01:54:35< celmin> Oh, right, there was a third use of delete_handler_index.
20160714 01:54:49< vultraz> yes
20160714 01:55:23< celmin> Hmm, looks like the goal there is to move the handler, rather than delete it.
20160714 01:55:43< vultraz> i can't really understand the code
20160714 01:55:45< celmin> add_handler always puts it at the end, right?
20160714 01:56:12< vultraz> also, isn't handlers[i] now invalid
20160714 01:56:18< celmin> Yes, absolutely.
20160714 01:56:32< vultraz> celmin: add_handler is push_back
20160714 01:56:43< celmin> Okay, lemme look up how splice works.
20160714 01:57:18< vultraz> hm
20160714 01:57:22< vultraz> how to replace handlers[i]..
20160714 01:57:33< vultraz> i is an int..
20160714 01:57:52< celmin> Hmm… splice is described as transferring elements from one list to another, so maybe it doesn't apply here.
20160714 01:58:02< vultraz> ok I can make it another iteratior look
20160714 01:58:02< celmin> vultraz: That depends on the situation.
20160714 01:58:06< vultraz> loop
20160714 01:58:12< celmin> But ultimately somehow you need to make it an iterator, yes.
20160714 01:58:23< celmin> for(auto iter = begin; iter != end(); ++iter)
20160714 01:58:35< celmin> If it's a loop, that's the basic form for working with iterators.
20160714 01:58:44< Aginor> can you guys please commit this to a branch so I can review it tonight? :)
20160714 01:58:51< vultraz> Aginor: obv
20160714 01:58:53-!- Greg-Boggs [~greg_bogg@173.240.241.83] has quit [Ping timeout: 258 seconds]
20160714 01:58:59< vultraz> celmin: ok, so if I make foc_i "const handler_list::iterator foc = event_contexts.back().focused_handler;"
20160714 01:59:00< vultraz> then..
20160714 01:59:15< celmin> Hmm. Maybe splice does apply...
20160714 01:59:25< celmin> Splice the specific element to the end of the list.
20160714 01:59:34< vultraz> can I just make foc_hand ..
20160714 01:59:40< vultraz> yeah no
20160714 01:59:44< vultraz> it's an iterator
20160714 01:59:49< vultraz> we want the handler
20160714 02:00:53< celmin> Where are foc_hand and hand even defined.
20160714 02:01:20< vultraz> right in has_focus
20160714 02:02:06< vultraz> if we have an iterator to a member of a lsit
20160714 02:02:08< vultraz> list
20160714 02:02:10< vultraz> how do we get that member
20160714 02:03:21< vultraz> sdl_handler *const foc_hand = event_contexts.back().(*foc)?
20160714 02:04:38< celmin> Oh I see, hand is a parameter.
20160714 02:05:01< vultraz> also why is it sdl_handler* const and not const first
20160714 02:05:15< celmin> Okay, so foc_i was the same as focused_handler.
20160714 02:05:30< celmin> So foc_hand ends up being the same as *focused_handler?
20160714 02:05:34< Aginor> careful with keeping copies of those objects around, they're pointers for a reason and should stay pointers/references
20160714 02:06:05< celmin> vultraz: sdl_handler* const is a pointer that cannot be reassigned, but const sdl_handler* is a pointer to an sdl_handler that cannot be changed.
20160714 02:06:35< vultraz> I see
20160714 02:07:21< celmin> Of couse you need to apply the event_contexts.back(). prefix 
20160714 02:07:52< vultraz> [13:03:20]	vultraz	sdl_handler *const foc_hand = event_contexts.back().(*foc)?
20160714 02:07:54< vultraz> is this valid
20160714 02:08:04< celmin> So I guess foc_i becomes e_c.back().focused_handler and foc_hand becomes *foc_i, but it's probably easier to just merge the too, eliminate foc_i and set foc_hand = e_c.back().focused_handler.
20160714 02:08:11< celmin> No.
20160714 02:08:16< celmin> That's not valid.
20160714 02:08:34< celmin> The * would move in front of even_contexts, but see the other thing I just said.
20160714 02:09:17< celmin> Hmm, though if you did that, foc_hand-> would need to become (*foc_hand)-> or (**foc_hand).
20160714 02:09:29< celmin> (The . is part of that, not a period.)
20160714 02:09:48< vultraz> except we use foc_i before foc_hand's assignment
20160714 02:10:07< celmin> I guess you can stick with foc_i being set to focused_handler.
20160714 02:10:19< celmin> Imagine the i stands for iterator, I guess.
20160714 02:10:35< celmin> Of course, you're comparing it to end() instead of -1 there.
20160714 02:10:50< celmin> Which is … event_contexts().handlers.end()?
20160714 02:10:51< vultraz> I renamed foc_i to foc
20160714 02:10:54< celmin> Sure.
20160714 02:11:35< vultraz> ok, so uh
20160714 02:11:37< vultraz> sdl_handler* const foc_hand = *event_contexts.back().foc; ?
20160714 02:11:44< vultraz> since we still need foc before this
20160714 02:11:47-!- aeth [~Michael@wesnoth/umc-dev/developer/aethaeryn] has quit [Ping timeout: 244 seconds]
20160714 02:12:15< celmin> ...
20160714 02:12:20< celmin> No, just *foc should do.
20160714 02:12:27< vultraz> oh
20160714 02:12:31< vultraz> deerrppp
20160714 02:12:35< celmin> :P
20160714 02:12:52< vultraz> wait, why not just get rid of foc_hand and do **foc
20160714 02:13:20< celmin> I think that might be similar to what I said above.
20160714 02:13:35-!- aeth [~Michael@wesnoth/umc-dev/developer/aethaeryn] has joined #wesnoth-dev
20160714 02:13:45< celmin> You can do that if you really want, but I dislike seeing stuff like (*foc)->blah
20160714 02:13:50< vultraz> eh, having it as its own pointer might be cleaner, code-wise
20160714 02:14:18< celmin> Yeah, look a couple lines down where you're accessing members of it - it's cleaner if it's only a single layer of indirection.
20160714 02:15:23< vultraz> doesn't this render if(foc_hand == hand) incorrect?
20160714 02:15:34< celmin> Hm? Why so?
20160714 02:15:39< vultraz> i think that needs to be moved up to just foc == hand
20160714 02:15:51< vultraz> eh... oh right, no, it's still valid
20160714 02:16:06< celmin> Well, one of those will compile and the other won't, so it should be obvious.
20160714 02:16:08< vultraz> since foc_hand is a pointer to a derefed pointer (foc)
20160714 02:16:29< celmin> Okay, so it seems that splice can operate on a single list, so I think that's the best thing to do in has_focus instead of the delete+add.
20160714 02:17:00< vultraz> how do I use spilce?
20160714 02:17:03< vultraz> splice
20160714 02:17:10< celmin> Maybe add a new function for it because otherwise it would look kinda ugly.
20160714 02:17:16< celmin> The basic idea I'm thinking is like this:
20160714 02:17:53< celmin> handlers.splice(handlers.end(), handlers, foc)
20160714 02:18:29< celmin> What that does is remove the third argument from the list which is the second argument and attaches it at the position specified by the first argument.
20160714 02:18:52< vultraz> hm, I see
20160714 02:19:26< celmin> It's more efficient than doing a remove+add because all it does is juggle pointers, whereas a remove+add deconstructs the list node and then constructs a new one.
20160714 02:19:30< vultraz> ok, so first we need to make that loop an iterator
20160714 02:19:36< vultraz> loop
20160714 02:19:53< celmin> The reason I said it would look ugly though is because handlers is actually event_contexts.back().handlers
20160714 02:20:28< celmin> Another advantage of splice is that the iterator remains valid, so there's no need to update focused_iterator.
20160714 02:20:49< celmin> Well, I guess maybe there's no need anyway, because it will have changed by the time we splice.
20160714 02:21:21< celmin> Still, hypothetically speaking. A splice doesn't invalidate any iterators, though it can make the iterators point to a different list than they originally did.
20160714 02:21:51< celmin> …hmm, I wonder if comparison is valid on bidirectional iterators.
20160714 02:21:51< vultraz> loop would be something like this? 
20160714 02:21:53< vultraz> for(handler_list::iterator i = event_contexts.back().handlers.back(), i != event_contexts.back().handlers.front(), --i) {
20160714 02:22:13< celmin> Close but not quite.
20160714 02:22:26< celmin> handlers.begin() and handlers.end(), respectively
20160714 02:22:33< celmin> (But still event_contexts.back().)
20160714 02:22:43< celmin> Also those commas should be semicolons.
20160714 02:23:30< celmin> As I suspected, comparison is not valid on bidirectional iterators. That requires a random-access iterator. Hmm.
20160714 02:23:56< celmin> Ah.
20160714 02:24:03< celmin> I think I see what you can do.
20160714 02:24:19< celmin> Use back_i as the sentinel instead of end()
20160714 02:24:37< celmin> auto back = e_c.back().h.end()
20160714 02:24:40< celmin> --back
20160714 02:25:06< celmin> And then for(auto iter = e_c.back().h.begin(); iter != back; ++iter)
20160714 02:25:41< celmin> Then if(foc_i < back_i) can be taken to be always true and thus removed, reducing the nesting by one level.
20160714 02:25:41< vultraz> why would you need to decrement end instead of using back()
20160714 02:25:59< celmin> Because back() returns the element, but you want the iterator to that element.
20160714 02:26:07< vultraz> ohhh
20160714 02:26:21< celmin> You could technically use back(), if you can guarantee that a given handler would never be inserted twice into the list.
20160714 02:26:34< celmin> But it's probably bad practice and won't work in general cases.
20160714 02:26:49< celmin> (It could work here since the list elements are pointers.)
20160714 02:27:03< vultraz> and don't you mean --iter?
20160714 02:27:10< vultraz> er
20160714 02:27:12< vultraz> wait
20160714 02:27:27< vultraz> celmin: it was written as a reverse-for list
20160714 02:27:29< vultraz> er
20160714 02:27:31< vultraz> loop
20160714 02:27:36< celmin> Oh, was it?
20160714 02:27:39< vultraz> yes
20160714 02:27:46< celmin> Ah, you're right, hm.
20160714 02:27:52< celmin> So I guess swap the sentinels.
20160714 02:27:54< vultraz> dso
20160714 02:27:56< vultraz> so
20160714 02:27:57< celmin> ie, begin() and back
20160714 02:28:02< celmin> (And replace ++ with --)
20160714 02:28:14< vultraz> 		auto back = event_contexts.back().handlers.end();
20160714 02:28:15< vultraz> 		--back;
20160714 02:28:17< vultraz> 		for(auto i = back; i != event_contexts.back().handlers.begin(); --i) {
20160714 02:28:18< vultraz> ?
20160714 02:28:19< celmin> Hmm.
20160714 02:28:57< celmin> That's basically what I was thinking, yes, but let's think about one thing first...
20160714 02:29:05< vultraz> (honestly, syntax-wise I find it weird you can't do auto back = --event_contexts.back().handlers.end();
20160714 02:29:26< celmin> That's because -- mutates its argument.
20160714 02:29:40< celmin> You can do end() - 1, but only if it's a random-access iterator, which this isn't.
20160714 02:29:48< vultraz> I see
20160714 02:30:10< celmin> I'm looking at the code again and trying to understand whether what I (we?) came up with is truly correct.
20160714 02:30:40< celmin> So it runs through all handlers in reverse order.
20160714 02:30:45< vultraz> yes
20160714 02:30:50< celmin> It skips the currently focused handler.
20160714 02:31:05< celmin> So not quite all handlers.
20160714 02:31:19< celmin> If it finds a handler that needs focus, it focuses it and exits the loop.
20160714 02:32:02< celmin> And it ensures the the formerly focused handler appears later than the newly focused handler.
20160714 02:32:08< celmin> By moving it to the end.
20160714 02:32:17< celmin> It would probably be sufficent to swap them, but whatever.
20160714 02:32:44< celmin> But it only moves it to the end if it's not already later.
20160714 02:32:54< vultraz> event_contexts.back().handlers.splice(event_contexts.back().handlers.end(), event_contexts.back().handlers, foc);
20160714 02:32:56< vultraz> wow
20160714 02:32:57< vultraz> long
20160714 02:33:22< celmin> You could make have a temporary to make it less long (would also apply to the first line of the loop).
20160714 02:33:30< celmin> auto& handlers = e_c.back(),h
20160714 02:33:37< celmin> Something like that.
20160714 02:33:38< vultraz> does typedef not work in this case?
20160714 02:33:39< celmin> . not ,
20160714 02:33:58< celmin> No, typedef is for types, this is a variable. You want a reference, like I just said.
20160714 02:34:02< vultraz> k
20160714 02:34:43< celmin> Hmm, there's a problem with this loop as written. It won't cover the first element of the list.
20160714 02:35:19< celmin> It's probably better to use a reverse iterator, actually.
20160714 02:35:36< celmin> ie for(auto iter = rbegin(); iter != rend(); ++iter)
20160714 02:35:45< celmin> But then how to compare...
20160714 02:35:59< vultraz> does that mean we don't need the back pointer?
20160714 02:36:18< celmin> Ah, it would be if(iter.base() == foc && …
20160714 02:36:24< celmin> I think so, yes.
20160714 02:36:38< celmin> I'm not entirely sure though.
20160714 02:36:47< vultraz> .base?
20160714 02:37:03< celmin> Yes, if you have a reverse iterator, then .base() returns the non-reversed version of that iterator.
20160714 02:37:15< celmin> Oh wait.
20160714 02:37:22< vultraz> for(auto i = handlers.rend(); i >= handlers.rbegin(); --i) {
20160714 02:37:24< vultraz> right?
20160714 02:37:29< celmin> That won't work after all, because the base points to a different element. Ugh.
20160714 02:37:30< vultraz> er
20160714 02:37:32< vultraz> wait
20160714 02:37:34< vultraz> LOL
20160714 02:37:36< vultraz> im an idiot
20160714 02:37:40< vultraz> doble negatives here
20160714 02:37:42< vultraz> double
20160714 02:37:44< vultraz> hang on
20160714 02:37:50< celmin> That's almost right, but you need ++
20160714 02:37:58< vultraz> for(auto i = handlers.rbegin(); i >= handlers.rend(); ++i) {
20160714 02:38:03< celmin> Oh right, swap … yeah like that.
20160714 02:38:09< vultraz> rbegin first?
20160714 02:38:18< celmin> You should use != rather than >=
20160714 02:38:23< vultraz> ah
20160714 02:38:23< celmin> Yeah.
20160714 02:38:32< celmin> Hmm.
20160714 02:38:34< vultraz> i had, but theold loop was >=
20160714 02:38:36< vultraz> will use != again
20160714 02:38:55< celmin> You can't compare bidirectional iterators with < <= > >=
20160714 02:38:58< celmin> Only with == !=
20160714 02:39:15< vultraz> celmin: what I'm looking at now http://pastebin.com/U68NRN2T
20160714 02:40:12< celmin> Yeah, pretty much, just need to figure out line 32.
20160714 02:42:33< celmin> Because with this, i != foc is (probably) not a valid comparison.
20160714 02:44:15< celmin> And i.base() points to a different element than i.
20160714 02:44:27< celmin> Um… I guess it points to the element after i.
20160714 02:44:49< celmin> Yeah, that seems right.
20160714 02:44:58< celmin> So you want to check if i and foc point to the same element.
20160714 02:45:14< vultraz> if (*i) == (*foc)?
20160714 02:45:37< celmin> Hmm. It would work in this situation, I guess, since they're pointers.
20160714 02:45:46< celmin> I'm guessing a handler doesn't get added twice anyway.
20160714 02:45:47< vultraz> actually, if(thief_hand == foc_hand
20160714 02:46:00< celmin> Don't forget it's supposed to be != rather than ==
20160714 02:46:08< vultraz> so instead of i != foc
20160714 02:46:11< vultraz> we use what I just said?
20160714 02:46:16< celmin> Sure, why not.
20160714 02:46:47< vultraz> ALRIGHT
20160714 02:46:50< vultraz> let's see if this builds
20160714 02:47:27< celmin> Hmm, well… I guess splice would probably be a no-op if the element being spliced in is already in the location it's being spliced to, so I guess that should be fine.
20160714 02:48:58< vultraz> hm
20160714 02:49:07< vultraz> just realized Imight have screwed something up
20160714 02:49:48< celmin> Quite possibly!
20160714 02:49:51< vultraz> hopefully simple fix here
20160714 02:50:11< vultraz> i accidentally changed another vector to a list in the header :P
20160714 02:50:39< celmin> Accidentally?
20160714 02:50:54< vultraz> blanket find/replace :|
20160714 02:52:06< vultraz> ok, ya know what, changng it back to a vector gives immediate compile errors. let's see how far it gets as a list
20160714 02:52:35< celmin> What's it called?
20160714 02:52:53< vultraz> typedef std::list<events::sdl_handler*> sdl_handler_vector;
20160714 02:53:12< vultraz> not to be confused with typedef std::list<sdl_handler*> handler_list;
20160714 02:53:20< celmin> ...
20160714 02:53:21< celmin> ...
20160714 02:53:22< celmin> ...
20160714 02:53:23< celmin> ...
20160714 02:53:49< celmin> That's not a vector, it's a typedef.
20160714 02:53:56< celmin> Obviously.
20160714 02:54:10< vultraz> it used to be a typedef of  vector
20160714 02:54:21< celmin> If I'm not mistaken, didn't you make another typedef like that?
20160714 02:54:32< Aginor> 14:52 < vultraz> typedef std::list<events::sdl_handler*> sdl_handler_vector;
20160714 02:54:46< vultraz> [13:53:11]	vultraz	not to be confused with typedef std::list<sdl_handler*> handler_list; this is what I just made.
20160714 02:54:51< Aginor> ^^^ I don't really like the name of that type :D
20160714 02:55:02< vultraz> Aginor: you wrote it, presumably :P
20160714 02:55:06< celmin> Yeah, just delete it and replace it with handler_list.
20160714 02:55:20< celmin> vultraz: He's saying he doesn't like the name because the name is "vector" when it's actually a list.
20160714 02:55:32< celmin> So just delete it and replace it everywhere with handler_list.
20160714 02:55:43< vultraz> Aginor, celmin: it used to be a vector. I accidentally changed it to a list.
20160714 02:55:51< vultraz> and it's in the events namespace
20160714 02:56:04< celmin> Even if you say accidentally, I'd guess it does need to be a list.
20160714 02:56:08< vultraz> whhereas my typedef is in the events::context scruct
20160714 02:56:15< vultraz> sctuct
20160714 02:56:23< vultraz> STRUCT 
20160714 02:56:31< celmin> Okay, so delete your typedef and rename the other one? Maybe change it to sdl_handler_list if you want.
20160714 02:56:39< vultraz> sdl_handler_vector is used in many files :|
20160714 02:57:32 * vultraz :| :| :|
20160714 02:57:46< celmin> Oh my.
20160714 02:58:02< vultraz> why does the sdl_handler class have it's OWN VIRTUAL VECTOR OF SDL_HANDLERS
20160714 02:58:22< vultraz> which was something else I accidentally made a list with find/replace..
20160714 02:58:30< celmin> Huhwhat.
20160714 02:58:33< vultraz> IDK!
20160714 02:58:51< vultraz> wait
20160714 02:58:53< vultraz> it's a function
20160714 02:58:54< celmin> But even if you say it was an accident, shouldn't it be made a list after all?
20160714 02:59:05< vultraz> that returns a vector/list of sdl_handlers
20160714 02:59:30< vultraz> ok, let's make this a vector again
20160714 03:03:15< celmin> Whatever.
20160714 03:03:52< vultraz> yay... many many errors
20160714 03:04:35< vultraz> oh, handlers.erase(handlers.back()) isn't right, apparenrtly
20160714 03:05:10< vultraz> uh...
20160714 03:05:12< vultraz> wait why
20160714 03:05:18< vultraz> oh
20160714 03:05:20< vultraz> end
20160714 03:05:26< celmin> No, never erase end()
20160714 03:05:34< celmin> You need one less than end()
20160714 03:05:51< vultraz> pop_back?
20160714 03:05:53< celmin> Which I think means an intermediate variable? One sec.
20160714 03:06:03< celmin> Oh wait, does it have pop_back()? That would be perfect.
20160714 03:06:11< vultraz> it does
20160714 03:06:17< celmin> Then do that.
20160714 03:07:21< celmin> What was the other one? handlers.erase(i)?
20160714 03:07:31< vultraz> hm?
20160714 03:07:41< celmin> A few lines down from that.
20160714 03:08:01< celmin> Well, it's probably fine, I guess.
20160714 03:08:09< celmin> As long as it's not erase(focused_handler)
20160714 03:08:43< celmin> Mind you, if it was, the proper way would be focused_handler = handlers.erase(focused_handler)
20160714 03:10:32< vultraz> hmmm
20160714 03:10:36< vultraz> what do I do in this situatio
20160714 03:10:37< vultraz> n
20160714 03:10:45< vultraz> 	const handler_list::const_iterator i = std::find(handlers.begin(),handlers.end(),ptr);
20160714 03:10:46< vultraz> 	if(i != handlers.end() && (**i).requires_event_focus()) {
20160714 03:10:48< vultraz> 		focused_handler = i;
20160714 03:10:49< vultraz> 	}
20160714 03:10:53< vultraz> the 3rd line here is invalid because i is const
20160714 03:11:02< vultraz> ... if I'm reading this correctly
20160714 03:11:11< celmin> Yeah, could be… hmm...
20160714 03:11:30-!- wedge010 [~Thunderbi@60-241-236-92.static.tpgi.com.au] has joined #wesnoth-dev
20160714 03:11:34< celmin> I think your only option is to not use const_iterator there but instead use iterator.
20160714 03:11:37< celmin> Wait no.
20160714 03:11:46< celmin> You could also make focused_handler a const_iterator.
20160714 03:11:48< celmin> One of those.
20160714 03:13:00< celmin> Maybe the second one would be better.
20160714 03:13:08< vultraz> trying that
20160714 03:13:22< celmin> Though it might need to propagate a bit - changing several other places to const_iterator, that is.
20160714 03:13:30< vultraz> yeah
20160714 03:13:38< vultraz> at least one in has_focus
20160714 03:13:55< vultraz> I have to remove the typedef there
20160714 03:13:57< vultraz> anyway
20160714 03:14:03< vultraz> since it's not part of context::
20160714 03:14:18-!- wedge009 [~Thunderbi@60-241-236-92.static.tpgi.com.au] has quit [Ping timeout: 246 seconds]
20160714 03:14:18-!- wedge010 is now known as wedge009
20160714 03:14:32< vultraz> ya know, it'll be pretty funny if after all this it doesn't fix the bug :P
20160714 03:14:55< celmin> What was the bug?
20160714 03:15:04< vultraz> crash in the editor when closing a menu
20160714 03:15:14< celmin> What kind of crash?
20160714 03:15:30< vultraz> "has stopped working correctly" kind of crash
20160714 03:15:41< vultraz> (segfault?)
20160714 03:15:43< celmin> In other words, you have no idea.
20160714 03:16:32< celmin> I have no idea if it would fix the bug and also no idea why you think it would fix the bug (but that's mainly because I have no idea what the bug even is)..
20160714 03:16:53< celmin> It may be a change that has value by itself, though?
20160714 03:20:16< vultraz> well Aginor suggested it
20160714 03:25:00< vultraz> hmm
20160714 03:25:02< vultraz> bunch of theeese
20160714 03:25:09< vultraz> events.cpp|426|error: no match for 'operator[]' (operand types are 'const handler_list {aka const std::__cxx11::list<events::sdl_handler*>}' and 'size_t {aka unsigned int}')|
20160714 03:25:13< vultraz> let us unvestigate
20160714 03:26:44< celmin> Yeah, we already established that handlers[i] won't work, right? This is more of that.
20160714 03:26:56< vultraz> ah, right
20160714 03:27:04< vultraz> ok this is a pretty common look
20160714 03:27:05< vultraz> loop
20160714 03:27:37< vultraz> so we just need to make it an iterator loop
20160714 03:28:03< vultraz> celmin: can range-forbe used here?
20160714 03:28:15< celmin> How am I supposed to know?
20160714 03:28:36< celmin> If it's a regular forward iteration and focused_handler is not involved, it's probably fine with range-for, though.
20160714 03:28:57< vultraz> it's just iterating over the handlers in a specific context
20160714 03:29:03< vultraz> from begin to end
20160714 03:39:16< vultraz> ok, looks like it's building now
20160714 03:44:17< vultraz> ok, it builds
20160714 03:46:35< vultraz> but doesn't run 
20160714 03:48:55< vultraz> going to open a PR for this so it's easier to look at, though
20160714 03:50:59< vultraz> Aginor, celticminstrel what I have so far https://github.com/wesnoth/wesnoth/pull/691
20160714 03:54:08-!- Greg-Boggs [~greg_bogg@173.240.241.83] has joined #wesnoth-dev
20160714 03:55:29< vultraz> segfaults in sld_handler::leave
20160714 03:55:33< vultraz> hmmm
20160714 03:57:10< vultraz> celmin: didn't you say there was some problem with remove_handler being called on 'this'?
20160714 03:58:54-!- Greg-Boggs [~greg_bogg@173.240.241.83] has quit [Ping timeout: 276 seconds]
20160714 03:59:10< celmin> The only problem with that is it makes it very hard to convert remove_handler to take an iterator. So not a problem that would create a bug.
20160714 04:00:01< vultraz> hm
20160714 04:00:11< vultraz> removing that line allows the game to hit the titlescreen but not much else
20160714 04:00:15< vultraz> the issue must be deeper
20160714 04:00:44< vultraz> but the stacktrace doesn't go deeper..
20160714 04:00:45< vultraz> hm
20160714 04:01:24< vultraz> ill let Aginor look at it later
20160714 04:01:41< vultraz> thanks for helping with this, btw
20160714 04:01:43< vultraz> celmin
20160714 04:02:26< celmin> Did you ever get around to looking through the comments on the stoi commit, by the way?
20160714 04:02:58< vultraz> yes
20160714 04:03:00< vultraz> I'll get to that
20160714 04:03:05< vultraz> i got sidetracked :P
20160714 04:03:41< celmin> That's fine.
20160714 04:03:55-!- molt [~molt@dynamic-213-198-235-143.adsl.eunet.rs] has joined #wesnoth-dev
20160714 04:03:58< celmin> …whoa, it's Thursday already?
20160714 04:04:06< celmin> Well okay, only barely, but still.
20160714 04:31:26-!- celmin [~celticmin@unaffiliated/celticminstrel] has quit [Quit: And lo! The minstrel departs, to spread the music to the masses!]
20160714 04:45:03< vultraz> celticminstrel: https://github.com/Vultraz/wesnoth/commit/13c19bb7fc24af464b429070bd89315dfbd4379d
20160714 04:47:44< Aginor> ok, I don't think we are in a shape to release tomorrow
20160714 04:48:02< celticminstrel> Ah... remind me to look at that tomorrow, okay?
20160714 04:48:08< Aginor> I would suggest we adjourn the release by a week to make sure we have addressed the stability issues sufficiently
20160714 04:48:41< celticminstrel> I have no idea what issues Aginor is talking about, but I agree we can't release if it's unstable.
20160714 04:49:42< Aginor> celticminstrel: what yo've been helping vultraz with over the day as it's to fix a crash
20160714 04:50:15< Aginor> and the lua stoi thing gave me an impression that it should be a blocker too
20160714 04:50:28< Aginor> as it breaks things for the user that we intend not to break
20160714 04:50:32< Aginor> I think ;)
20160714 04:50:43< Aginor> I haven't been following that well, I've been busy working
20160714 04:52:57< vultraz> Aginor: I hadn't intended to release on Friday exactly, but I agree
20160714 04:53:02< vultraz> I'll push it back till next week
20160714 04:53:10-!- vultraz changed the topic of #wesnoth-dev to: 1.13.5 expected next  week | Wesnoth Developers Channel | >>> Want to help? Go here: http://r.wesnoth.org/t42911 (and thanks!) <<< | Logs: http://irclogs.wesnoth.org | Bug tracker: http://bugs.wesnoth.org
20160714 04:53:23< vultraz> we can get the stoi issues fixed (which aren't exclusive to lua)
20160714 04:53:28< vultraz> and hopefully this crash I found
20160714 04:54:38< celticminstrel> Huh? Did it have something to do with Lua?
20160714 04:55:01< vultraz> erm
20160714 04:55:06< vultraz> dont think so, actually
20160714 04:58:34< vultraz> Aginor: let me know when you take a look at the events pr
20160714 04:58:41< vultraz> Aginor: id really like to know where we went wrong
20160714 05:03:30-!- Greg-Boggs [~greg_bogg@173.240.241.83] has joined #wesnoth-dev
20160714 05:07:22< Aginor> vultraz: I'll have a look in 3h or so
20160714 05:07:30< vultraz> sweet :)
20160714 05:07:41< Aginor> when I'm home and have had dinner, etc
20160714 05:08:10-!- Greg-Boggs [~greg_bogg@173.240.241.83] has quit [Ping timeout: 252 seconds]
20160714 05:27:31-!- mjs-de [~mjs-de@x4db50acd.dyn.telefonica.de] has joined #wesnoth-dev
20160714 05:32:36-!- Sapient [~yourstrul@wesnoth/developer/Sapient] has joined #wesnoth-dev
20160714 05:32:40-!- mjs-de [~mjs-de@x4db50acd.dyn.telefonica.de] has quit [Remote host closed the connection]
20160714 05:42:17-!- edgrey [~edgrey@178.204.47.40] has joined #wesnoth-dev
20160714 05:43:07-!- edgrey [~edgrey@178.204.47.40] has quit [Remote host closed the connection]
20160714 05:43:37-!- edgrey [~edgrey@178.204.47.40] has joined #wesnoth-dev
20160714 05:44:08-!- edgrey [~edgrey@178.204.47.40] has quit [Remote host closed the connection]
20160714 05:44:32-!- edgrey [~edgrey@178.204.47.40] has joined #wesnoth-dev
20160714 05:44:54-!- celticminstrel [~celmin@unaffiliated/celticminstrel] has quit [Quit: And lo! The computer falls into a deep sleep, to awake again some other day!]
20160714 06:24:26-!- bumbadadabum [~bumbadada@wesnoth/developer/bumbadadabum] has quit [Ping timeout: 244 seconds]
20160714 06:34:10-!- Kwandulin [~Miranda@p200300760F2D81DBD8B8A14613F03194.dip0.t-ipconnect.de] has joined #wesnoth-dev
20160714 06:35:49-!- Sapient [~yourstrul@wesnoth/developer/Sapient] has quit [Quit: Have fun, cya!]
20160714 06:49:44-!- zookeeper [~lmsnie@wesnoth/developer/zookeeper] has joined #wesnoth-dev
20160714 06:51:47-!- Greg-Boggs [~greg_bogg@173.240.241.83] has joined #wesnoth-dev
20160714 06:56:12-!- Greg-Boggs [~greg_bogg@173.240.241.83] has quit [Ping timeout: 260 seconds]
20160714 07:52:50< Aginor> 'lo again
20160714 07:53:09< vultraz> allos
20160714 07:53:24< Aginor> my 3h was a pretty good estimate :D
20160714 07:54:12< vultraz> indeed
20160714 08:02:04 * Aginor starts his IDE
20160714 08:02:34< vultraz> I'll probably be going out to dinner soon, but I should be around for another half hour or so yet.
20160714 08:02:41< Aginor> ok
20160714 08:04:10< Aginor> I need to buy a ssd
20160714 08:04:35< Aginor> having an SSD at work is spoiling me completely
20160714 08:06:10< vultraz> dont we all :P
20160714 08:06:48< Aginor> I could do with an SSD, one of those i7's I linked to earlier and some 32 gigs of ram
20160714 08:07:00< Aginor> I need to do more consulting
20160714 08:07:03< Aginor> :D
20160714 08:07:11< Aginor> to add more to the toy budget
20160714 08:07:57< Aginor> I bought new monitors last weekend, it's glorious
20160714 08:08:07< Aginor> the old ones are ~8 years
20160714 08:08:28< Aginor> new ones was pretty nice
20160714 08:08:40< vultraz> mine is older too
20160714 08:08:45< vultraz> probably ~2009
20160714 08:09:08< Aginor> I'd give you my old ones if you'd want them :D
20160714 08:09:12< Aginor> and if you were closer
20160714 08:09:20< Aginor> but frankly, they're not worth much
20160714 08:09:34< vultraz> I have a 20" HP S2031
20160714 08:10:14< Aginor> ok
20160714 08:10:32< Aginor> the old ones are a couple of 22" acer, 1680x1050
20160714 08:10:38< vultraz> actually, I should see what year that is..
20160714 08:11:19< vultraz> first available on amazon in may 2010
20160714 08:13:23< Aginor> also old
20160714 08:13:33< Aginor> you're due for a replacement soon I reckon :D
20160714 08:14:06< vultraz> yeah
20160714 08:14:13< vultraz> maybe this black friday
20160714 08:14:36-!- ChipmunkV [~vova@d0017-2-88-172-31-68.fbx.proxad.net] has joined #wesnoth-dev
20160714 08:16:45< Aginor> hmm
20160714 08:16:57< Aginor> I found your crash
20160714 08:17:06< Aginor> as in, encountered it ;)
20160714 08:18:29< vultraz> without my patch?
20160714 08:18:47< Aginor> no, I'm looking at your branch now
20160714 08:18:49< Aginor> running the code
20160714 08:20:12< Aginor> you never intialise focused_handler to a value, I wonder if that's a problem
20160714 08:20:46 * Aginor ponders
20160714 08:20:50< vultraz> what could it be initialized as?
20160714 08:21:03< Aginor> .back()?
20160714 08:21:17< Aginor> if that's what you're using as an empty value
20160714 08:21:26< Aginor> s/back()/end()/
20160714 08:22:37< Aginor> hmm
20160714 08:22:58< Aginor> let's see if this stops crashing
20160714 08:23:20< Aginor> but you have an iterator that's never been initialised that you're then dereferencing
20160714 08:23:35< Aginor> that's my prime suspect for the crash
20160714 08:24:08< Aginor> changing events.hpp is terrible though :/
20160714 08:24:18< vultraz> ill give it a try
20160714 08:24:25< vultraz> building now
20160714 08:24:30< Aginor> give me a chance, I'm building now :)
20160714 08:24:55< Aginor> I changed events.cpp to check the value of focued_handler before dereferencing it
20160714 08:26:47< Aginor> if(focused_handler != handlers.end() && *focused_handler == ptr) {
20160714 08:27:00< Aginor> context() :
20160714 08:27:01< Aginor> 		handlers(),
20160714 08:27:01< Aginor> 		focused_handler(handlers.end())
20160714 08:27:01< Aginor> 	{
20160714 08:27:02< Aginor> 	}
20160714 08:27:34< Aginor> still crashes though
20160714 08:27:46< Aginor> ah, different palce
20160714 08:29:40< Aginor> the implementation of cycle_focus() looks a bit odd
20160714 08:29:58< vultraz> why is it needed, again?
20160714 08:30:45< Aginor> as I said, I'm not sure it does
20160714 08:31:33< Aginor> in fact, I don't think any class is implementing it
20160714 08:32:32< Aginor> no, I'm wrong
20160714 08:32:36< Aginor> GUI1 relies on it
20160714 08:32:43< Aginor> menu, slider and textbox
20160714 08:34:48< Aginor> so what happens when you increment an iterator beyond end()?
20160714 08:34:55< Aginor> is it still the same as end()?
20160714 08:35:29< vultraz> I don't know
20160714 08:36:15< Aginor> http://stackoverflow.com/questions/1057724/what-if-i-increment-an-iterator-by-2-when-it-points-onto-the-last-element-of-a-v
20160714 08:36:19< Aginor> bad things
20160714 08:40:05-!- Greg-Boggs [~greg_bogg@173.240.241.83] has joined #wesnoth-dev
20160714 08:41:56< Aginor> vultraz: I'm sorry, I didn't anticipate there to be a focus mess exploding in your face when I suggested the change earlier
20160714 08:42:35< vultraz> well, if I hadn't undertaken this it would have blown up in yours :P
20160714 08:42:47< vultraz> plus it's good practice for me
20160714 08:43:00< vultraz> iterators have always been one of my weak spots
20160714 08:43:47< vultraz> I just hope it's not too complicated to get this working
20160714 08:44:19-!- Greg-Boggs [~greg_bogg@173.240.241.83] has quit [Ping timeout: 252 seconds]
20160714 08:48:48< vultraz> for the record, I too thought it would just be a simple vector -> list substitution when I started 
20160714 08:57:46< vultraz> hm
20160714 08:58:18< vultraz> thought this line: for(++temp_focused_handler; temp_focused_handler != focused_handler; ++temp_focused_handler) { might be causing problems if temp is already at end, but that doesn't fix the crash
20160714 09:01:10< vultraz> blah
20160714 09:01:49< Aginor> yeah
20160714 09:01:57< Aginor> I'm rewriting that function
20160714 09:02:06< vultraz> ahhh
20160714 09:02:06< Aginor> the logic in it is wrong
20160714 09:02:12< vultraz> I've tracked the crash to cycle_focus
20160714 09:02:19< vultraz> comment out that call in remove_handler and stuff works
20160714 09:02:22< Aginor> so have I
20160714 09:02:48< Aginor> I think that forcing it into a for-loop semantic is making things harder than it should be
20160714 09:02:50< vultraz> and it fixed the crash I found in the editor :D
20160714 09:03:23< vultraz> did *not* fix some of the icon overlays disappearing in-game, though
20160714 09:03:32< vultraz> (happens after opening a menu)
20160714 09:03:37< vultraz> also
20160714 09:03:46< vultraz> the game crashes on exit. heh
20160714 09:04:07< vultraz> anyway, I'll see what you come up with
20160714 09:04:28< vultraz> anyway
20160714 09:04:50< vultraz> the fact that the original crash that prompted this is fixed with these changes proves they're worthwhile
20160714 09:06:15< Aginor> it will save us from crashing in the future too
20160714 09:07:51< vultraz> :)
20160714 09:13:07-!- Kwandulin [~Miranda@p200300760F2D81DBD8B8A14613F03194.dip0.t-ipconnect.de] has quit [Read error: Connection reset by peer]
20160714 09:13:09< Soliton> vultraz: if you try to convert something and it fails it is not useful to print the not-properly-converted value in the error message. (re https://github.com/Vultraz/wesnoth/commit/13c19bb7fc24af464b429070bd89315dfbd4379d)
20160714 09:13:27< Soliton> instead print whatever you tried to convert from.
20160714 09:13:38< vultraz> oh
20160714 09:13:41< vultraz> that was my intention
20160714 09:14:08< vultraz> but how do I get the value pre-conversion without saving a copy?
20160714 09:14:36< Soliton> well, if you have f.e. "time = std::stoi(sub_items.back());" then "sub_items.back()" is what you want to print.
20160714 09:15:30< Soliton> even worse are the cases where you have several conversions in one try/catch so you don't know which values are valid in case of an exception.
20160714 09:16:09< Soliton> in those cases you should explicitely assign default values or whatever if you need to continue despite the exception.
20160714 09:18:54< Soliton> for the error messages i also suggest putting quotes or something around the problematic value since it looks odd if it says f.e.: "invalid time value: " (better: "invalid time value: ''")
20160714 09:28:58-!- ideuler [~textual@0.213.62.94.rev.vodafone.pt] has joined #wesnoth-dev
20160714 09:35:13-!- RatArmy [~RatArmy@om126211116174.13.openmobile.ne.jp] has joined #wesnoth-dev
20160714 09:41:47< Aginor> bah, for some reason the focus handler has a stuffed up alue in cycle_focus
20160714 09:41:54< Aginor> and I cannot find what causes it
20160714 09:52:41-!- Kwandulin [~Miranda@p200300760F2D81DB64D768219022C061.dip0.t-ipconnect.de] has joined #wesnoth-dev
20160714 09:54:37-!- JyrkiVesterinen [~JyrkiVest@87-92-50-49.bb.dnainternet.fi] has joined #wesnoth-dev
20160714 09:55:13-!- midzer_ is now known as midzer
20160714 10:22:08< vultraz> Aginor: hm?
20160714 10:24:21< Aginor> oooh, fedora 24 comes with wesnoth 1.12.6
20160714 10:24:39< vultraz> wow
20160714 10:25:30< Aginor> trying to run valgrind on it now
20160714 10:28:24-!- Greg-Boggs [~greg_bogg@173.240.241.83] has joined #wesnoth-dev
20160714 10:30:22-!- hk238 [~kvirc@t224.ip7.netikka.fi] has joined #wesnoth-dev
20160714 10:31:09< Aginor> vultraz: focus_handler contains a rubbish value when it enters cycle_focus, causing everything to break horribly
20160714 10:32:40-!- Greg-Boggs [~greg_bogg@173.240.241.83] has quit [Ping timeout: 252 seconds]
20160714 10:36:52< vultraz> eh?
20160714 10:37:07< vultraz> what is this rubbish value
20160714 10:40:08< Aginor> hmm
20160714 10:40:22< Aginor> if I knew that, it'd be easy to fix
20160714 10:40:32-!- stikonas [~gentoo@wesnoth/translator/stikonas] has joined #wesnoth-dev
20160714 10:40:34< Aginor> ok, have a good implementation of cycle_focus
20160714 10:40:41< vultraz> pastebin?
20160714 10:41:24< Aginor> soon
20160714 10:41:29< Aginor> :D
20160714 10:41:33-!- molt [~molt@dynamic-213-198-235-143.adsl.eunet.rs] has quit [Quit: Leaving]
20160714 10:41:45< Aginor> will uncomment a bunch of other lines now to see when it breaks
20160714 10:44:52< vultraz> see if it crashes when you quit the game
20160714 10:45:41< Aginor> it's not
20160714 10:45:59< Aginor> hmm
20160714 10:46:10< Aginor> the constructor isn't doing what I'm expecting
20160714 10:46:59< Aginor> http://pastebin.com/Y93pmhiZ
20160714 10:47:03< Aginor> warts and all
20160714 10:47:43< Aginor> restoring this to the original makes it crash again
20160714 10:47:43< Aginor> event_context::event_context()
20160714 10:47:44< Aginor> { event_contexts.push_back(context()); event_contexts.back().focused_handler = event_contexts.back().handlers.end();
20160714 10:47:45< vultraz> tís hard to read
20160714 10:47:47< Aginor> }
20160714 10:47:54< Aginor> vultraz: apply it ;)
20160714 10:48:08< Aginor> vultraz: it's all my changes
20160714 10:50:01< vultraz> gah
20160714 10:50:04< vultraz> doesn't apply
20160714 10:50:38-!- Kwandulin [~Miranda@p200300760F2D81DB64D768219022C061.dip0.t-ipconnect.de] has quit [Ping timeout: 272 seconds]
20160714 10:52:54< Aginor> git stash && patch < file :D
20160714 10:54:50< vultraz> bah
20160714 10:54:52< vultraz> i always forget about patch
20160714 10:56:12< Aginor> anyhow, I've tidied it up somewhat now
20160714 10:56:18< Aginor> less debug junk
20160714 10:56:23< vultraz> lot of #if 0
20160714 10:56:39< Aginor> temporarily removing junk
20160714 10:56:59< vultraz> so does it work?
20160714 10:57:11< Aginor> yes
20160714 10:57:16< Aginor> testing it more now
20160714 10:58:30< Aginor> hmm
20160714 10:58:33< Aginor> now it crashed :D
20160714 10:58:36< vultraz> D:
20160714 10:58:38< vultraz> doing what?
20160714 10:59:20< Aginor> leave_global
20160714 11:00:00< vultraz> well, that works on a vector
20160714 11:00:02< Aginor> which may have been deleted already
20160714 11:00:24< vultraz> unless you mean the removal_handler call
20160714 11:01:01< Aginor> something fishy is happening
20160714 11:01:14-!- ideuler [~textual@0.213.62.94.rev.vodafone.pt] has quit [Quit: My Mac has gone to sleep. ZZZzzz…]
20160714 11:01:23< vultraz> why does that not surprise me :P
20160714 11:02:30< Aginor> ok, it's not been deleted
20160714 11:02:38< Aginor> the debugger is showing me some other junk
20160714 11:05:32< Aginor> same problem, focused_handlers() isn't initialised properly
20160714 11:05:51< vultraz> but it's in the constructor 
20160714 11:06:02< Aginor> yes, it's confusing me
20160714 11:06:13< Aginor> I wonder if it's being initialised in the wrong order
20160714 11:06:28< vultraz> unless the constructor isn't being called?
20160714 11:06:34< vultraz> er
20160714 11:06:37< vultraz> let me rephrase that
20160714 11:06:54< Aginor> I'm not sure how the ordering will work on a struct
20160714 11:07:04< vultraz> is context initialized when leave_global is called?
20160714 11:07:15-!- Kwandulin [~Miranda@p200300760F2D81DB64D768219022C061.dip0.t-ipconnect.de] has joined #wesnoth-dev
20160714 11:07:18< Aginor> no, it shouldn't be
20160714 11:07:31< vultraz> well...
20160714 11:07:45< vultraz> then wouldn't that mean focused_handler isn't either?
20160714 11:08:03< Aginor> why should leave_global initialise anything?
20160714 11:08:30< vultraz> by when i meant at the time of
20160714 11:08:36< Aginor> oh, right
20160714 11:08:46< Aginor> great question
20160714 11:08:52< Aginor> I suspect the answer is "no"
20160714 11:09:22-!- RatArmy [~RatArmy@om126211116174.13.openmobile.ne.jp] has quit [Ping timeout: 260 seconds]
20160714 11:09:46< vultraz> well that would mean focused_handler isn't either
20160714 11:09:48< vultraz> right?
20160714 11:10:12< Aginor> which is why I changed:
20160714 11:10:16< Aginor> event_context::event_context()
20160714 11:10:16< Aginor> { event_contexts.push_back(context()); event_contexts.back().focused_handler = event_contexts.back().handlers.end();
20160714 11:10:19< Aginor> }
20160714 11:11:45< Aginor> ah, stopped crashing
20160714 11:11:53< Aginor> I tidied away a bit too much
20160714 11:12:27< vultraz> ah :D
20160714 11:12:29< vultraz> so it works?
20160714 11:12:40< Aginor> the jury is still compiling
20160714 11:12:50< Aginor> but generally, yes
20160714 11:13:37< vultraz> you can put your changes in a commit and fromat-patch it and I'll add it to the pr.
20160714 11:13:50< vultraz> wouldn't want to steal credit for your work
20160714 11:15:21< Aginor> http://pastebin.com/tTgD47RK
20160714 11:15:30< Aginor> hmm
20160714 11:15:38< Aginor> or just take it
20160714 11:15:40< Aginor> :D
20160714 11:15:46< Aginor> I want to go to bed
20160714 11:16:18< Aginor> this is annoying me though, what I introduce on line 158 shouldn't be needed
20160714 11:16:58< vultraz> no need to finish work on this tonight 
20160714 11:17:03< vultraz> we're not rushing for a release 
20160714 11:17:06-!- molt [~molt@dynamic-213-198-235-143.adsl.eunet.rs] has joined #wesnoth-dev
20160714 11:17:26< Aginor> tell you what
20160714 11:17:42< Aginor> I'll commit and push that branch onto the wesnoth repo
20160714 11:17:51< Aginor> then you can poke it there too
20160714 11:17:57< vultraz> alright
20160714 11:18:16< vultraz> sounds good :)
20160714 11:20:29-!- irker416 [~irker@uruz.ai0867.net] has joined #wesnoth-dev
20160714 11:20:29< irker416> wesnoth: Charles Dang wesnoth:Vultraz-event_handling_fixes 61ccf2fc1623 / src/ (events.cpp events.hpp): WIP: some refactoring of event handling w/ celticminstrel https://github.com/wesnoth/wesnoth/commit/61ccf2fc1623d4effea9360ef5e8940faf1db387
20160714 11:20:30< irker416> wesnoth: Andreas Löf wesnoth:Vultraz-event_handling_fixes fb02c3442f4c / src/ (events.cpp events.hpp): Merge branch 'event_handling_fixes' of https://github.com/Vultraz/wesnoth into V https://github.com/wesnoth/wesnoth/commit/fb02c3442f4cb61f228430720f27a76ba8b25344
20160714 11:20:31< irker416> wesnoth: Andreas Löf wesnoth:Vultraz-event_handling_fixes 90b6b58aa43e / src/ (events.cpp events.hpp): Fix crashes in the focus cycling code. https://github.com/wesnoth/wesnoth/commit/90b6b58aa43ea58dc0b4cb3b6f45b01e6ffc1a8a
20160714 11:21:34< Aginor> vultraz: if you look at it and figure out why it crashes without line 158, please do tell me
20160714 11:21:45< Aginor> it's a mystery that I don't understand at the moment
20160714 11:21:49< vultraz> will do 
20160714 11:22:41< Aginor> I shall however go to bed now
20160714 11:22:56< vultraz> ill give it a look since im two hours earlier :D
20160714 11:23:02< vultraz> thanks for the help
20160714 11:23:04< Aginor> enjoy :D
20160714 11:23:06< Aginor> no worries
20160714 11:23:22< Aginor> thanks for doing most of the work
20160714 11:23:50< vultraz> and celmin, of course.
20160714 11:23:52< vultraz> team efforts
20160714 11:24:03< Aginor> indeed
20160714 11:24:11 * Aginor fistbumps celticminstrel
20160714 11:24:29< Aginor> and now
20160714 11:24:31< Aginor> good night
20160714 11:29:48-!- edgrey [~edgrey@178.204.47.40] has quit [Quit: Konversation terminated!]
20160714 11:30:25-!- edgrey [~edgrey@178.204.47.40] has joined #wesnoth-dev
20160714 11:38:05-!- edgrey [~edgrey@178.204.47.40] has quit [Ping timeout: 250 seconds]
20160714 11:41:30-!- travis-ci [~travis-ci@ec2-54-158-87-153.compute-1.amazonaws.com] has joined #wesnoth-dev
20160714 11:41:31< travis-ci> wesnoth/wesnoth#9777 (Vultraz-event_handling_fixes - 90b6b58 : Andreas Löf): The build failed.
20160714 11:41:31< travis-ci> Build details : https://travis-ci.org/wesnoth/wesnoth/builds/144711922
20160714 11:41:31-!- travis-ci [~travis-ci@ec2-54-158-87-153.compute-1.amazonaws.com] has left #wesnoth-dev []
20160714 12:16:41-!- Greg-Boggs [~greg_bogg@173.240.241.83] has joined #wesnoth-dev
20160714 12:17:28-!- edgrey [~edgrey@178.204.232.6] has joined #wesnoth-dev
20160714 12:21:01-!- Greg-Boggs [~greg_bogg@173.240.241.83] has quit [Ping timeout: 252 seconds]
20160714 12:23:09-!- celticminstrel [~celmin@unaffiliated/celticminstrel] has joined #wesnoth-dev
20160714 12:23:29-!- Kwandulin [~Miranda@p200300760F2D81DB64D768219022C061.dip0.t-ipconnect.de] has quit [Read error: Connection reset by peer]
20160714 12:39:53-!- Duthlet [~Duthlet@dslb-188-106-029-177.188.106.pools.vodafone-ip.de] has joined #wesnoth-dev
20160714 13:03:39< celticminstrel> vultraz: I responded to your comments on the commit, but I haven't looked over the revised commit yet.
20160714 13:03:55< celticminstrel> Oh, you made it a PR, convenient.
20160714 13:19:16< irker416> wesnoth: Celtic Minstrel wesnoth:master 4b513f6a5953 / changelog: Update changelog https://github.com/wesnoth/wesnoth/commit/4b513f6a595370cfcf1ab03002ec43eb449613d3
20160714 13:21:55-!- ideuler [~textual@0.213.62.94.rev.vodafone.pt] has joined #wesnoth-dev
20160714 13:27:41< celticminstrel> Did I update that page to include the Info.plist in the list of files that contain version numbers...?
20160714 13:43:24< celticminstrel> I wonder, is this an issue that existed in 1.13.4 or was it introduced and fixed after the relese?
20160714 13:43:26< celticminstrel> https://github.com/wesnoth/wesnoth/commit/308cd8b34f02ef2397a5bcc881b86eec2640130f
20160714 13:43:28< celticminstrel> ^release
20160714 13:49:14-!- prkc [~prkc@46.166.138.150] has quit [Ping timeout: 272 seconds]
20160714 13:50:07-!- molt [~molt@dynamic-213-198-235-143.adsl.eunet.rs] has quit [Quit: Leaving]
20160714 13:53:57-!- travis-ci [~travis-ci@ec2-107-22-115-196.compute-1.amazonaws.com] has joined #wesnoth-dev
20160714 13:53:58< travis-ci> wesnoth/wesnoth#9778 (master - 4b513f6 : Celtic Minstrel): The build passed.
20160714 13:53:58< travis-ci> Build details : https://travis-ci.org/wesnoth/wesnoth/builds/144736756
20160714 13:53:58-!- travis-ci [~travis-ci@ec2-107-22-115-196.compute-1.amazonaws.com] has left #wesnoth-dev []
20160714 14:02:33-!- ideuler [~textual@0.213.62.94.rev.vodafone.pt] has quit [Quit: My Mac has gone to sleep. ZZZzzz…]
20160714 14:03:13-!- ideuler [~textual@0.213.62.94.rev.vodafone.pt] has joined #wesnoth-dev
20160714 14:03:46-!- prkc [~prkc@catv-80-98-46-199.catv.broadband.hu] has joined #wesnoth-dev
20160714 14:21:08-!- Kwandulin [~Miranda@p200300760F2D81DBE12AF8F2EE8B5C94.dip0.t-ipconnect.de] has joined #wesnoth-dev
20160714 14:40:04< celticminstrel> I wonder if Anura's FFL properly made exponentiation right-associative.
20160714 15:05:11-!- ideuler [~textual@0.213.62.94.rev.vodafone.pt] has quit [Quit: My Mac has gone to sleep. ZZZzzz…]
20160714 15:08:24-!- JyrkiVesterinen [~JyrkiVest@87-92-50-49.bb.dnainternet.fi] has quit [Quit: .]
20160714 15:08:52-!- edgrey [~edgrey@178.204.232.6] has quit [Read error: Connection reset by peer]
20160714 15:11:29-!- wedge010 [~Thunderbi@60-241-236-92.static.tpgi.com.au] has joined #wesnoth-dev
20160714 15:12:45-!- edgrey [~edgrey@178.204.232.6] has joined #wesnoth-dev
20160714 15:14:54-!- wedge009 [~Thunderbi@60-241-236-92.static.tpgi.com.au] has quit [Ping timeout: 276 seconds]
20160714 15:14:55-!- wedge010 is now known as wedge009
20160714 15:15:49-!- ideuler [~textual@0.213.62.94.rev.vodafone.pt] has joined #wesnoth-dev
20160714 15:21:08-!- ideuler [~textual@0.213.62.94.rev.vodafone.pt] has quit [Quit: Chakalaka.]
20160714 15:36:30-!- Kwandulin [~Miranda@p200300760F2D81DBE12AF8F2EE8B5C94.dip0.t-ipconnect.de] has quit [Read error: Connection reset by peer]
20160714 15:39:19-!- molt [~molt@dynamic-213-198-235-143.adsl.eunet.rs] has joined #wesnoth-dev
20160714 15:57:06-!- JyrkiVesterinen [~JyrkiVest@87-92-50-49.bb.dnainternet.fi] has joined #wesnoth-dev
20160714 16:06:16< celticminstrel> I haven't seen mattsc for awhile, but maybe he'll see this... I'm finally updating the wiki on the new AI changes and would like feedback.
20160714 16:08:11-!- Greg-Boggs [~greg_bogg@173.240.241.83] has joined #wesnoth-dev
20160714 16:19:18-!- irker416 [~irker@uruz.ai0867.net] has quit [Quit: transmission timeout]
20160714 16:25:06-!- esr [~esr@wesnoth/developer/esr] has quit [Quit: WeeChat 1.3]
20160714 16:27:30-!- esr [~esr@static-71-162-243-5.phlapa.fios.verizon.net] has joined #wesnoth-dev
20160714 16:27:30-!- esr [~esr@static-71-162-243-5.phlapa.fios.verizon.net] has quit [Changing host]
20160714 16:27:30-!- esr [~esr@wesnoth/developer/esr] has joined #wesnoth-dev
20160714 17:00:32-!- prkc [~prkc@catv-80-98-46-199.catv.broadband.hu] has quit [Ping timeout: 260 seconds]
20160714 17:05:09-!- Kwandulin [~Miranda@p200300760F2D81DB540DCE5EAA597C02.dip0.t-ipconnect.de] has joined #wesnoth-dev
20160714 17:11:14-!- Greg-Boggs [~greg_bogg@173.240.241.83] has quit [Remote host closed the connection]
20160714 17:12:26-!- prkc [~prkc@46.166.137.225] has joined #wesnoth-dev
20160714 17:42:39-!- Greg-Boggs [~greg_bogg@173.240.241.83] has joined #wesnoth-dev
20160714 17:58:17-!- Greg-Boggs [~greg_bogg@173.240.241.83] has quit [Remote host closed the connection]
20160714 17:59:22-!- Duthlet [~Duthlet@dslb-188-106-029-177.188.106.pools.vodafone-ip.de] has quit [Quit: leaving]
20160714 18:13:50-!- lipkab [~the_new_l@host-91-147-210-58.biatv.hu] has joined #wesnoth-dev
20160714 18:17:12-!- lipkab [~the_new_l@host-91-147-210-58.biatv.hu] has quit [Client Quit]
20160714 18:22:55-!- stikonas [~gentoo@wesnoth/translator/stikonas] has quit [Quit: Konversation terminated!]
20160714 18:23:45-!- Greg-Boggs [~greg_bogg@173.240.241.83] has joined #wesnoth-dev
20160714 18:28:03< vultraz> celticminstrel: ok, thanks
20160714 18:28:51< celticminstrel> Sorry vultraz, what exactly are you thanking me for?
20160714 18:29:05< vultraz> looking over the pr
20160714 18:29:43< celticminstrel> I haven't looked over the events one yet.
20160714 18:29:46< vultraz> i should probably close the event one, though, since it's now in a branch in the main repo....
20160714 18:30:11< celticminstrel> I responded to your responses on the original commit.
20160714 18:30:21< celticminstrel> Or wait, that was the stoi one?
20160714 18:30:31< celticminstrel> Once of them.
20160714 18:30:47< celticminstrel> ...which one was I supposed to look over, again?
20160714 18:30:52< celticminstrel> Or was I supposed to look over both?
20160714 18:31:11< vultraz> you replied to comments on both
20160714 18:31:30< vultraz> but I should close the event handling one in favor of the branch in the main repo Aginor pushed
20160714 18:32:24-!- prkc [~prkc@46.166.137.225] has quit [Quit: Leaving]
20160714 18:32:52-!- prkc [~prkc@catv-80-98-46-199.catv.broadband.hu] has joined #wesnoth-dev
20160714 18:33:02< celticminstrel> I'm guessing his is based on yours?
20160714 18:33:42-!- Greg-Boggs [~greg_bogg@173.240.241.83] has quit [Remote host closed the connection]
20160714 18:33:48< vultraz> yes
20160714 18:33:55< vultraz> he pushed some fixes
20160714 18:34:09< vultraz> https://github.com/wesnoth/wesnoth/commit/90b6b58aa43ea58dc0b4cb3b6f45b01e6ffc1a8a
20160714 18:34:16-!- Greg-Boggs [~greg_bogg@173.240.241.83] has joined #wesnoth-dev
20160714 18:34:17< vultraz> but we're still puzzling over why line 158 is needed
20160714 18:34:59< vultraz> if focused_handler is set to handler.end in the context struct constructor, why does it need to be done again explicitly or it crashes
20160714 18:37:24< celticminstrel> Wait, what?
20160714 18:37:28-!- ancestral [~ancestral@63.92.240.233] has joined #wesnoth-dev
20160714 18:38:54< celticminstrel> Um, where else is it set to end()? I don't see it.
20160714 18:39:26< vultraz> 	context() :
20160714 18:39:28< vultraz> 		handlers(),
20160714 18:39:29< vultraz> 		focused_handler(handlers.end())
20160714 18:39:31< vultraz> 	{
20160714 18:39:32< vultraz> 	}
20160714 18:40:19< celticminstrel> Oh, in the header.
20160714 18:41:32-!- Kwandulin [~Miranda@p200300760F2D81DB540DCE5EAA597C02.dip0.t-ipconnect.de] has quit [Read error: Connection reset by peer]
20160714 18:42:29< celticminstrel> vultraz: Does it work if you remove line 158 and replace push_back with emplace_back?
20160714 18:42:45< celticminstrel> Maybe the problem is related to copying.
20160714 18:43:11< celticminstrel> Speaking of emplace, I was also supposed to do that in the spirit_po branch...
20160714 18:43:53< vultraz> it does not :(
20160714 18:44:30< celticminstrel> What if you call emplace_back() with no arguments?
20160714 18:45:13< vultraz> so just event_contexts.emplace_back()?
20160714 18:45:31< celticminstrel> Yes.
20160714 18:46:32< vultraz> it seems to, yes
20160714 18:47:06< vultraz> barring the crash on exit, which existed earlier anyway.
20160714 18:47:28-!- irker214 [~irker@uruz.ai0867.net] has joined #wesnoth-dev
20160714 18:47:28< irker214> wesnoth: Celtic Minstrel wesnoth:spirit_po 5ce3e28f3738 / src/gettext_boost.cpp: Try to construct catalogs in-place for less copies https://github.com/wesnoth/wesnoth/commit/5ce3e28f37381435fd1a7d87098f64dbe634dc0c
20160714 18:47:33< celticminstrel> In that case, I'd recommend adding this to the context definition:
20160714 18:47:46< celticminstrel> context(const context&) = delete;
20160714 18:48:09< celticminstrel> (Make it officially non-copyable, in other words.)
20160714 18:48:24< vultraz> line 41 right
20160714 18:48:51< vultraz> or inside the {}
20160714 18:49:17< celticminstrel> Around there, yes. It's like adding a new constructor, so not in the {}. (I'd actually put it at line 46, personally.)
20160714 18:49:32< vultraz> ohh
20160714 18:49:49< vultraz> does it need to be explicit
20160714 18:49:52< celticminstrel> But instead of defining it you're deleting it.
20160714 18:49:58< celticminstrel> It doesn't need to be explicit.
20160714 18:50:07< celticminstrel> Copy constructors aren't usually explicit.
20160714 18:50:50< vultraz> ok, building now
20160714 18:51:11< vultraz> wonder if this will have any effect on the remaining crash...
20160714 18:51:23< celticminstrel> If it does, then it'll turn the crash into a compile error.
20160714 18:51:30< celticminstrel> Probably.
20160714 18:51:33-!- Greg-Boggs [~greg_bogg@173.240.241.83] has quit [Remote host closed the connection]
20160714 18:52:45-!- hk238 [~kvirc@t224.ip7.netikka.fi] has quit [Quit: http://www.kvirc.net/ 4.9.1 Aria]
20160714 19:00:14-!- Greg-Boggs [~greg_bogg@173.240.241.83] has joined #wesnoth-dev
20160714 19:01:44-!- prkc [~prkc@catv-80-98-46-199.catv.broadband.hu] has quit [Quit: Leaving]
20160714 19:02:03-!- prkc [~prkc@46.166.190.205] has joined #wesnoth-dev
20160714 19:02:57< vultraz> ok, crash on exit still there
20160714 19:03:27< celticminstrel> So no compiler errors, that's good.
20160714 19:03:29< vultraz> http://pastebin.com/Z2sb2DBk
20160714 19:03:34< vultraz> stacktrace
20160714 19:04:10< celticminstrel> Why are there no line numbers? It should have line numbers.
20160714 19:04:53< vultraz> there are
20160714 19:05:29< celticminstrel> Not in the most important place.
20160714 19:05:44< celticminstrel> Something's probably amiss in your project settings or something.
20160714 19:05:50< vultraz> it only appeared for steps 1 - 3
20160714 19:05:58< vultraz> the line numbers, that is
20160714 19:06:14< vultraz> eh..
20160714 19:06:15< vultraz> hm
20160714 19:06:16< celticminstrel> Like I said, something seems to be amiss in your project settings. There should be line numbers for the Wesnoth code.
20160714 19:06:37< vultraz> why is the debugger output saying  727	./gthr-default.h: No such file or directory.
20160714 19:06:38< celticminstrel> I'm actually surprised that 1-3 have line numbers.
20160714 19:06:53-!- ancestral [~ancestral@63.92.240.233] has quit [Quit: i go nstuf kthxbai]
20160714 19:06:57< celticminstrel> That shouldn't matter, since you don't need to step into that file.
20160714 19:07:02< vultraz> ok
20160714 19:07:19< celticminstrel> Obviously you don't have that header. Maybe it's a private header or something, I dunno.
20160714 19:07:43< vultraz> well, I don't know how to make it show more line numbers otherwise
20160714 19:07:45< celticminstrel> You need line numbers for stack frame 0.
20160714 19:08:04-!- stikonas [~gentoo@wesnoth/translator/stikonas] has joined #wesnoth-dev
20160714 19:08:26< vultraz> going to see if MSVC can give me better
20160714 19:08:58< celticminstrel> Huh, you have MSVC?
20160714 19:09:14< vultraz> yeah i have 2015 installed
20160714 19:09:22< celticminstrel> There's probably some option somewhere in CodeBlocks's settings that says "strip line numbers" or something similar.
20160714 19:09:35< celticminstrel> And that option is enabled when it shouldn't be.
20160714 19:10:04< vultraz> Unhandled exception.. access violation... 
20160714 19:10:12-!- molt [~molt@dynamic-213-198-235-143.adsl.eunet.rs] has quit [Ping timeout: 276 seconds]
20160714 19:10:22< celticminstrel> Then you click uh... retry, I think, to enter the debugger?
20160714 19:10:32< vultraz> where is the...callstack
20160714 19:11:00< celticminstrel> Probably along the bottom somewhere. Not sure if it's visible by default though... might have to show it from a View menu or something.
20160714 19:11:42< celticminstrel> What GCC version are you using?
20160714 19:11:56< celticminstrel> I found this which could be relevant to the missing line numbers: https://stackoverflow.com/questions/18407563/gcc-doesnt-produce-line-number-information-even-with-g-option
20160714 19:12:11< vultraz> well this is useless
20160714 19:12:19< vultraz> celticminstrel: 5.1.0
20160714 19:12:41< celticminstrel> And what version of gdb?
20160714 19:13:02< vultraz> 7.6.1
20160714 19:13:12< celticminstrel> Hmm, I guess that post doesn't apply then... probably...
20160714 19:13:35< celticminstrel> Well, I suppose you could try -ggdb.
20160714 19:14:20< vultraz> msvc is being absolutely useless
20160714 19:14:20< vultraz> I don't need the disassembly
20160714 19:14:48< celticminstrel> I don't really mind that much when XCode shows me the dissassembly.
20160714 19:15:06< celticminstrel> Though if it was my own code I would be annoyed.
20160714 19:15:06-!- boucman [~rosen@wesnoth/developer/boucman] has joined #wesnoth-dev
20160714 19:15:07-!- boucman [~rosen@wesnoth/developer/boucman] has quit [Remote host closed the connection]
20160714 19:15:14< celticminstrel> (Which kinda actually happens sometimes in Wesnoth, for some reason.)
20160714 19:17:19-!- Ravana_ [~Ravana@unaffiliated/ravana/x-2327071] has joined #wesnoth-dev
20160714 19:17:54-!- ancestral [~ancestral@63.92.240.233] has joined #wesnoth-dev
20160714 19:19:07-!- Greg-Boggs [~greg_bogg@173.240.241.83] has quit [Remote host closed the connection]
20160714 19:21:58-!- ancestral [~ancestral@63.92.240.233] has quit [Client Quit]
20160714 19:22:10< vultraz> but it tells me nothing
20160714 19:23:46< irker214> wesnoth: Charles Dang wesnoth:Vultraz-event_handling_fixes a399a0071711 / src/ (events.cpp events.hpp): Use emplace_back when adding new event_contexts member https://github.com/wesnoth/wesnoth/commit/a399a0071711691abc0aabe1ff22048516f47d00
20160714 19:24:31< vultraz> Aginor: ^
20160714 19:24:37< celticminstrel> You didn't commit making it non-copyable?
20160714 19:24:42< vultraz> I did
20160714 19:24:45< celticminstrel> Oh wait, it's there, never mind.
20160714 19:24:50< celticminstrel> It's so tiny I missed it.
20160714 19:25:02< celticminstrel> I think that should still compile in MSVC 2013.
20160714 19:25:16< celticminstrel> As I recall, it supports deleted copy constructors but not deleted move constructors.
20160714 19:26:13< celticminstrel> (The pre-C++11 way is to make it private and not define it.)
20160714 19:27:00< JyrkiVesterinen> Yes, MSVC 2013 supports deleted move constructors. I have added a couple of them here.
20160714 19:27:45< celticminstrel> I distinctly recall getting errors with deleted move constructors in MSVC 2013.
20160714 19:28:05< celticminstrel> (In a different project though)
20160714 19:28:13< JyrkiVesterinen> Whoops, I meant to write "deleted *copy* constructors".
20160714 19:28:20< celticminstrel> Ah, okay.
20160714 19:29:52< vultraz> gonna restart my laptop for some updates
20160714 19:30:08-!- vultraz [~chatzilla@wesnoth/developer/vultraz] has quit [Remote host closed the connection]
20160714 19:33:08-!- travis-ci [~travis-ci@ec2-54-82-76-192.compute-1.amazonaws.com] has joined #wesnoth-dev
20160714 19:33:09< travis-ci> wesnoth/wesnoth#9779 (spirit_po - 5ce3e28 : Celtic Minstrel): The build is still failing.
20160714 19:33:09< travis-ci> Build details : https://travis-ci.org/wesnoth/wesnoth/builds/144814071
20160714 19:33:09-!- travis-ci [~travis-ci@ec2-54-82-76-192.compute-1.amazonaws.com] has left #wesnoth-dev []
20160714 19:33:51-!- molt [~molt@dynamic-213-198-235-143.adsl.eunet.rs] has joined #wesnoth-dev
20160714 19:51:08-!- vultraz [~chatzilla@wesnoth/developer/vultraz] has joined #wesnoth-dev
20160714 19:55:45-!- travis-ci [~travis-ci@ec2-107-22-115-196.compute-1.amazonaws.com] has joined #wesnoth-dev
20160714 19:55:46< travis-ci> wesnoth/wesnoth#9780 (Vultraz-event_handling_fixes - a399a00 : Charles Dang): The build is still failing.
20160714 19:55:46< travis-ci> Build details : https://travis-ci.org/wesnoth/wesnoth/builds/144822540
20160714 19:55:46-!- travis-ci [~travis-ci@ec2-107-22-115-196.compute-1.amazonaws.com] has left #wesnoth-dev []
20160714 19:56:05-!- Greg-Boggs [~greg_bogg@173.240.241.83] has joined #wesnoth-dev
20160714 20:00:49-!- Greg-Boggs [~greg_bogg@173.240.241.83] has quit [Ping timeout: 252 seconds]
20160714 20:08:08< irker214> wesnoth: gfgtdf wesnoth:master 9648d9c98951 / changelog: Update changelog https://github.com/wesnoth/wesnoth/commit/9648d9c98951e74e91d14617fa53bd20efa02d42
20160714 20:15:59< irker214> wesnoth: gfgtdf wesnoth:master ceea75cf0a6b / / (31 files in 13 dirs): replace some victory with scenario_end events. https://github.com/wesnoth/wesnoth/commit/ceea75cf0a6baf535c731a7b6f132d4317471909
20160714 20:17:15< celticminstrel> What.
20160714 20:17:47< celticminstrel> Why are you making it so complicated. Just sync the victory event's results.
20160714 20:19:28< celticminstrel> Or maybe somehow make the whole event synched. (The lose event too, of course.)
20160714 20:19:47< celticminstrel> (It looks like the scenario_end event already existed? I don't see it being added by that commit.)
20160714 20:25:44< Aginor> vultraz, celticminstrel: good job :)
20160714 20:25:57< Aginor> does it still crash or did that fix it too?
20160714 20:26:08< celticminstrel> Oh, gfgtdf isn't even here, no wonder he's not answering.
20160714 20:26:26< celticminstrel> Aginor: Apparently it fixed a crash but there's still a crash on exit.
20160714 20:26:50< celticminstrel> Should be easy to locate with a proper debugger (vultraz's is omitting line numbers for some reason). Dunno if it'll be as easy to fix.
20160714 20:27:10< celticminstrel> Maybe you can get a line number?
20160714 20:29:39-!- Greg-Boggs [~greg_bogg@173.240.241.83] has joined #wesnoth-dev
20160714 20:30:17< vultraz> Aginor: yeah, it seems to work correctly, but the game 'crashes' on exit.
20160714 20:30:42< vultraz> window disappears, and then windows tells me the program stopped working
20160714 20:31:04< vultraz> I'd guess it's some issue with late cleanup
20160714 20:32:48< celticminstrel> It's a crash in event_context::leave, right?
20160714 20:34:23< vultraz> no, 	events::context::remove_handler
20160714 20:34:40< celticminstrel> Oh right.
20160714 20:39:58-!- SigurdFD [~SigurdFD@dynamic-acs-72-23-110-58.zoominternet.net] has joined #wesnoth-dev
20160714 20:40:15-!- gfgtdf [~chatzilla@x4e3685ab.dyn.telefonica.de] has joined #wesnoth-dev
20160714 20:40:51< gfgtdf> celticminstrel: you cannot 'sync' an event, beeing synced means that an event is frired on all clients in the same way, ands thats not the case for victory events
20160714 20:41:03< celticminstrel> Why couldn't it be?
20160714 20:42:49< gfgtdf> becasue victory only fires if your side has won, and im mp its happens quite often that only one side wins while the other looses
20160714 20:43:38-!- SigurdFireDragon [~SigurdFD@dynamic-acs-72-23-110-58.zoominternet.net] has joined #wesnoth-dev
20160714 20:43:38-!- SigurdFireDragon [~SigurdFD@dynamic-acs-72-23-110-58.zoominternet.net] has quit [Client Quit]
20160714 20:44:22< celticminstrel> I know there's already a way to sync outcomes, what prevents that from being (safely) used in a voctory event?
20160714 20:44:34< celticminstrel> ^victory
20160714 20:46:16< zookeeper> gfgtdf, why did you make those changes in campaigns other than LoW then..?
20160714 20:46:28< gfgtdf> zookeeper: i said the the commit description
20160714 20:46:33< celticminstrel> ...oh, that's actually a very good question too.
20160714 20:46:38< vultraz> because you can play all campaigns in mp
20160714 20:46:52 * celticminstrel dislikes this [proceed_to_next_scenario] condition, for some reason.
20160714 20:46:55< celticminstrel> Since when?
20160714 20:47:00 * zookeeper scratches head
20160714 20:47:06< gfgtdf> vultraz: that might also be a reason but was not the mian reason why i did this
20160714 20:47:11< gfgtdf> main*
20160714 20:47:25< vultraz> there's an option for SP Campaigns in the Mp Create screen
20160714 20:47:40< celticminstrel> Why?
20160714 20:48:26-!- SigurdFireDragon [~SigurdFD@dynamic-acs-72-23-110-58.zoominternet.net] has joined #wesnoth-dev
20160714 20:48:30< zookeeper> gfgtdf, i don't understand what problems "it can casue bugs to create units in unsynced events which are used later" is referring to
20160714 20:48:45< celticminstrel> How can it possible cause bugs when there's no synching involved?
20160714 20:48:46-!- SigurdFD [~SigurdFD@dynamic-acs-72-23-110-58.zoominternet.net] has quit [Ping timeout: 240 seconds]
20160714 20:48:49< celticminstrel> ^posibly
20160714 20:48:52< celticminstrel> ^possibly
20160714 20:49:20< celticminstrel> Logically, in SP, everything is synched.
20160714 20:49:20< gfgtdf> zookeeper: i made a bugrepot for it http://gna.org/bugs/?24850
20160714 20:49:37< celticminstrel> Because there is nothing else to sync with.
20160714 20:50:07 * zookeeper blinks
20160714 20:50:27< celticminstrel> So basically you're just muddling through and trying to patch a sinking boat.
20160714 20:50:49< zookeeper> gfgtdf, "units created during unsynced events are assigned temporary ids" <- i don't understand what that means.
20160714 20:50:58< celticminstrel> I don't understand why the heck you would make the change that caused that bug in the first place.
20160714 20:51:12< celticminstrel> If it's string IDs, just make sure temporary units actually have one explicitly specified.
20160714 20:51:16< zookeeper> does it ignore the id you explicitly specify, or something else? how can an id be "temporary"?
20160714 20:51:43< celticminstrel> If it's underlying IDs, then you should make the temporariness of the ID somehow explicit rather than assuming things like that.
20160714 20:54:14< celticminstrel> Do please explain what you even mean by "are assigned temporary ids", because it's not in the least bit obvious.
20160714 20:55:29< zookeeper> and like... if there was a bug introduced that somehow screws up units created in perfectly regular events like victory, then the correct fix can't be to just change every victory event where units are created into some other event
20160714 20:55:41< celticminstrel> Creating non-temporary units in a victory even should be viewed as the exception to some bizarre rule.
20160714 20:55:57-!- molt [~molt@dynamic-213-198-235-143.adsl.eunet.rs] has quit [Quit: Leaving]
20160714 20:56:06< celticminstrel> In my opinion the commit that introduced bug 24850 should be reverted.
20160714 20:57:02< zookeeper> i don't know what change it was or who did it, but "since placing units on the map during unsynced events will casue OOS, those units are usually used for tempoary calauclation or animations and not placed on the map." seems like a very strange and frankly wrong assumption
20160714 20:57:19< celticminstrel> Yeah, it's not a reasonable assumption at all.
20160714 20:57:38< celticminstrel> And this most recent commit shows that it's definitely a wrong assumption.
20160714 20:57:40< gfgtdf> zookeeper: the game has an internal counter to create underlaying unit ids (which is used to create ids) if this counter goes out of sync then newly creates untis will get different ids whihc casues OOS if peolpe try to recall them
20160714 20:57:57< celticminstrel> gfgtdf: What if you create the unit into a variable rather than on the map?
20160714 20:58:16< celticminstrel> gfgtdf: Although, I'd argue that underlying IDs do not need to be synched.
20160714 20:58:27< celticminstrel> The game shouldn't care if underlying IDs differ on different clients.
20160714 20:58:43< celticminstrel> ^should not need
20160714 20:58:59< gfgtdf> zookeeper: so increasing that counter during unsynced events could casue OOS, to prevent this the game has a second interna counter whcih is used for 'tempoaary' units, so o prevent OOS the game uses that seond counter when a unit is cretes auring an unsynced evnet  liek select
20160714 20:59:25< celticminstrel> Seriously, why would you do that.
20160714 20:59:32< gfgtdf> celticminstrel: well, the underlying is is used to create the normal id, which usualyl has teh format unittype-underlying_id the 
20160714 20:59:50< vultraz> OOS are so annoying...
20160714 21:00:08< zookeeper> gfgtdf, right, okay...
20160714 21:00:12< vultraz> we should implement a game server model like Anura uses
20160714 21:00:28< celticminstrel> Oh, I see.
20160714 21:00:50< celticminstrel> That's a pretty ridiculous way to create IDs as well, though.
20160714 21:01:03< celticminstrel> On the other hand, the only other way I can think of is even more ridiculous...
20160714 21:01:23< irker214> wesnoth: Gregory A Lundberg wesnoth:master f377ae44bff2 / src/generators/cave_map_generator.cpp: Missing end-of-line showing seed on stderr https://github.com/wesnoth/wesnoth/commit/f377ae44bff293606cb2903a2bcf15fa3262280b
20160714 21:01:26< irker214> wesnoth: Charles Dang wesnoth:master 3bef1c6c91ef / src/generators/cave_map_generator.cpp: Merge pull request #695 from GregoryLundberg/GL_cmg https://github.com/wesnoth/wesnoth/commit/3bef1c6c91efa97d781730e01e1c1080bee66d19
20160714 21:01:55< celticminstrel> gfgtdf: You didn't answer the question about creating the unit into a variable instead of on the map.
20160714 21:02:39< celticminstrel> vultraz: That's probably easier said than done.
20160714 21:02:49< vultraz> celticminstrel: this is true
20160714 21:03:12-!- prkc [~prkc@46.166.190.205] has quit [Ping timeout: 260 seconds]
20160714 21:03:17< gfgtdf> celticminstrel: the unit ids are created when the units is created so it doent matter whether  it placed on the map or not
20160714 21:03:23< celticminstrel> I see.
20160714 21:03:29< zookeeper> hmh
20160714 21:03:51< celticminstrel> gfgtdf: Do you have any particular examples of places where these units are indeed temporary as you (or someone) falsely assumed?
20160714 21:05:04< zookeeper> it sounds like a legitimate issue, so i'd be inclined to say that a counter is the wrong approach for creating new underlying id's. couldn't it somehow deterministically generate a new one based on existing units (tally up all the existing units of the same type, use that as the number, or something)?
20160714 21:05:47< gfgtdf> celticminstrel: im quite sure i have seen code liek creatign a temporary unti to use if with wesnoth.find_cost map, or to to calculate a terrain defende of a unit type at a certain terrain, but i cannot give you a conrete example sry
20160714 21:05:47< celticminstrel> Oh, I just thought of something else. If the game tracked which units had been assigned temporary IDs, then it should be possible to reassign the IDs of any remaining units once the unsynched event is finished.
20160714 21:06:34< vultraz> celticminstrel: I closed the events pr and opened a new one https://github.com/wesnoth/wesnoth/pull/696
20160714 21:06:38< celticminstrel> Okay.
20160714 21:07:58< gfgtdf> zookeeper: i personally think we shodul try to remove the internal unit id entireley and just use something liek a hashmap, string(id) ->unit in the map or soemthing
20160714 21:08:07< gfgtdf> zookeeper: not wure if this oudl fix this isssue though
20160714 21:08:09< gfgtdf> sure*
20160714 21:08:22< celticminstrel> I wouldn't mind removing the underlying ID.
20160714 21:09:00< celticminstrel> Of course you'd still need a way to generate unique string IDs though.
20160714 21:10:43< zookeeper> gfgtdf, internal unit id == underlying_id?
20160714 21:11:06< gfgtdf> zookeeper: right , i meant underlying_id
20160714 21:11:12< zookeeper> okay. well if the problem is OOS id's stemming from OOS underlying_id's, then the fix would need to be more or less the same regardless of whether underlying_id stays or not, right?
20160714 21:12:01< gfgtdf> zookeeper: hmm yes but using string might give us more freedom than those ints,
20160714 21:12:30< zookeeper> maybe
20160714 21:13:02-!- JyrkiVesterinen [~JyrkiVest@87-92-50-49.bb.dnainternet.fi] has quit [Quit: .]
20160714 21:14:25-!- Greg-Boggs [~greg_bogg@173.240.241.83] has quit [Remote host closed the connection]
20160714 21:17:19< gfgtdf> celticminstrel	: the game does track which ids are temparay (actuall anythign with an id > max_int/2 is a temporary id)
20160714 21:17:27< vultraz> celticminstrel: I've narrowed down the crash
20160714 21:17:36-!- prkc [~prkc@catv-80-98-46-199.catv.broadband.hu] has joined #wesnoth-dev
20160714 21:17:41< vultraz> celticminstrel: seems to happen in cycle_focus, specifically the loop.
20160714 21:17:47-!- Greg-Boggs [~greg_bogg@173.240.241.83] has joined #wesnoth-dev
20160714 21:18:04< gfgtdf> celticminstrel: while 'once the unsynched event is finished' might not work i think we coudl just remove all those tempara id during scenario transition, which might at least fix those victory events.
20160714 21:18:48< Aginor> probably the global context is the cause of the crash
20160714 21:19:03< Aginor> I'm at work, so I can't fire up and run a debugger here
20160714 21:19:37< Aginor> I suspect that the changes you made to the constructor of context might be the cause, but I haven't looked at the code so I don't know
20160714 21:21:31< celticminstrel> gfgtdf: You mean like omitting the ID from the saved game?
20160714 21:22:09< gfgtdf> celticminstrel: in case it a a temporary
20160714 21:23:32< celticminstrel> Maybe that could work. I guess new IDs would then be assigned on load or something, though I wonder if that would really be safe...
20160714 21:26:07< gfgtdf> celticminstrel: hmm well usually temparary units shouldn't go into saves anways, if we assum ethat this just effect viecroy events and scenario start saves, then those unit will get their ids assignes liek all other units thta wmkl devs write in  [scenario][side]
20160714 21:26:16< vultraz> ok what... is happening here
20160714 21:26:36< celticminstrel> I see,
20160714 21:26:50< vultraz> if I comment out the call to cycle_focus() in remove_handler, the game exist fine
20160714 21:26:54< celticminstrel> Besides victory, loss, and select, are any other events inherently unsynched?
20160714 21:27:13< vultraz> if I remove the contents of cycle_focus, it works fine
20160714 21:27:18< vultraz> if I comment it out, it does not :|
20160714 21:27:38< celticminstrel> I think it's safe to assume that units won't be created in select events, so if only victory and loss events are affected by this, then what you've described sounds like a potential fix.
20160714 21:28:36< vultraz> how the hell do *code comments* affect how something works
20160714 21:30:25< gfgtdf> celticminstrel: preload is also unsynced
20160714 21:30:27< vultraz> no sense does this make.
20160714 21:31:05< celticminstrel> I don't think preload is an issue either.
20160714 21:31:24< zookeeper> gfgtdf, but in practise is it not impossible for one client to execute the preload event and another client not to?
20160714 21:32:29< gfgtdf> zookeeper: i think th reason why preload event are unsynced is more about replays, when you reload the game the preload event is fired 2 times in tootal during the original replay but still only once during replay
20160714 21:32:43< celticminstrel> Huhwhat?
20160714 21:32:57< gfgtdf> during the original game*
20160714 21:33:37< celticminstrel> But if the preload event changes game state it's already known that that's asking for trouble, so I don't think it's a problem here.
20160714 21:33:59< zookeeper> ah, right
20160714 21:43:17< vultraz> gfgtdf: i know it'd probably be a lot of work to set up a whole game server model, but what about something simpler... what if we sync *actions* instead of events
20160714 21:43:32< vultraz> gfgtdf: so like, no matter what event creates a unit, that action is synched
20160714 21:43:33-!- Appleman1234 [~Appleman1@KD036012037023.au-net.ne.jp] has quit [Ping timeout: 240 seconds]
20160714 21:44:39< gfgtdf> vultraz: hmm yes thats how it currently works, its just tha all events are assignes to certain actions so that we can talk about synced and not synced events
20160714 21:45:14< gfgtdf> vultraz: if you fire a perload event from a moveto event for example that code in it would run as synced.
20160714 21:46:11< vultraz> then I don't see how we're having issues here  :/
20160714 21:46:18-!- edgrey [~edgrey@178.204.232.6] has quit [Remote host closed the connection]
20160714 21:46:29-!- edgrey [~edgrey@178.204.232.6] has joined #wesnoth-dev
20160714 21:46:41-!- edgrey [~edgrey@178.204.232.6] has quit [Remote host closed the connection]
20160714 21:47:01< vultraz> what I'm proposing is that it shouldn't matter if the event is classified as unsynced or not
20160714 21:47:13< celticminstrel> gfgtdf: The basic idea is that any time a client does, such as executing a [unit] tag, that action should be transmitted across the network.
20160714 21:47:19< vultraz> so in this case, you're worried about units created in victory events
20160714 21:47:25< vultraz> because victory is unsynced
20160714 21:47:39< celticminstrel> Though that's not without problems.
20160714 21:47:42< vultraz> but if we do as celticminstrel just described, it wouldn't matter if victory is synced or not
20160714 21:48:10< celticminstrel> Like the possibility of a cheating client.
20160714 21:48:23< vultraz> The TBS server wesnoth 2 has in Anura basically calculates gamestate changes itself based on instructions from clients and dispatches the info to the clients (if I understand correctly)
20160714 21:48:57< vultraz> (server in this case doesn't mean an actual mp server or anything)
20160714 21:49:13< vultraz> Such a system would solve what you describe
20160714 21:49:25< vultraz> But since it'd be hard to do, perhaps synced actions is a good start.
20160714 21:49:49< celticminstrel> vultraz: A central server is certainly advantageous but might require you to upload your addons to the server as part of the game initialization.
20160714 21:50:20< celticminstrel> Sending all actions across the network theoretically solves the synchronization problem but makes it easy to cheat.
20160714 21:51:15< celticminstrel> I think it should be possible to change the criteria for firing victory and loss events.
20160714 21:51:29< celticminstrel> Currently they are fired when the current client controls a side that has won or lost, right?
20160714 21:51:30< vultraz> the design paradigm applies to sp games too, fyi.
20160714 21:51:45< celticminstrel> Why can't they fire when any human side wins or loses?
20160714 21:51:46< vultraz> as I said, it doesn't necessarily invole a network
20160714 21:52:11< celticminstrel> It's pointless for a SP game, though admittedly it doesn't hurt.
20160714 21:53:00< celticminstrel> Anyway, all clients should be informed of which sides are human-controlled and fire victory or loss events based on all human sides at once, rather than only firing the one that pertains to their own side.
20160714 21:53:15< vultraz> since the calculations are performed by the 'server' using the game's own rules, I'm pretty sure it would be able to tell if a unit appears via illegal action
20160714 21:53:16< celticminstrel> (That would mean that a game with N human players fires N victory or loss events at the end of the scenario.)
20160714 21:53:50< vultraz> that's the theory anyway, I think
20160714 21:54:05< celticminstrel> I think you've misassociated what I said.
20160714 21:54:14< vultraz> celticminstrel: are you saying every side should fire their own victory event 
20160714 21:54:20< vultraz> er, player
20160714 21:54:23< celticminstrel> "Sending all actions across the network" referred to the idea of synching every action.
20160714 21:54:40< celticminstrel> vultraz: I'm saying every client should fire everyone's victory events and also everyone's loss events.
20160714 21:54:52< vultraz> that makes sense
20160714 21:55:06< vultraz> except...
20160714 21:55:11< celticminstrel> It's not as if the client doesn't know exactly who won and who lost, right?
20160714 21:55:19< vultraz> what if you're in an mp game and there's a victory event that creates a unit for side 1
20160714 21:55:19< zookeeper> celticminstrel, i'd agree with that if not for the fact that you couldn't put any victory/defeat dialogue and such in them then
20160714 21:55:33< vultraz> and all fire
20160714 21:55:42< vultraz> does side 1 get n copies of the unit
20160714 21:56:06< zookeeper> then everyone creates the side for side 1 as with any other event and there's no OOS?
20160714 21:56:11< zookeeper> s/side/unit
20160714 21:56:12< celticminstrel> zookeeper: You could do it with side-specific messages.
20160714 21:56:27< zookeeper> celticminstrel, sure
20160714 21:56:29-!- Appleman1234 [~Appleman1@KD036012046026.au-net.ne.jp] has joined #wesnoth-dev
20160714 21:56:30< celticminstrel> vultraz: The victory event would only fire twice if there were two human players on a team.
20160714 21:57:44< celticminstrel> If it's a one-on-one game there's no problem. If it's a scenario intended to be eg two-vs-two, then it would have to account for the fact that the event will fire twice.+
20160714 22:01:30< gfgtdf> celticminstrel: hmm dont reall like the idea pof fireing a victory event for each side, why not just fore victory event if an human side won (whihc woudl basially the sme condition as the filtered scneario_end i used)
20160714 22:01:58< gfgtdf> celticminstrel: we coudl also add a "local victory" event that fires under the samecinditions as the victory even currently does
20160714 22:02:21< gfgtdf> same conditions*
20160714 22:03:49< zookeeper> random thought i apparently can't think through myself: can't you solve the id OOS by having the client on which the unit-creating action happens send its own id counter number too? then all other clients could automatically sync with that. maybe it'd need to be incremented automatically to avoid conflicts if another client's counter is ahead, but still.
20160714 22:08:47< gfgtdf> zookeeper: usually you can onyl send such stoff during a synced context, for or examepl when its side1 turn, and side 2 creates a somehow increases the id counter in a select event and it woudl dend an update about the counter, it migth happen that side1 already executed action that use the counter but which he didnt send to the server yet (for example becasue they are undoable)
20160714 22:10:06< celticminstrel> You make it sound like it's impossible to get into a synched context if you've started in an unsynched context (like a victory event).
20160714 22:10:13< zookeeper> how could side 1 execute any actions before side 2, during side 2's turn?
20160714 22:10:29< zookeeper> oh right, you said side 1's turn
20160714 22:10:36< zookeeper> in which case surely side 2 can't trigger any select events?
20160714 22:10:58< zookeeper> might trigger (or should be able to) set_menu_items though...
20160714 22:11:09< gfgtdf> zookeeper: im quite sure you can trigger select events white its not your turn
20160714 22:11:20< zookeeper> yeah, maybe
20160714 22:11:46< vultraz> gfgtdf: honestly, it seems just making sure any actions like [unit] are immediately synced would solve a lot of these problems
20160714 22:12:04-!- RatArmy [~RatArmy@om126211116174.13.openmobile.ne.jp] has joined #wesnoth-dev
20160714 22:12:12< zookeeper> the thing is that it's not hard to imagine temporary units being created in a select event for instance
20160714 22:12:19< celticminstrel> And I still don't think the underlying IDs should care about synchronization.
20160714 22:12:22< gfgtdf> celticminstrel: the onyl way (in wml/lua) to 'enter' a  synced state is [do_command] or the ai attack\move\recuit etc functions, and they all only work during your won turn
20160714 22:12:51< gfgtdf> your own turn*
20160714 22:12:57< zookeeper> celticminstrel, i believe the problem was the actual id's, which when not specified by WML apparently get generated from the underlying_id
20160714 22:13:10< celticminstrel> Yes, I got that.
20160714 22:13:21< zookeeper> right, okay
20160714 22:13:27< celticminstrel> Maybe a different method of generating the actual IDs?
20160714 22:13:57< celticminstrel> If the ID included the owner's side number, maybe it would be easier to ensure IDs are unique even if underlying IDs might differ.
20160714 22:14:16< Aginor> UUIDs?
20160714 22:14:24< celticminstrel> Well yes, UUIDs are also an option.
20160714 22:14:32< zookeeper> i think that whatever extra numbers you tack onto it, it still only lessens the likelihood of them going OOS
20160714 22:14:47< vultraz> Aginor: Underlying Unit ID
20160714 22:14:52< celticminstrel> They're ugly, but I guess auto-IDs aren't intended to be user-friendly anyway.
20160714 22:14:57< Aginor> vultraz: no.
20160714 22:15:04< celticminstrel> vultraz: No, Unique Uniform ID.
20160714 22:15:13< Aginor> vultraz: https://en.wikipedia.org/wiki/Universally_unique_identifier
20160714 22:15:22< celticminstrel> Oh, or that. Whatever.
20160714 22:15:39< vultraz> so, which of the three are we talking about
20160714 22:15:40< Aginor> v5 would work for us
20160714 22:15:55< celticminstrel> Aginor: Isn't the random seed also synchronized? Would that cause problems with UUID generations?
20160714 22:16:08< celticminstrel> ^generation
20160714 22:16:22< Aginor> celticminstrel: for v4 it would, v5 would be fine
20160714 22:16:26< Aginor> https://tools.ietf.org/html/rfc4122
20160714 22:16:27< celticminstrel> I see.
20160714 22:16:47< Aginor> celticminstrel: or use a different PRNG for v4 UUIDs
20160714 22:16:52< zookeeper> one thing i was wondering was why do units need to get generated id's in the first place? can't the game handle a unit that doesn't have an id because it now references them by it in replays or whatever?
20160714 22:17:03< Aginor> there's still a chance of collision though, wich we wouldn't want
20160714 22:17:04< celticminstrel> Aginor: I was about to mention that as well.
20160714 22:17:22< celticminstrel> So we'd need some method of collision resolution, just in case?
20160714 22:17:33< celticminstrel> Or is that only with v4 and would not be needed with v5?
20160714 22:17:44< Aginor> celticminstrel: v5 should work if we use username or whatever as namespace
20160714 22:18:02< celticminstrel> I see.
20160714 22:18:27< Aginor> celticminstrel: trunkated sha1 hash + namespace for each UUID
20160714 22:19:09< gfgtdf> celticminstrel: what exactly do you want to hash ?
20160714 22:19:16< celticminstrel> ...
20160714 22:19:20< Aginor> However, these probabilities only hold when the UUIDs are generated using sufficient entropy. Otherwise, the probability of duplicates could be significantly higher, since the statistical dispersion might be lower. Where unique identifiers are required for distributed applications, so that UUIDs do not clash even when data from many devices is merged, the randomness of the seeds and generators used on 
20160714 22:19:22< gfgtdf> Aginor: ^
20160714 22:19:26< Aginor> every device must be reliable for the life of the application. Where this is not feasible, RFC4122 recommends using a namespace variant, such as as Type 5 UUID, instead.
20160714 22:19:42 * zookeeper doesn't understand how something like that would help here
20160714 22:20:25< celticminstrel> It would basically mean that the unit string IDs are independent of the underlying IDs.
20160714 22:20:31< Aginor> gfgtdf: I'd use some attributes of the unit that would result in a locally unique set of identifiers
20160714 22:20:55< Aginor> gfgtdf: as long as the unit is unique on the client it can uniquely be identified globally
20160714 22:21:15< gfgtdf> Aginor: hmm but you do somethines create unti which exactly the same attributes, just as in [unit] type=spearman, side=1[/unit][unit] type=spearman, side=1[/unit]
20160714 22:21:45< Aginor> gfgtdf: they'll still have more unique properties within the engine though
20160714 22:21:48< zookeeper> yeah, say if you create three identical units onto a recall list so you can't even take location into account
20160714 22:22:37< gfgtdf> Aginor: well unless you talk about the pinter value of the unit unit object, they don't and since we want the id to by synced we cannot use that
20160714 22:22:43< zookeeper> Aginor, are those properties guaranteed to be the same across all clients?
20160714 22:23:15< celticminstrel> gfgtdf: No, that's not true.
20160714 22:23:43< Aginor> use a local ID that's a part of the unit properties, the actual ID is a part of that
20160714 22:23:49< celticminstrel> gfgtdf: The physical address of the unit is a distinct possibility if the goal is for an ID that is generated by one client and distributed to the others.
20160714 22:23:55< Aginor> use a local ID that's a part of the unit properties, the actual ID utilises it
20160714 22:24:04< gfgtdf> Aginor: the id is generated with the underlaying id usually
20160714 22:24:06< celticminstrel> Not that I'd recommend basing anything on physical address, mind you.
20160714 22:24:33< celticminstrel> gfgtdf: The point is that it doesn't matter whether the data it's based on is synched, because the resulting ID is going to be synched to all clients.
20160714 22:24:50< gfgtdf> celticminstrel: who said that its synced to all the clients?
20160714 22:24:53< Aginor> location is probably not good as it's a transient property that shouldn't give the unit a new UUID when it changes
20160714 22:25:00< celticminstrel> gfgtdf: That's the whole point of this idea.
20160714 22:25:11< gfgtdf> celticminstrel: i actuall thought we are just thinking about alternative underlying id generators
20160714 22:25:20< celticminstrel> gfgtdf: Not underlying ID, string ID.
20160714 22:26:00-!- mjs-de [~mjs-de@x4db50acd.dyn.telefonica.de] has joined #wesnoth-dev
20160714 22:26:16< zookeeper> i bet i'll have lots of confusing backlog to read when i return in 20 minutes...
20160714 22:26:21< Aginor> easy solution: have a counter locally, starts at 0 or whatever, derive UUID from counter+type+username to ensure uniqueness. Sync UUIDs and use UUIDs to identify units globally
20160714 22:27:32< celticminstrel> Hmm, there might be a problem here in that the logic of creating units probably runs simultaneously on all clients rather than just occurring on one and then being broadcast...
20160714 22:27:51< celticminstrel> I don't think it's an unsolvable problem, though...
20160714 22:28:46< celticminstrel> What I mean is that it might be necessary to choose something that can produce identical results when run on different clients while still ensuring global uniqueness.
20160714 22:28:57< Aginor> no, as the changes on one client will not collide with other client
20160714 22:29:06< celticminstrel> Hmm?
20160714 22:29:28< Aginor> celticminstrel: my 3-tuple above would yield that if the counter is initialised correctly ;)
20160714 22:29:54< Aginor> we may want to add a few more things to the tuple to make it more unique
20160714 22:30:00< celticminstrel> Username meaning the Wesnoth login name?
20160714 22:30:04< Aginor> yes
20160714 22:30:14< celticminstrel> (As opposed to the system username.)
20160714 22:30:38< Aginor> I'm assuming this is for a networked context where we have the multiplayer username around
20160714 22:30:47< Aginor> and they multiplayer username is guaranteed to be unique
20160714 22:31:03-!- RatArmy [~RatArmy@om126211116174.13.openmobile.ne.jp] has quit [Ping timeout: 276 seconds]
20160714 22:31:14< celticminstrel> Yeah, that's the general idea, though we'd still need to generate IDs in single-player contexts too, and it's probably easier if the same method is used.
20160714 22:31:18< Aginor> otherwise we can simply generate a new unique namespace for each instance that's created and save in the general game settings
20160714 22:31:25< celticminstrel> Maybe the username could be empty for SP.
20160714 22:31:36< celticminstrel> Or something, I dunno.
20160714 22:31:51< celticminstrel> I think the MP username is guaranteed to be unique on a given server.
20160714 22:32:08< Aginor> ok
20160714 22:32:48< Aginor> making a unique namespace for a client won't be hard though
20160714 22:33:08< Aginor> we may need to pick a few other things too
20160714 22:33:17< celticminstrel> In theory you can't assume that a given client always uses the same username though.
20160714 22:33:39< Aginor> celticminstrel: for a multiplayer game though, I cna?
20160714 22:33:49< Aginor> but ok
20160714 22:33:57< Aginor> no username as namespace
20160714 22:33:57< celticminstrel> No, someone could choose to switch nicknames each time they connect, if they wanted to,
20160714 22:34:32< celticminstrel> Mind you, if they're using their forum credentials to log into the Wesnoth server, they would probably consistently use the same nickname; and I would expect that most people would tend to even without that.
20160714 22:34:56< Aginor> generate and keep a unique identifier UUIDv4(? :D) and use as namespace for each client, store the namespace in game config so it remains the same until the user wipes the config
20160714 22:35:21< celticminstrel> I dunno.
20160714 22:35:42< vultraz> Aginor: how does next Wednesday sound for a release?
20160714 22:35:44< Aginor> I shouldn't get too bogged down in this though, I should be writing user stories
20160714 22:35:51-!- Greg-Boggs [~greg_bogg@173.240.241.83] has quit [Remote host closed the connection]
20160714 22:35:52< celticminstrel> User stories?
20160714 22:36:05-!- Greg-Boggs [~greg_bogg@173.240.241.83] has joined #wesnoth-dev
20160714 22:36:16< Aginor> vultraz: make it a sunday so peple have time to test/package/whatnot over the weekend
20160714 22:36:24< Aginor> celticminstrel: for work
20160714 22:36:29< vultraz> Aginor: sunday the 24th?
20160714 22:36:46< Aginor> celticminstrel: https://en.wikipedia.org/wiki/User_story
20160714 22:37:32< Aginor> vultraz: sure
20160714 22:37:38< Aginor> if blockers are resolved ;)
20160714 22:37:58-!- vultraz changed the topic of #wesnoth-dev to: 1.13.5 scheduled for 7/24 | Wesnoth Developers Channel | >>> Want to help? Go here: http://r.wesnoth.org/t42911 (and thanks!) <<< | Logs: http://irclogs.wesnoth.org | Bug tracker: http://bugs.wesnoth.org
20160714 22:38:11< Aginor> I'd suggest make sure that blockers are resolved this weekend and put in a week of testing/bugfixes before release
20160714 22:38:22< vultraz> And yes, I used 7/24 and not 24/7. DWI :P
20160714 22:38:52< Aginor> although I'd really like to try to get in my flickerfix idea before the release but I don't think I'll have time
20160714 22:39:00< celticminstrel> So the next release is the 7th day of the 24th month, got it.
20160714 22:39:15 * Aginor has celticminstrel's back
20160714 22:39:36< celticminstrel> And here I thought we hadn't made it to Mars yet.
20160714 22:40:05< vultraz> Aginor: there are still 3 unfixed blockers but they're older/harder ones, so they're not an obstacle for this release
20160714 22:40:12-!- prkc [~prkc@catv-80-98-46-199.catv.broadband.hu] has quit [Ping timeout: 250 seconds]
20160714 22:40:24-!- mjs-de [~mjs-de@x4db50acd.dyn.telefonica.de] has quit [Remote host closed the connection]
20160714 22:40:34< celticminstrel> vultraz: What sort of blockers, and why are blockers not blockers?
20160714 22:41:19< vultraz> celticminstrel: https://gna.org/bugs/index.php?24541 I don't know if ancestral resolved this; I will ask
20160714 22:42:14< vultraz> https://gna.org/bugs/index.php?23752 there's a temporary workaround in place, but unfortunately it caused https://gna.org/bugs/?24780 Furthermore, it's a GUI2 issue
20160714 22:43:07< vultraz> https://gna.org/bugs/index.php?23203 and I actually think this might have been fixed...
20160714 22:44:05< vultraz> I'm pretty sure I remember that being fixed
20160714 22:45:23< celticminstrel> The discussion appears to support your memory.
20160714 22:53:06-!- prkc [~prkc@46.166.188.245] has joined #wesnoth-dev
20160714 22:53:15< vultraz> celticminstrel: I think I'll mark it as fixed
20160714 22:54:32< zookeeper> Aginor, but currently it's not uniqueness that's a problem, it's the use of a counter and how to ensure it can't go OOS
20160714 22:56:26< Aginor> zookeeper: but by identifying units with global identifiers that are guaranteed to be unique, we don't need to worry about the counter at all as it can be kept local to a client
20160714 22:56:35< vultraz> celticminstrel: I think the gist of it was that the newer vorbis dll was the problem.
20160714 22:56:53< vultraz> going to assume our packagers addressed that last year
20160714 22:57:32< zookeeper> Aginor, so how do you make different clients agree what the global identified for a given unit is, when it is derived partially from a local counter that can differ on each client?
20160714 22:58:21< vultraz> shadowm: IIRC we addressed that vorbis issue, did we not?
20160714 22:59:45< Aginor> zookeeper: client A spawns unit A1. Client A broadcast's unit A1's globally unique ID to client B, C, D. Client B uses A1's unique ID to identify it
20160714 23:00:24< Aginor> zookeeper: in that model, the client that drives the decision to spawn a unit is responsible for broadcasting its ID
20160714 23:00:48< celticminstrel> I think I mentioned this already, but I suspect that each client independently spawns the unit.
20160714 23:01:03< celticminstrel> Which is of course absurd, but also very hard to change.
20160714 23:01:08< Aginor> zookeeper: scripted units that are spawned across all clients concurrently can be managed in a different namespace, or never broadcast at all
20160714 23:01:08< zookeeper> so client A broadcasts unit A1's id even when it never broadcasts the action which triggers the event which creates A1? and the other clients just... sort of keep the id stashed away somewhere, in case they too will create A1 in the future?
20160714 23:02:15< Aginor> zookeeper: shouldn't B,C,D know about A1's units?
20160714 23:02:25< gfgtdf> i really think that, if we want to revert those wml changes in my last commit, the best way is to change the definition of victory event to fire when any human side got a victory.
20160714 23:02:26< Aginor> zookeeper: shouldn't B,C,D know about A's units?
20160714 23:03:14< celticminstrel> gfgtdf: As long as there's some way to determine which human side actually did win.
20160714 23:03:19 * zookeeper needs to think for a moment
20160714 23:04:03< Aginor> I will be rude and close this window
20160714 23:04:09< Aginor> I need to get work done, sorry
20160714 23:04:10< celticminstrel> gfgtdf: I'm less concerned about the changes to LoW, mind you.
20160714 23:04:26< zookeeper> it's only rude if you don't read the logs later :J
20160714 23:04:31< celticminstrel> Single-player campaigns should not have to worry about desynching though.
20160714 23:04:39< gfgtdf> celticminstrel: we'd just add a a second event "local victory" that fired under teh same conditions as teh victor event previousl did
20160714 23:04:46< celticminstrel> Geh.
20160714 23:04:59< celticminstrel> No.
20160714 23:10:50-!- ChipmunkV [~vova@d0017-2-88-172-31-68.fbx.proxad.net] has quit [Quit: ChipmunkV]
20160714 23:11:51< zookeeper> Aginor, ok so i guess it could work so that when A transmits an action which triggers an event which leads to the creation of units, it also transmits the list of id's to be used for those units, and other clients obey? :x
20160714 23:12:49< zookeeper> (and the list is created based on A's local counter, of course)
20160714 23:15:15< gfgtdf> i wonder which case your you currently traing to fix ?
20160714 23:15:55< celticminstrel> I've lost track a bit.
20160714 23:16:08< gfgtdf> s/your you/you are
20160714 23:16:14< gfgtdf> trying*
20160714 23:21:14< zookeeper> maybe i lost track too, but isn't the root problem of all this basically that units created in unsynced events cause the local underlying_id counters on different clients to go OOS?
20160714 23:22:55< zookeeper> and that if we could fix _that_, then none of these other workarounds like messing with victory events or assigning temporary id's would be necessary?
20160714 23:28:43< Aginor> zookeeper: yes
20160714 23:28:50 * Aginor disappears again
20160714 23:29:03< Aginor> also, yes again for your most recent statement ;)
20160714 23:30:06< gfgtdf> zookeeper: no the counter doesnt go out of sync becasue each clinet has a seperate counter for units created during unsynced events
20160714 23:30:46< zookeeper> yes that's the temporary id's workaround, is it not?
20160714 23:31:07< gfgtdf> zookeeper: it is
20160714 23:32:29< zookeeper> gfgtdf, and that in turn leads to bug #24850, right?
20160714 23:32:38< Aginor> I think it removes the need to sync any counters as long as the appropriate events are broadcast, and of course the new unique IDs
20160714 23:33:07< Aginor> what clients do internally doesn't matter as long as they agree on the unique id's, and each can create new ones that are still unique
20160714 23:33:10< gfgtdf> zookeeper: yes i fixed #24850 in latest master by not using victory events in that case
20160714 23:33:24< Aginor> unique across all clients
20160714 23:34:00< gfgtdf> yes but*
20160714 23:35:30< zookeeper> gfgtdf, yes and that commit in turn has its own problem as we've seen.
20160714 23:36:06< zookeeper> so what i was suggesting is that couldn't we make the whole mess go away if we just broadcasted those id lists?
20160714 23:36:24< zookeeper> as per this:
20160714 23:36:25< zookeeper> <zookeeper> Aginor, ok so i guess it could work so that when A transmits an action which triggers an event which leads to the creation of units, it also transmits the list of id's to be used for those units, and other clients obey? :x
20160714 23:37:11< Aginor> it would also resolve a whole slew of potential concurrency pitfalls
20160714 23:39:28< gfgtdf> zookeeper: hmm well we dont know in advance wher an action will create a unit, so you mena just sending the current id counter with each action ?
20160714 23:40:07< gfgtdf> whether
20160714 23:40:09< gfgtdf> *
20160714 23:41:27< gfgtdf> zookeeper: also the current issue is about actions that are not transmitted to other clients (victory, select)
20160714 23:41:30< zookeeper> i mean that as a client executes events and such, it writes down a list of all the id's it generates, and then afterwards when the action (which triggered those events) is broadcasted, that id list gets broadcasted along with it, and when the other clients replay the action, they will use the id's from that list when creating the units
20160714 23:42:27< zookeeper> and yeah maybe you could just send the current id counter with each action, i don't know if there's any reason why that couldn't work
20160714 23:43:58< zookeeper> gfgtdf, i don't see those as a problem. one just needs to take care when using victory or select events in multiplayer contexts
20160714 23:45:07< zookeeper> as i see it, those are just normal potential sources of OOS that the author must be careful with
20160714 23:45:22< gfgtdf> zookeeper: actions are often bradcasts beore all triggered events are finished.
20160714 23:45:41< gfgtdf> broadcased*
20160714 23:46:18< zookeeper> really?
20160714 23:46:23< zookeeper> well, i didn't know that
20160714 23:47:48< gfgtdf> zookeeper: an action is broadcases as soon as it requirs a 'remote choice' this doesnt only imply [get_global_variable] from an other side, but also random values required form the server.
20160714 23:48:30< gfgtdf> zookeeper: so since [unit] for examepl usualyl needs ranom traits, a [unit] will usually imply (if we are ina synced action) sending the current action immidiatele
20160714 23:50:00< zookeeper> right, right.
20160714 23:50:43-!- celmin [~celticmin@unaffiliated/celticminstrel] has joined #wesnoth-dev
20160714 23:53:12< celmin> gfgtdf: It is entirely within possibility to forcibly synchronize the victory event.
20160714 23:53:37< celmin> The simplest brute-force method would be to simply resend the entire gamestate after the event completes.
20160714 23:54:05< celmin> Though that's probably a bit much, particularly when considering that the victory event is more or less the last thing that happens.
20160714 23:55:36< celmin> The victory is unsynched because someone specifically decided not to sync it, not because it is impossible to sync it.
20160714 23:55:57< zookeeper> gfgtdf, so what happens if an event is triggered, and it first does something that requires a "remote choice", and then immediately afterwards has a [message] with [option]s? it can't broadcast and start executing that event on the other clients before the current player has made their choice.
--- Log closed Fri Jul 15 00:00:31 2016