--- Log opened Sun Apr 02 00:00:07 2017 --- Day changed Sun Apr 02 2017 20170402 00:00:07-!- noy [~Noy@wesnoth/developer/noy] has joined #wesnoth-dev 20170402 00:09:26-!- trewe [~trewe@2001:8a0:d10f:4301:ab9d:b22c:9847:c6f0] has quit [Quit: quit] 20170402 00:13:51-!- noy [~Noy@wesnoth/developer/noy] has quit [Quit: noy] 20170402 00:24:59-!- noy [~Noy@wesnoth/developer/noy] has joined #wesnoth-dev 20170402 00:26:03-!- noy [~Noy@wesnoth/developer/noy] has quit [Client Quit] 20170402 00:28:05-!- Greg-Boggs [~greg_bogg@c-76-115-139-154.hsd1.or.comcast.net] has quit [Remote host closed the connection] 20170402 00:29:34-!- Greg-Boggs [~greg_bogg@2601:1c2:f00:9780:68a6:da81:bf55:d6d7] has joined #wesnoth-dev 20170402 00:52:52-!- RatArmy_ [~ratarmy@om126161120064.8.openmobile.ne.jp] has quit [Ping timeout: 260 seconds] 20170402 01:00:38-!- Greg-Boggs [~greg_bogg@2601:1c2:f00:9780:68a6:da81:bf55:d6d7] has quit [Remote host closed the connection] 20170402 01:25:24-!- RatArmy [~ratarmy@om126200112070.15.openmobile.ne.jp] has joined #wesnoth-dev 20170402 01:26:22-!- stikonas [~gentoo@wesnoth/translator/stikonas] has quit [Quit: Konversation terminated!] 20170402 01:32:29-!- vultraz_iOS [uid24821@wesnoth/developer/vultraz] has joined #wesnoth-dev 20170402 01:33:38< vultraz_iOS> celticminstrel: i already addressed the copy ctor and comment 20170402 01:52:52< vultraz_iOS> celticminstrel: if travis passes should i merge? 20170402 02:11:24< vultraz_iOS> aww it's not passing 20170402 02:11:36< vultraz_iOS> *** Error in `./test': double free or corruption (out): 0x00000000019cea10 *** 20170402 02:11:36< vultraz_iOS> unknown location(0): fatal error in "test_formula_callable": signal: SIGABRT (application abort requested) 20170402 02:11:36< vultraz_iOS> /home/travis/build/wesnoth/wesnoth/src/tests/test_formula_core.cpp(112): last checkpoint 20170402 02:11:36< vultraz_iOS> pure virtual method called 20170402 02:11:36< vultraz_iOS> terminate called without an active exception 20170402 02:11:37< vultraz_iOS> unknown location(0): fatal error in "test_formula_where_clause": signal: SIGABRT (application abort requested) 20170402 02:11:37< vultraz_iOS> /home/travis/build/wesnoth/wesnoth/src/tests/test_formula_core.cpp(124): last checkpoint 20170402 02:11:41< vultraz_iOS> :( 20170402 02:15:18< vultraz_iOS> what.... does this even do 20170402 02:15:19< vultraz_iOS> BOOST_CHECK_EQUAL(formula("char.strength").evaluate(p).as_int(), 15); 20170402 02:22:29< celticminstrel> You did address them? I guess I missed it then... 20170402 02:23:35< celticminstrel> That line is basically the Boost.Unit_Test equivalent of assert(formula("char.strength").evaluate(p).as_int() == 15); 20170402 02:24:47< celticminstrel> vultraz_iOS: About the null() vs (null) for to_debug_string(), I mildly prefer the latter, but it's not a big thing. 20170402 02:27:18< celticminstrel> vultraz_iOS: So, you removed that commented version of begin() declared with decltype? 20170402 02:27:48< celticminstrel> And what about the variant copy constructor? As far as I can tell from the PR, it's still there, and you haven't said anything about it. 20170402 02:30:10< celticminstrel> vultraz_iOS: According to this, MSVC2013 only partially supports thread-local storage, so I think it might be better to avoid it... also it causes the Mac build to fail on Travis for some reason. https://msdn.microsoft.com/en-us/library/hh567368.aspx 20170402 02:34:11< celticminstrel> vultraz_iOS: The crash in the unit tests appears to have something to do with variant_callable. 20170402 02:36:52< celticminstrel> vultraz_iOS: If const_formula_callable_ptr is a shared_ptr, that might be the cause of the crash. Try changing it to a bare pointer. 20170402 02:37:19< celticminstrel> (It's dangerous to initialize a shared_ptr with a bare pointer.) 20170402 02:37:35< celticminstrel> (Except for one newly allocated with new.) 20170402 02:38:11< celticminstrel> (It'll typically cause a double-free.) 20170402 02:47:14-!- Greg-Boggs [~greg_bogg@2601:1c2:f00:9780:68a6:da81:bf55:d6d7] has joined #wesnoth-dev 20170402 03:22:51< vultraz_iOS> celticminstrel: I did remove the variant copy ctor~ 20170402 03:22:52< vultraz_iOS> ! 20170402 03:24:36< celticminstrel> Huh, so you did. Why isn't github marking that comment as outdated, then? Oh well... 20170402 03:25:33< vultraz_iOS> I'll try changing the callable storage back to a raw ptr 20170402 03:25:41< vultraz_iOS> it will also address the .get() stuff you disliked 20170402 03:26:19< celticminstrel> The thing is, you can't mix shared_ptr and raw pointer. Either use shared_ptr everywhere, or use raw pointers everywhere. 20170402 03:26:35< celticminstrel> The latter is obviously not ideal. 20170402 03:27:02< celticminstrel> In particular, it might mean that callables are never freed? 20170402 03:27:37< vultraz_iOS> well, I can't use shared_ptr everywhere 20170402 03:27:39< celticminstrel> But, never freeing the callables is better than freeing them twice, which is what's likely to happen if you try to mix shared_ptr and raw pointers. 20170402 03:27:53< celticminstrel> Why can't you use shared_ptr everywhere? 20170402 03:27:59< vultraz_iOS> or I suppose I could.. 20170402 03:28:09< celticminstrel> That was a question, not a "no you're wrong". 20170402 03:28:33< vultraz_iOS> I thought i wouldn't be able to find every usecase, but i guess i can by just changing the ctor and fixing all compiler errors 20170402 03:29:35< vultraz_iOS> so you recommend shared_ptrs just to ensure the callables are always freed? 20170402 03:30:06< vultraz_iOS> i guess there's no other way to do it anyway without the crappy memory management stuff the old variant class had 20170402 03:30:13< vultraz_iOS> ok, shared_ptrs everywhere it is.. 20170402 03:30:19< vultraz_iOS> inb4 another hour's work 20170402 03:30:32< celticminstrel> Well, shared_ptrs would certainly be a good idea. 20170402 03:30:47< celticminstrel> If you want, you could use a weak_ptr in variant_callable, though. 20170402 03:30:58< vultraz_iOS> weak_ptr? 20170402 03:31:01< vultraz_iOS> what does that do? 20170402 03:31:06< vultraz_iOS> (I've heard of it but never used it) 20170402 03:31:45< celticminstrel> It basically means that it holds a reference to the object but does not own it (and does not increase the reference count), meaning that the object could be destroyed externally... but unlike with raw pointers, that situation can be detected. 20170402 03:32:39< celticminstrel> It seems to me that the original design had variant not owning a contained formula_callable, so weak_ptr might be worth considering... but shared_ptr would also work just fine. 20170402 03:33:50< vultraz_iOS> "std::weak_ptr is a smart pointer that holds a non-owning ("weak") reference to an object that is managed by std::shared_ptr. It must be converted to std::shared_ptr in order to access the referenced object." 20170402 03:33:52< vultraz_iOS> uh... 20170402 03:33:58< vultraz_iOS> I don't think I can use this alone 20170402 03:34:04< celticminstrel> Right. 20170402 03:34:12< celticminstrel> It needs to be used in conjunction with shared_ptr. 20170402 03:34:21< vultraz_iOS> I'd have to convert to shared_ptrs and THEN change variant_callable to a weak_ptr 20170402 03:34:30< celticminstrel> Something like that. 20170402 03:34:47< celticminstrel> I'm not quite sure if it's a good idae. 20170402 03:34:50< celticminstrel> ^idea 20170402 03:35:19< vultraz_iOS> so it looks like I should deploy shared_ptrs anyway 20170402 03:35:34< celticminstrel> Right, you should deploy shared_ptr no matter what. 20170402 03:36:11< vultraz_iOS> btw, if thread_local is causing problems, we should add a global macro for it 20170402 03:37:38< celticminstrel> Okay, it sounds like MSVC doesn't support it. 20170402 03:37:49< celticminstrel> (MSVC 2013, that is.) 20170402 03:38:06< vultraz_iOS> of course it doesn't... 20170402 03:38:11< celticminstrel> It does support __declspec(thread), but it sounds like that wouldn't work in the case where you're using it. 20170402 03:38:24< vultraz_iOS> well we should drop 2013 anyway, but in the meantime I guess macro it is 20170402 03:38:29< celticminstrel> ...wait... what's MSVC 2014 CTP 3? 20170402 03:39:27< celticminstrel> I'm going to test on my own MSVC 2013. 20170402 03:39:29< vultraz_iOS> return std::dynamic_pointer_cast(const_cast>(as_callable())); 20170402 03:39:30< vultraz_iOS> heh 20170402 03:39:34< vultraz_iOS> sometimes tells me this won't build 20170402 03:39:46< vultraz_iOS> something 20170402 03:39:52< celticminstrel> Probably. 20170402 03:40:33< vultraz_iOS> well then what do I do to go from shared_ptr to shared_ptr 20170402 03:40:47< celticminstrel> Okay yeah, no thread_local in MSVC 2013. It does hilite it as a keyword, but that's it. 20170402 03:41:08< celticminstrel> vultraz_iOS: Apparently const_pointer_cast. 20170402 03:41:32< vultraz_iOS> ahhhh 20170402 03:41:40< vultraz_iOS> std library saving us again 20170402 03:44:21< vultraz_iOS> and i assume the variant_callable ctor should take a shared_ptr copy 20170402 03:50:54< vultraz_iOS> well this is already off to a bad start 20170402 03:51:26< vultraz_iOS> cannot make a shared_ptr of formula_callable since it has a pure virtual function 20170402 03:52:07< vultraz_iOS> i suppose i can make it non-pure 20170402 03:52:30< vultraz_iOS> or.. 20170402 03:52:35< vultraz_iOS> wait, no, we don't want make_shared 20170402 03:52:38< vultraz_iOS> nvm 20170402 03:53:15< celticminstrel> If you use std::make_shared it should be with the subclass, not formula_callable. 20170402 03:53:24< vultraz_iOS> is this safe? return variant(const_formula_callable_ptr(this)); 20170402 03:53:29< celticminstrel> eg, std::make_shared(...). 20170402 03:53:41< celticminstrel> That's probably not safe, no. 20170402 03:54:11< celticminstrel> This is the sort of situation where you'd want to use std::enable_shared_from_this, except... that really only works for a class with no subclasses. 20170402 03:54:12< vultraz_iOS> fucking fuck 20170402 03:54:24< vultraz_iOS> why? 20170402 03:54:38< celticminstrel> I'm not entirely clear on the details... 20170402 03:55:32-!- Greg-Boggs [~greg_bogg@2601:1c2:f00:9780:68a6:da81:bf55:d6d7] has quit [Remote host closed the connection] 20170402 03:58:18< vultraz_iOS> plus shared_from_this only works if it's already managed by a shared_ptr 20170402 03:58:42< celticminstrel> Which I thought was going to be the case though. 20170402 03:59:03< vultraz_iOS> not if query_value is called from within formula_callable 20170402 03:59:16< vultraz_iOS> or... 20170402 03:59:20< vultraz_iOS> or no, i guess it would be 20170402 03:59:21< vultraz_iOS> evermind 20170402 04:00:15< celticminstrel> IIRC, there are formula_callables that don't get wrapped in a variant, so you'd need to make sure they're shared_ptrs too. 20170402 04:00:43-!- madgoat [~gk.1wm.su@23.247.147.4] has joined #wesnoth-dev 20170402 04:00:43< vultraz_iOS> so much memory management 20170402 04:00:51-!- madgoat [~gk.1wm.su@23.247.147.4] has left #wesnoth-dev [] 20170402 04:01:56< vultraz_iOS> I think the thing about base classes is it looks like you need to publicly inherit from shared_from_this... 20170402 04:02:10< vultraz_iOS> enable_shared_from_this* 20170402 04:04:22< celticminstrel> I found a stack overflow where they use enable_shared_from_this and static cast to the derived class, but I'm not sure if that's safe... 20170402 04:10:01< vultraz_iOS> SO MUCH variant(new ;_; 20170402 04:11:13< celticminstrel> Okay, so yeah, it sounds like enable_shared_from_this can't be used here. 20170402 04:11:21< celticminstrel> "Specifically, you must derive from enable_shared_from_this exactly once and give it the most derived type of the object." 20170402 04:12:09< vultraz_iOS> well then we CAN'T use shared_ptrs everywhere 20170402 04:12:19< celticminstrel> I dunno. 20170402 04:12:35< vultraz_iOS> maybe I should use rawptr for now... 20170402 04:13:25< celticminstrel> I wonder what would happen if that if statement were simply deleted altogether. 20170402 04:13:25< vultraz_iOS> then explore more comprehensive options 20170402 04:13:32< vultraz_iOS> what if statement? 20170402 04:13:41< celticminstrel> The one where it returns this. 20170402 04:14:02< vultraz_iOS> if(has_self_ && key == "self") { ? 20170402 04:14:06< celticminstrel> Yeah. 20170402 04:14:29< vultraz_iOS> depends 20170402 04:14:32< celticminstrel> I'm a little confused about why "self" is checked for both in formula_callable::query_value and in variant::operatorp[]. 20170402 04:14:35< celticminstrel> ^-p 20170402 04:14:53< vultraz_iOS> only variant::get_member checks for "self" 20170402 04:15:06< celticminstrel> Eh... 20170402 04:15:14< celticminstrel> Ah, right. 20170402 04:15:27< celticminstrel> get_member is for dot access, operator[] is for [] access. 20170402 04:17:01< celticminstrel> query_value is called directly in several places in callable.hpp and formula.cpp... 20170402 04:17:15< celticminstrel> Also in some of the formula functions... 20170402 04:17:25< celticminstrel> Ohhh. 20170402 04:17:49< vultraz_iOS> hm? 20170402 04:17:51< celticminstrel> get_member only checks for "self' after checking if it's a callable (did you change that in your refactor?) 20170402 04:18:06< vultraz_iOS> uhhh 20170402 04:18:09< vultraz_iOS> not that i recall? 20170402 04:18:30< celticminstrel> Yeah, you didn't. 20170402 04:18:31< celticminstrel> Hmmm...3 20170402 04:18:44< vultraz_iOS> I did not 20170402 04:19:59< celticminstrel> The case in lua_formula_bridge probably shouldn't include self... 20170402 04:20:29< vultraz_iOS> just tell me what to do 20170402 04:20:31< celticminstrel> Looks like the cases in function.cpp would need to include self, though... 20170402 04:20:35< celticminstrel> I'm trying to decide. 20170402 04:21:21< vultraz_iOS> query_value("self") is never used 20170402 04:21:40< celticminstrel> Right, but there are cases where a variable is passed to query_value. 20170402 04:23:30< celticminstrel> Ahhh, "self" does need to be handled within formula_callable, because it's a formula_callable that gets passed to evaluate(). 20170402 04:24:32< celticminstrel> I suspect the technique used by enable_shared_from_this would be able to work. 20170402 04:24:44< celticminstrel> But you'd have to implement it manually rather than using enable_shared_from_this. 20170402 04:25:01< celticminstrel> So I guess, either use raw pointers or go back to intrusive_ptr. 20170402 04:25:19< vultraz_iOS> back to intrusive ptr? o-O 20170402 04:26:09< celticminstrel> Well, it would solve the problem of not being able to construct a shared pointer from "this". 20170402 04:26:19< vultraz_iOS> ;_; 20170402 04:27:17< celticminstrel> It's silly to reject intrusive_ptr just because. 20170402 04:27:25< vultraz_iOS> manual refcounting 20170402 04:27:43< celticminstrel> There are advantages to manual refcounting though. 20170402 04:28:18< vultraz_iOS> but it cleaned up the code massively when removed 20170402 04:29:06< celticminstrel> Sure, but that's because the old code was manually calling add_ref in many places. 20170402 04:29:59< celticminstrel> If you use it properly, it's pretty transparent - you just have an extra member variable in the formula_callable, two functions that increment and decrement it, and then you use it pretty much the same as a shared_ptr, except that it's safe to construct one from a raw pointer. 20170402 04:30:46< celticminstrel> (Of course, if you're using it you should still use it everywhere, so that it doesn't end up being deleted by the intrusive_ptr while a raw pointer still holds a reference to it elsewhere.) 20170402 04:31:16-!- nore [~ncourant@sas.eleves.ens.fr] has quit [Ping timeout: 260 seconds] 20170402 04:31:22< celticminstrel> Also, previously it was being used for more than just formula_callable IIRC - it was also used for the lists and maps. 20170402 04:33:02< celticminstrel> I mean, if you look at how it's used for unit_attack, it's really not very messy at all IMO. 20170402 04:33:15< celticminstrel> If you just did the same with formula_callable, again, it wouldn't be very messy at all. 20170402 04:33:22-!- nore [~ncourant@sas.eleves.ens.fr] has joined #wesnoth-dev 20170402 04:34:20< vultraz_iOS> btw, is it supposed to be that shared_from_this won't build in a base class or that it won't work in a base class 20170402 04:34:44< celticminstrel> I'm not 100% sure. 20170402 04:35:05< vultraz_iOS> because ftr it seems to be building 20170402 04:35:47< celticminstrel> Yeah, it would be a runtime problem if anything. 20170402 04:36:18< celticminstrel> Maybe it would be safe if you don't use make_shared? 20170402 04:36:52< celticminstrel> ie, instead of make_shared(arguments), do shared_ptr(new map_formula_callable(arguments)) 20170402 04:37:14< celticminstrel> I'm not entirely sure. 20170402 04:37:16< vultraz_iOS> blah 20170402 04:37:25< vultraz_iOS> ya know, maybe i should just revert to raw pointers fornow 20170402 04:37:32< vultraz_iOS> the old code doesn't actually memory manage it 20170402 04:37:39< celticminstrel> Maybe Jyrki could help more with the question of enable_shared_from_this. 20170402 04:37:43< vultraz_iOS> ie, the variant dtor never deleted it 20170402 04:37:47< celticminstrel> Reverting to raw pointers should be safe, at least. 20170402 04:38:04< celticminstrel> It'll probably result in memory leaks, though. 20170402 04:38:46< vultraz_iOS> wouldn't it have done so before too, though 20170402 04:39:24< celticminstrel> Probably. 20170402 04:39:34< celticminstrel> ISTR someone complaining about memory leaks actually? 20170402 04:40:00< vultraz_iOS> oh 20170402 04:40:01< vultraz_iOS> right 20170402 04:40:10< vultraz_iOS> gfgtdf told me to use shared_ptrs... 20170402 04:40:30< celticminstrel> But shared_ptrs have the problem with the "return this". 20170402 04:45:25< vultraz_iOS> let's revert to raw for now, then we'll deal with the memory leaks 20170402 04:45:28< vultraz_iOS> but i just want the pr merged 20170402 04:48:51< Aginor> would revering to raw pointers actually solve it or does it just use references to freed memory? 20170402 04:49:02< Aginor> (which is a Bad idea) 20170402 04:49:11< Aginor> (hi) 20170402 04:49:59< celticminstrel> IIUC, reverting to raw pointers would mostly mean the memory is never released. 20170402 04:50:07< celticminstrel> Though I'm not entirely sure. 20170402 04:53:47< Aginor> which PR is it? 20170402 04:57:45< vultraz_iOS> Aginor: 967 20170402 04:58:33< vultraz_iOS> celticminstrel: what about adding a custom deleter? 20170402 04:59:27< vultraz_iOS> You say the shared ptr might lead to double free... what about adding a delete that checks if the ptr is not null before deleting it? 20170402 05:00:05< Aginor> that wouldn't work, calling free() on a raw pointer doesn't alter the value of the pointer itself 20170402 05:01:01-!- Kwandulin [~Kwandulin@p200300760F3E7D1AA95C408529EFAC44.dip0.t-ipconnect.de] has joined #wesnoth-dev 20170402 05:02:34-!- noy [~Noy@wesnoth/developer/noy] has joined #wesnoth-dev 20170402 05:03:02-!- JyrkiVesterinen [~JyrkiVest@87-100-157-75.bb.dnainternet.fi] has joined #wesnoth-dev 20170402 05:03:45< vultraz_iOS> :/ 20170402 05:03:54< vultraz_iOS> Damn memory management! 20170402 05:05:08< celticminstrel> Well, you could use a null deleter for the "return this". 20170402 05:05:19< celticminstrel> I'm not sure if that would solve the problem fully, but it would help, at least. 20170402 05:05:31-!- Ravana_ [~Ravana@unaffiliated/ravana/x-2327071] has quit [Ping timeout: 240 seconds] 20170402 05:05:52< Aginor> the memory accesses and management in those parts looks.. scary 20170402 05:06:06< Aginor> what's the old semantics of those raw pointers? 20170402 05:06:19< Aginor> who retains ownership of the pointers? 20170402 05:06:53< celticminstrel> Well, originally the variant held the formula_callable with an intrusive_ptr. 20170402 05:07:06< Aginor> ah... 20170402 05:07:10< celticminstrel> So I guess the variant mostly held ownership? 20170402 05:07:21< vultraz_iOS> Then it was 100% raw pointers most of which are allocated with new 20170402 05:07:26< vultraz_iOS> And never freed 20170402 05:07:27< celticminstrel> Though the code also manually added to the reference count in many places. 20170402 05:08:05< vultraz_iOS> Now I wanted to use shared_ptr but we can't use enable_shared_from_this because it's in a base class 20170402 05:10:16< celticminstrel> Not all formula_callables were owned by a variant though; some were owned by other formula_callables, for example. 20170402 05:10:37< Aginor> sorry, I need to disappear again 20170402 05:10:42< celticminstrel> eg, some formula_callable subclasses store a second formula_callable as a fallback. 20170402 05:11:05< celticminstrel> And it's possible that some formula_callables aren't owned by either thing. 20170402 05:11:18< vultraz_iOS> We need to rethink this 20170402 05:11:19< celticminstrel> By a variant or another formula_callable, I mean. 20170402 05:11:31< celticminstrel> I actually think intrusive_ptr is the simplest solution. 20170402 05:12:00< celticminstrel> Unless it's possible to get enable_shared_from_this to work. 20170402 05:12:06< celticminstrel> Which I'm very wary of. 20170402 05:14:27< vultraz_iOS> I still don't understand why it shouldn't 20170402 05:14:33< vultraz_iOS> It seems like such a design oversight 20170402 05:14:39< vultraz_iOS> That it wouldn't work in base classes 20170402 05:15:36< celticminstrel> It quite possibly is a design oversight. 20170402 05:15:50< celticminstrel> However, I don't fully understand it. 20170402 05:16:07< celticminstrel> There seem to be suggestions that it's possible for it to work. 20170402 05:16:37< vultraz_iOS> We need JyrkiVesterinen 20170402 05:16:39< vultraz_iOS> To weigh in 20170402 05:17:01< celticminstrel> If he knows, sure. 20170402 05:17:26< celticminstrel> If I were doing it I'd probably just go with intrusive_ptr though. :P 20170402 05:17:54< JyrkiVesterinen> I don't know much of boost::intrusive_ptr, unfortunately. 20170402 05:18:08< celticminstrel> JyrkiVesterinen: What about std::enable_shared_from_this? 20170402 05:18:17< JyrkiVesterinen> Same thing. 20170402 05:18:26< celticminstrel> Okay. 20170402 05:20:01< vultraz_iOS> celticminstrel: what about virtual inheritance of all derived classes of formula_callable 20170402 05:20:45< celticminstrel> Pointless. 20170402 05:21:08< vultraz_iOS> http://stackoverflow.com/questions/14939190/boost-shared-from-this-and-multiple-inheritance 20170402 05:21:13< celticminstrel> There's no reason to use virtual inheritance in a single-inheritance class hierarchy. 20170402 05:21:15< celticminstrel> Oh that. 20170402 05:21:28< celticminstrel> But still, it doesn't apply to our case. 20170402 05:21:38< celticminstrel> I actually found that article too. 20170402 05:22:16< celticminstrel> The formula_callable class hierarchy does not use multiple inheritance AFAIK, so that solution is irrelevant. 20170402 05:22:33< celticminstrel> Though the existence of that solution does imply there should exist a solution for us... 20170402 05:23:04< vultraz_iOS> Huh, I guess virtual inheritance is almost useless for my variant callable stuff 20170402 05:23:14< vultraz_iOS> Except for the fact that it's not single-tier 20170402 05:23:17< vultraz_iOS> Er 20170402 05:23:20< vultraz_iOS> Variant value 20170402 05:23:27< vultraz_iOS> So I guess worth it 20170402 05:23:32< celticminstrel> What? 20170402 05:24:19< vultraz_iOS> I use virtual inheritance for the value classes 20170402 05:24:25< celticminstrel> ...oh yeah, there's no point in using virtual inheritance for the variant_value class hierarchy. 20170402 05:24:40< celticminstrel> That's a single-inheritance class hierarchy. 20170402 05:24:54< vultraz_iOS> Except it isn't? 20170402 05:24:57< celticminstrel> Yes it is. 20170402 05:25:02< celticminstrel> Each class inherits from a single base. 20170402 05:25:03< vultraz_iOS> It's two tier in the case of variant container 20170402 05:25:11< celticminstrel> I didn't say single-tier. 20170402 05:25:23< celticminstrel> Single-inheritance means each class has only one direct base class. 20170402 05:25:32< vultraz_iOS> Oh 20170402 05:26:01< celticminstrel> I mean, maybe it's not utterly useless in the presence of single inheritance, but it's intended to be used with multiple inheritance. 20170402 05:26:10< celticminstrel> https://en.wikipedia.org/wiki/Virtual_inheritance 20170402 05:29:49< vultraz_iOS> I see 20170402 05:29:56< vultraz_iOS> Ok I misunderstood what that's for 20170402 05:33:05-!- boucman [~rosen@wesnoth/developer/boucman] has joined #wesnoth-dev 20170402 05:36:55< celticminstrel> I have this suspicion that enable_shared_from_this might be safe if you never construct a shared pointer to a derived type... but casting the pointer to the derived type might count as constructing a shared pointer to a derived type... 20170402 05:38:45< celticminstrel> What I mean is, like I said earlier, don't use make_shared; instead use a function like template std::shared_ptr make_callable(Args... args) {return shared_ptr(new T(args...));} 20170402 05:39:00< celticminstrel> I'm not confident that this is safe though. 20170402 05:47:13< vultraz_iOS> bbbbbbballlllllllllghhhhhhhhhhhhhhhhh 20170402 05:49:17< celticminstrel> :P 20170402 05:52:20< vultraz_iOS> raw it is 20170402 05:52:22< vultraz_iOS> so i can merge 20170402 05:52:26< vultraz_iOS> and then we'll redesign everything 20170402 05:52:33< celticminstrel> "everything" 20170402 05:52:47< vultraz_iOS> some of the things 20170402 05:53:03< vultraz_iOS> you'll probably have to rebase here 20170402 05:53:08< vultraz_iOS> once i merge 20170402 05:53:15< celticminstrel> Probably. 20170402 05:53:24< celticminstrel> Though I suspect there will be few or no conflicts. 20170402 05:53:57< vultraz_iOS> I'll probably do further refactoring of variant later but for now at least it's much cleaner 20170402 05:54:16< celticminstrel> I'll take your word for it? 20170402 05:54:31< celticminstrel> I don't have any problems with your implementation though. 20170402 05:54:38< vultraz_iOS> well that's good 20170402 05:54:42< vultraz_iOS> i expected you would have 20170402 05:54:57< celticminstrel> It's not the way I would have done it, but it seems quite workable to me. 20170402 05:55:04< vultraz_iOS> but since both you and jyrki like it it must be good 20170402 06:04:46-!- mjs-de [~mjs-de@x4e305112.dyn.telefonica.de] has joined #wesnoth-dev 20170402 06:05:54-!- boucman [~rosen@wesnoth/developer/boucman] has quit [Remote host closed the connection] 20170402 06:11:29< vultraz_iOS> rebased and force pushed to my fork. now let's see what travis says. 20170402 06:25:10-!- Elsi_ [~Elsi@luwin.ulrar.net] has quit [Ping timeout: 258 seconds] 20170402 06:38:32-!- Appleman1234 [~Appleman1@pl19584.ag1212.nttpc.ne.jp] has quit [Ping timeout: 268 seconds] 20170402 06:38:56-!- Ravana_ [~Ravana@unaffiliated/ravana/x-2327071] has joined #wesnoth-dev 20170402 06:42:25-!- celticminstrel is now known as celmin|ZzzZzzz 20170402 06:52:38-!- Appleman1234 [~Appleman1@pl19787.ag1212.nttpc.ne.jp] has joined #wesnoth-dev 20170402 06:53:05-!- Ravana_ [~Ravana@unaffiliated/ravana/x-2327071] has quit [Ping timeout: 240 seconds] 20170402 06:55:35< vultraz_iOS> celmin|ZzzZzzz: looks like thread_local is only not supported in that version of clang travis builds the max build one 20170402 06:55:36< vultraz_iOS> on* 20170402 06:55:40< vultraz_iOS> https://stackoverflow.com/questions/28094794/why-does-apple-clang-disallow-c11-thread-local-when-official-clang-supports 20170402 06:56:19-!- irker540 [~irker@uruz.ai0867.net] has joined #wesnoth-dev 20170402 06:56:19< irker540> wesnoth: Charles Dang wesnoth:master 99874f425378 / / (22 files in 10 dirs): Refactor variant class https://github.com/wesnoth/wesnoth/commit/99874f4253788624281e1bd1778c299995813448 20170402 06:56:19< irker540> wesnoth: Charles Dang wesnoth:master 3f10883c456f / src/ (11 files in 5 dirs): Made use of emplace_back for variant creation https://github.com/wesnoth/wesnoth/commit/3f10883c456fa1f26ef1e1e2272e607c0ed07345 20170402 06:56:20< irker540> wesnoth: Charles Dang wesnoth:master 29f04bdfc2ad / src/ (4 files in 2 dirs): Removed external variant conversion functions in favor of in-class ones. https://github.com/wesnoth/wesnoth/commit/29f04bdfc2ad336d12364c912ba0998cba67f770 20170402 06:56:23< irker540> wesnoth: Charles Dang wesnoth:master 00a987369d31 / src/formula/ (variant.hpp variant_private.cpp variant_private.hpp): Removed 'mutable' member of variant_callable https://github.com/wesnoth/wesnoth/commit/00a987369d31ac27cda0d6318fc1305aac17c80e 20170402 06:56:26< irker540> wesnoth: Charles Dang wesnoth:master ccadd7625efa / src/formula/variant.cpp: Fixed nullptr access if variant is of type null https://github.com/wesnoth/wesnoth/commit/ccadd7625eface6ecdd1b46cfa5ecbc3a127c49f 20170402 06:56:29< irker540> wesnoth: Charles Dang wesnoth:master 4d7fd5fa1c85 / src/ai/formula/candidates.cpp: Use variant::try_convert when possible https://github.com/wesnoth/wesnoth/commit/4d7fd5fa1c85c0a58f5ff4dd116d8b9c057038af 20170402 06:56:32< irker540> wesnoth: Charles Dang wesnoth:master d3b85060d01a / src/formula/variant_private.hpp: Added/improved variant value documentation https://github.com/wesnoth/wesnoth/commit/d3b85060d01a2020fbc5de0e508d648f467cd41b 20170402 06:56:35< irker540> wesnoth: Charles Dang wesnoth:master 5b5013547474 / src/tests/test_formula_core.cpp: Attempt to fixup tests https://github.com/wesnoth/wesnoth/commit/5b5013547474a902813489f28f3390cbaf8098c6 20170402 06:56:38< irker540> wesnoth: Charles Dang wesnoth:master 725bd6291f33 / src/ (8 files in 2 dirs): Refactored handling of the callable variant debug stack https://github.com/wesnoth/wesnoth/commit/725bd6291f33928d625535f644fe0b1e88774090 20170402 06:56:41< irker540> wesnoth: Charles Dang wesnoth:master c5de7fd50e3a / src/formula/ (variant.cpp variant.hpp variant_private.hpp): Address a bunch of critique from the feedback Minstrel https://github.com/wesnoth/wesnoth/commit/c5de7fd50e3ad3a6ca6ab771c8a050897a5d95ae 20170402 06:56:44< irker540> wesnoth: Charles Dang wesnoth:master dec492d6dc9d / / (7 files in 3 dirs): Renamed variable_private.*pp to better reflect its role https://github.com/wesnoth/wesnoth/commit/dec492d6dc9d5f7d3750d42605a744fd32da25f5 20170402 06:56:47< irker540> wesnoth: Charles Dang wesnoth:master 7cab3f29a3d2 / src/formula/variant.cpp: Minor cleanup to variant error reporting https://github.com/wesnoth/wesnoth/commit/7cab3f29a3d2bc1b35b9ea860a40a1745bbcfba3 20170402 06:56:50< irker540> wesnoth: Charles Dang wesnoth:master 978c8d34199f / src/formula/variant_value.cpp: Attempt to fix tests again https://github.com/wesnoth/wesnoth/commit/978c8d34199f3ea26b30033fdcb307965e3d1e60 20170402 06:56:53< irker540> wesnoth: Charles Dang wesnoth:master 9fe45ce42f70 / src/ (5 files in 2 dirs): Revert variant_callable handling to raw pointers temporarily https://github.com/wesnoth/wesnoth/commit/9fe45ce42f70db3245253ef8ff1380f445f47ba7 20170402 07:03:52< JyrkiVesterinen> Then we should do something about it. 20170402 07:04:04< JyrkiVesterinen> All Travis builds are going to fail because of it. 20170402 07:05:09< JyrkiVesterinen> And it's no longer possible to build the game with Xcode 7 or earlier, which also means that compiling with Xcode requires at least OS X 10.11. 20170402 07:06:08< JyrkiVesterinen> https://wiki.wesnoth.org/CompilingWesnothOnMacOSX#Build_using_XCode says that building with Xcode used to be supported with OS X versions down to 10.6. 20170402 07:15:49-!- Appleman1234 [~Appleman1@pl19787.ag1212.nttpc.ne.jp] has left #wesnoth-dev ["Leaving"] 20170402 07:16:01-!- Appleman1234 [~Appleman1@pl19787.ag1212.nttpc.ne.jp] has joined #wesnoth-dev 20170402 07:41:28-!- ChipmunkV [~vova@static-89-94-113-91.axione.abo.bbox.fr] has joined #wesnoth-dev 20170402 07:48:16-!- travis-ci [~travis-ci@ec2-107-20-121-44.compute-1.amazonaws.com] has joined #wesnoth-dev 20170402 07:48:17< travis-ci> wesnoth/wesnoth#13115 (master - 9fe45ce : Charles Dang): The build was broken. 20170402 07:48:17< travis-ci> Build details : https://travis-ci.org/wesnoth/wesnoth/builds/217705498 20170402 07:48:17-!- travis-ci [~travis-ci@ec2-107-20-121-44.compute-1.amazonaws.com] has left #wesnoth-dev [] 20170402 08:20:16-!- atarocch [~atarocch@151.41.104.21] has joined #wesnoth-dev 20170402 08:23:53-!- Kwandulin [~Kwandulin@p200300760F3E7D1AA95C408529EFAC44.dip0.t-ipconnect.de] has quit [Read error: Connection reset by peer] 20170402 08:42:41-!- atarocch [~atarocch@151.41.104.21] has quit [Remote host closed the connection] 20170402 08:43:32-!- zookeeper [~lmsnie@wesnoth/developer/zookeeper] has joined #wesnoth-dev 20170402 08:49:20< vultraz_iOS> guess we need that macro, then 20170402 08:50:25< vultraz_iOS> JyrkiVesterinen: not "all the builds" are failing, though, only the mac one 20170402 08:53:08< JyrkiVesterinen> I meant that Travis will mark all the *commits* as failing to build. 20170402 08:53:11-!- ChipmunkV [~vova@static-89-94-113-91.axione.abo.bbox.fr] has quit [Remote host closed the connection] 20170402 08:53:41-!- ChipmunkV [~vova@static-89-94-113-91.axione.abo.bbox.fr] has joined #wesnoth-dev 20170402 08:54:46< vultraz_iOS> oh 20170402 09:29:05-!- Kwandulin [~Kwandulin@p200300760F3E7D1A4428774DA0D2280F.dip0.t-ipconnect.de] has joined #wesnoth-dev 20170402 09:33:00-!- vincent_c [~bip@vcheng.org] has quit [Quit: Coyote finally caught me] 20170402 09:33:45-!- vincent_c [~bip@vcheng.org] has joined #wesnoth-dev 20170402 09:57:58-!- irker540 [~irker@uruz.ai0867.net] has quit [Quit: transmission timeout] 20170402 10:22:50-!- stikonas [~gentoo@wesnoth/translator/stikonas] has joined #wesnoth-dev 20170402 10:29:10-!- JyrkiVesterinen [~JyrkiVest@87-100-157-75.bb.dnainternet.fi] has quit [Quit: .] 20170402 10:35:10-!- irker302 [~irker@uruz.ai0867.net] has joined #wesnoth-dev 20170402 10:35:10< irker302> wesnoth: Matthias Krüger wesnoth:master cc1f33a32bac / src/formula/variant.cpp: resolve -Wunused-lambda-capture warning (clang) https://github.com/wesnoth/wesnoth/commit/cc1f33a32bac3db5efaaa579dd7e13ee239a3036 20170402 10:35:44-!- Duthlet [~Duthlet@dslb-188-106-146-119.188.106.pools.vodafone-ip.de] has joined #wesnoth-dev 20170402 10:36:38-!- Shiki [~Shiki@p54856413.dip0.t-ipconnect.de] has joined #wesnoth-dev 20170402 11:16:54-!- Ravana_ [~Ravana@unaffiliated/ravana/x-2327071] has joined #wesnoth-dev 20170402 11:23:51-!- stikonas [~gentoo@wesnoth/translator/stikonas] has quit [Remote host closed the connection] 20170402 11:24:23-!- stikonas [~gentoo@wesnoth/translator/stikonas] has joined #wesnoth-dev 20170402 11:29:20-!- travis-ci [~travis-ci@ec2-54-196-106-137.compute-1.amazonaws.com] has joined #wesnoth-dev 20170402 11:29:21< travis-ci> wesnoth/wesnoth#13117 (master - cc1f33a : Matthias Krüger): The build is still failing. 20170402 11:29:21< travis-ci> Build details : https://travis-ci.org/wesnoth/wesnoth/builds/217745631 20170402 11:29:21-!- travis-ci [~travis-ci@ec2-54-196-106-137.compute-1.amazonaws.com] has left #wesnoth-dev [] 20170402 11:30:09-!- Shiki_ [~Shiki@p548544DC.dip0.t-ipconnect.de] has joined #wesnoth-dev 20170402 11:31:08-!- Shiki [~Shiki@p54856413.dip0.t-ipconnect.de] has quit [Ping timeout: 240 seconds] 20170402 12:16:18-!- RatArmy [~ratarmy@om126200112070.15.openmobile.ne.jp] has quit [Read error: Connection reset by peer] 20170402 12:18:38-!- RatArmy [~ratarmy@om126200112070.15.openmobile.ne.jp] has joined #wesnoth-dev 20170402 12:37:10-!- JyrkiVesterinen [~JyrkiVest@87-100-157-75.bb.dnainternet.fi] has joined #wesnoth-dev 20170402 12:49:11< JyrkiVesterinen> https://msdn.microsoft.com/en-us/library/9w1sdazb(v=vs.120).aspx 20170402 12:49:22< JyrkiVesterinen> "Only POD classes may be instantiated using __declspec(thread)." 20170402 12:49:57< JyrkiVesterinen> Bummer. The seen_stack object itself can't be thread local in MSVC2013 builds. :( 20170402 12:53:19< JyrkiVesterinen> I'm going to revert commit 725bd629. It will deal with problems with both MCVS2013 and Xcode. 20170402 13:14:06-!- mattsc [~mattsc@wesnoth/developer/mattsc] has joined #wesnoth-dev 20170402 13:17:45-!- vultraz_iOS [uid24821@wesnoth/developer/vultraz] has quit [Quit: Connection closed for inactivity] 20170402 13:35:06-!- irker302 [~irker@uruz.ai0867.net] has quit [Quit: transmission timeout] 20170402 13:36:48-!- irker012 [~irker@uruz.ai0867.net] has joined #wesnoth-dev 20170402 13:36:48< irker012> wesnoth: Jyrki Vesterinen wesnoth:master 2143741617b0 / projectfiles/VC12/ (wesnoth.vcxproj wesnoth.vcxproj.filters): Update Visual Studio project https://github.com/wesnoth/wesnoth/commit/2143741617b0fca9959960be6f89474ff2615265 20170402 13:36:48< irker012> wesnoth: Jyrki Vesterinen wesnoth:master a37ce62f3047 / src/ (8 files in 2 dirs): Revert "Refactored handling of the callable variant debug stack" https://github.com/wesnoth/wesnoth/commit/a37ce62f304774e18259c1cb6e30ae32a3526f7e 20170402 13:36:49< irker012> wesnoth: Jyrki Vesterinen wesnoth:master 27817d2f4add / src/formula/ (variant_value.cpp variant_value.hpp): Fix build with Visual Studio https://github.com/wesnoth/wesnoth/commit/27817d2f4add8530e2d09ec7a2053eaea0685eea 20170402 13:44:22-!- ChipmunkV [~vova@static-89-94-113-91.axione.abo.bbox.fr] has left #wesnoth-dev [] 20170402 13:44:46-!- ChipmunkV[m] [chipmunkvm@gateway/shell/matrix.org/x-qleptctfwugwubxj] has joined #wesnoth-dev 20170402 13:45:04-!- Kwandulin [~Kwandulin@p200300760F3E7D1A4428774DA0D2280F.dip0.t-ipconnect.de] has quit [Read error: Connection reset by peer] 20170402 14:29:57-!- Kwandulin [~Kwandulin@p200300760F3E7D1A0D31162A329E700A.dip0.t-ipconnect.de] has joined #wesnoth-dev 20170402 14:42:21-!- travis-ci [~travis-ci@ec2-54-145-62-232.compute-1.amazonaws.com] has joined #wesnoth-dev 20170402 14:42:22< travis-ci> wesnoth/wesnoth#13118 (master - 27817d2 : Jyrki Vesterinen): The build was fixed. 20170402 14:42:22< travis-ci> Build details : https://travis-ci.org/wesnoth/wesnoth/builds/217782353 20170402 14:42:22-!- travis-ci [~travis-ci@ec2-54-145-62-232.compute-1.amazonaws.com] has left #wesnoth-dev [] 20170402 14:45:57-!- mjs-de [~mjs-de@x4e305112.dyn.telefonica.de] has quit [Ping timeout: 240 seconds] 20170402 14:53:12< irker012> wesnoth: mattsc wesnoth:master 35917891e925 / projectfiles/Xcode/Wesnoth.xcodeproj/project.pbxproj: Update Xcode project https://github.com/wesnoth/wesnoth/commit/35917891e925056d5963a75ffaad200bb53f6e08 20170402 14:59:35-!- bumbadadabum [~bumbadada@wesnoth/developer/bumbadadabum] has quit [Read error: Connection reset by peer] 20170402 14:59:41-!- bumba [~bumbadada@wesnoth/developer/bumbadadabum] has joined #wesnoth-dev 20170402 15:20:31-!- Greg-Boggs [~greg_bogg@c-76-115-139-154.hsd1.or.comcast.net] has joined #wesnoth-dev 20170402 15:46:23-!- Greg-Boggs [~greg_bogg@c-76-115-139-154.hsd1.or.comcast.net] has quit [Remote host closed the connection] 20170402 15:48:18-!- Greg-Boggs [~greg_bogg@2601:1c2:f00:9780:68a6:da81:bf55:d6d7] has joined #wesnoth-dev 20170402 15:51:02-!- Greg-Boggs [~greg_bogg@2601:1c2:f00:9780:68a6:da81:bf55:d6d7] has quit [Remote host closed the connection] 20170402 15:52:01-!- Greg-Boggs [~greg_bogg@2601:1c2:f00:9780:68a6:da81:bf55:d6d7] has joined #wesnoth-dev 20170402 15:52:01-!- Greg-Boggs [~greg_bogg@2601:1c2:f00:9780:68a6:da81:bf55:d6d7] has quit [Remote host closed the connection] 20170402 15:52:13-!- Greg-Boggs [~greg_bogg@2601:1c2:f00:9780:68a6:da81:bf55:d6d7] has joined #wesnoth-dev 20170402 15:58:44-!- bumba [~bumbadada@wesnoth/developer/bumbadadabum] has quit [Read error: Connection reset by peer] 20170402 15:58:50-!- bumbadadabum [~bumbadada@wesnoth/developer/bumbadadabum] has joined #wesnoth-dev 20170402 16:01:15-!- noy [~Noy@wesnoth/developer/noy] has quit [Quit: noy] 20170402 16:03:31< celmin|ZzzZzzz> ...why do I get C4189 in impl_end_level_data_collect? Surely a manual destructor call counts as referencing it. 20170402 16:04:23-!- Greg-Boggs [~greg_bogg@2601:1c2:f00:9780:68a6:da81:bf55:d6d7] has quit [Remote host closed the connection] 20170402 16:04:27-!- atarocch [~atarocch@93.56.160.28] has joined #wesnoth-dev 20170402 16:04:53< irker012> wesnoth: Celtic Minstrel wesnoth:master d2caa16515d7 / src/ (8 files in 2 dirs): Swap the arguments to variant::to_debug_string https://github.com/wesnoth/wesnoth/commit/d2caa16515d7238ba0a17080db68327e83229c8b 20170402 16:05:06-!- mjs-de [~mjs-de@x4e305112.dyn.telefonica.de] has joined #wesnoth-dev 20170402 16:05:14< celmin|ZzzZzzz> Still need to deal with the formula callable pointers. 20170402 16:05:21< celmin|ZzzZzzz> Either make them shared or intrusive pointers. 20170402 16:05:39-!- celmin|ZzzZzzz is now known as celticminstrel 20170402 16:06:19< irker012> wesnoth: Celtic Minstrel wesnoth:wfl_action_callable dcaad9dc1df9 / src/ (11 files in 3 dirs): WIP Generalizing the FAI concept of action objects https://github.com/wesnoth/wesnoth/commit/dcaad9dc1df9bda10bf36e6bfe40990b6324aaf7 20170402 16:21:57< celticminstrel> PR966 doesn't seem to have a Travis build for some reason... 20170402 16:25:52< celticminstrel> Oh right, I forgot it had [ci skip]. 20170402 16:29:46< irker012> wesnoth: Celtic Minstrel wesnoth:wfl_action_callable 5ddd32b152ee / src/ (11 files in 3 dirs): Generalizing the FAI concept of action objects https://github.com/wesnoth/wesnoth/commit/5ddd32b152ee5cb377a30312cda360c63883abe8 20170402 16:35:34-!- boucman [~rosen@wesnoth/developer/boucman] has joined #wesnoth-dev 20170402 16:45:43-!- Kwandulin [~Kwandulin@p200300760F3E7D1A0D31162A329E700A.dip0.t-ipconnect.de] has quit [Read error: Connection reset by peer] 20170402 16:51:48-!- Greg-Boggs [~greg_bogg@2601:1c2:f00:9780:68a6:da81:bf55:d6d7] has joined #wesnoth-dev 20170402 16:57:31-!- travis-ci [~travis-ci@ec2-107-20-121-44.compute-1.amazonaws.com] has joined #wesnoth-dev 20170402 16:57:32< travis-ci> wesnoth/wesnoth#13122 (wfl_action_callable - 5ddd32b : Celtic Minstrel): The build failed. 20170402 16:57:32< travis-ci> Build details : https://travis-ci.org/wesnoth/wesnoth/builds/217816232 20170402 16:57:32-!- travis-ci [~travis-ci@ec2-107-20-121-44.compute-1.amazonaws.com] has left #wesnoth-dev [] 20170402 17:03:48-!- RatArmy [~ratarmy@om126200112070.15.openmobile.ne.jp] has quit [Read error: Connection reset by peer] 20170402 17:03:53-!- Kwandulin [~Kwandulin@p200300760F3E7D1A6DEADCDB7173E6A0.dip0.t-ipconnect.de] has joined #wesnoth-dev 20170402 17:22:02-!- vultraz_iOS [uid24821@wesnoth/developer/vultraz] has joined #wesnoth-dev 20170402 17:22:33< vultraz_iOS> we could have just not made it thread_local on 2013 :| 20170402 17:28:32< vultraz_iOS> whatever, it doesnt matter 20170402 17:30:21-!- ToBeFree [uid51591@wikimedia/ToBeFree] has joined #wesnoth-dev 20170402 17:34:22< vultraz_iOS> (I guess there was no simple way to solve the mac build...) 20170402 17:39:37< irker012> wesnoth: Charles Dang wesnoth:master 0af91c4990e5 / src/formula/variant_value.hpp: Avoid unnecessary copying when fetching variant value type https://github.com/wesnoth/wesnoth/commit/0af91c4990e5b839d85b2899c3e9e03dfc28d9a9 20170402 17:40:50< JyrkiVesterinen> Then it would have been thread-safe in GCC, upstream Clang, MSVC >=2015 and Xcode >= 8 builds, and thread-unsafe in MSVC2013 and Xcode <= 7 builds. 20170402 17:41:20< JyrkiVesterinen> Worst of all, multithreading bugs are often intermittent and extremely hard to investigate. 20170402 17:41:33< JyrkiVesterinen> That option sounds outright horrible to me. 20170402 17:42:02< vultraz_iOS> when you put it that way, true 20170402 17:47:33 * vultraz_iOS ponders method to prevent rebuilding all of gui2 when modifying the variant code 20170402 17:49:15-!- Greg-Boggs [~greg_bogg@2601:1c2:f00:9780:68a6:da81:bf55:d6d7] has quit [Remote host closed the connection] 20170402 17:52:32< vultraz_iOS> ehh nope 20170402 17:53:16< vultraz_iOS> map_formula_callable needs to be complete in the canvas which needs to be complete in widget_definition which means All The Rebuilding :( 20170402 17:56:50< vultraz_iOS> and i assume T needs to be complete in a vector of T 20170402 18:03:50-!- Greg-Boggs [~greg_bogg@2601:1c2:f00:9780:68a6:da81:bf55:d6d7] has joined #wesnoth-dev 20170402 18:03:50< vultraz_iOS> celticminstrel: ok, so, from where do you want this stringstream argument to originally come? 20170402 18:07:45< vultraz_iOS> for serialize, that is 20170402 18:08:49< celticminstrel> Making it not thread_local in MSVC2013 does not strike me as a good idea. 20170402 18:09:30< celticminstrel> TBH, I think MAKE_ENUM types are sufficiently trivial that copying isn't particularly expensive. 20170402 18:09:48< celticminstrel> IIUC, they only contain a single data member. 20170402 18:10:07< celticminstrel> But I guess it doesn't hurt to take them by reference. 20170402 18:11:59< celticminstrel> Yes, T needs to be complete in a vector of T, why did you ask? 20170402 18:12:16< celticminstrel> I guess the stringstream argument would work the same way the seen stack does... 20170402 18:12:42< celticminstrel> Actually no, you could just take it as a parameter and force the caller to provide one. 20170402 18:12:53< celticminstrel> And then return the string stream when done with it. 20170402 18:13:06< celticminstrel> I think that would allow callers to do something like... 20170402 18:13:17< celticminstrel> ...oh right. 20170402 18:14:12< celticminstrel> If it takes an ostream as a parameter, callers can simply do v.to_debug_string(std::cout) or something. 20170402 18:14:31< celticminstrel> I guess v.to_debug_string(LOG_SF) rather. 20170402 18:15:11< celticminstrel> So yeah, I think it makes most sense for the caller to provide the stream. And no reason to force it to be a stringstream. 20170402 18:16:57-!- Greg-Boggs [~greg_bogg@2601:1c2:f00:9780:68a6:da81:bf55:d6d7] has quit [Read error: Connection reset by peer] 20170402 18:17:26< irker012> wesnoth: Celtic Minstrel wesnoth:wfl_action_callable d6851d92d009 / src/ (11 files in 3 dirs): Generalizing the FAI concept of action objects https://github.com/wesnoth/wesnoth/commit/d6851d92d0095a46ecb0683d88a07040a2b34d8e 20170402 18:17:38-!- Greg-Boggs [~greg_bogg@2601:1c2:f00:9780:68a6:da81:bf55:d6d7] has joined #wesnoth-dev 20170402 18:22:16-!- Greg-Boggs [~greg_bogg@2601:1c2:f00:9780:68a6:da81:bf55:d6d7] has quit [Ping timeout: 246 seconds] 20170402 18:25:08-!- Greg-Boggs [~greg_bogg@2601:1c2:f00:9780:68a6:da81:bf55:d6d7] has joined #wesnoth-dev 20170402 18:33:02-!- gfgtdf [~chatzilla@x4e368cad.dyn.telefonica.de] has joined #wesnoth-dev 20170402 18:33:32-!- Greg-Boggs [~greg_bogg@2601:1c2:f00:9780:68a6:da81:bf55:d6d7] has quit [Remote host closed the connection] 20170402 18:35:05-!- Greg-Boggs [~greg_bogg@2601:1c2:f00:9780:68a6:da81:bf55:d6d7] has joined #wesnoth-dev 20170402 18:41:10< celticminstrel> So, gfgtdf, JyrkiVesterinen, et al... any comments about whether the commented-out bits of code in PR966 need to be restored somehow? That's the following four locations: 20170402 18:41:10< celticminstrel> https://github.com/wesnoth/wesnoth/pull/966/files#diff-810f6f7d11d196f91db0bbea759cb8dfR379 20170402 18:41:12< celticminstrel> https://github.com/wesnoth/wesnoth/pull/966/files#diff-810f6f7d11d196f91db0bbea759cb8dfR398 20170402 18:41:13< celticminstrel> https://github.com/wesnoth/wesnoth/pull/966/files#diff-4c938a60a9fa7ea36977dbfdab523a24R688 20170402 18:41:15< celticminstrel> https://github.com/wesnoth/wesnoth/pull/966/files#diff-e1ecf4d337fd215955660c8b8d9558adR90 20170402 18:41:16< celticminstrel> (I think those four are all of them.) 20170402 18:41:41< celticminstrel> (The last is the declaration of the class that the first three make use of.) 20170402 18:42:19< celticminstrel> (Oh right, its implementations in ai.cpp are also commented out with #if 0) 20170402 18:44:12< JyrkiVesterinen> I don't have any feedback here. Not really an expert on FormulaAI. 20170402 18:45:15-!- markus_ [~mjs-de@x4db6c429.dyn.telefonica.de] has joined #wesnoth-dev 20170402 18:46:11-!- Shiki_ [~Shiki@p548544DC.dip0.t-ipconnect.de] has quit [Remote host closed the connection] 20170402 18:48:50-!- mjs-de [~mjs-de@x4e305112.dyn.telefonica.de] has quit [Ping timeout: 258 seconds] 20170402 18:52:04-!- Greg-Boggs [~greg_bogg@2601:1c2:f00:9780:68a6:da81:bf55:d6d7] has quit [Remote host closed the connection] 20170402 19:19:34< irker012> wesnoth: Celtic Minstrel wesnoth:wfl_action_callable 7c5411cedc11 / src/ (11 files in 3 dirs): Generalizing the FAI concept of action objects https://github.com/wesnoth/wesnoth/commit/7c5411cedc11493548e13d812001693aacb1aea6 20170402 19:20:34-!- travis-ci [~travis-ci@ec2-54-145-62-232.compute-1.amazonaws.com] has joined #wesnoth-dev 20170402 19:20:35< travis-ci> wesnoth/wesnoth#13125 (wfl_action_callable - d6851d9 : Celtic Minstrel): The build is still failing. 20170402 19:20:35< travis-ci> Build details : https://travis-ci.org/wesnoth/wesnoth/builds/217839033 20170402 19:20:35-!- travis-ci [~travis-ci@ec2-54-145-62-232.compute-1.amazonaws.com] has left #wesnoth-dev [] 20170402 19:26:18< irker012> wesnoth: Matthias Krüger wesnoth:master f6800c60251f / src/floating_label.cpp: floating label: remove dead code. Found by cppcheck. https://github.com/wesnoth/wesnoth/commit/f6800c60251f38de7a6a18d5893918046791ff55 20170402 19:26:56< celticminstrel> I wasn't convinced that removing it was the right option, but whatever... it's not like I had alternate ideas. 20170402 19:28:48< celticminstrel> Ah, gfgtdf did weigh in on it though, I see. 20170402 19:41:33-!- Kwandulin [~Kwandulin@p200300760F3E7D1A6DEADCDB7173E6A0.dip0.t-ipconnect.de] has quit [Quit: Miranda IM! Smaller, Faster, Easier. http://miranda-im.org] 20170402 19:52:19-!- janebot [~Gambot@unaffiliated/gambit/bot/gambot] has quit [Remote host closed the connection] 20170402 19:52:25-!- janebot [~Gambot@unaffiliated/gambit/bot/gambot] has joined #wesnoth-dev 20170402 19:59:22< irker012> wesnoth: Celtic Minstrel wesnoth:wfl_action_callable 6366b13e5902 / src/ (4 files in 2 dirs): Implement WFL function symbol table inheritance https://github.com/wesnoth/wesnoth/commit/6366b13e590208c4298efefefa6092ecce2b647e 20170402 20:00:01< celticminstrel> vultraz_iOS: ^ 20170402 20:00:25< vultraz_iOS> Hm? 20170402 20:00:28< celticminstrel> Basically, all you need to support actions in GUI2 with that is: 20170402 20:00:49< celticminstrel> 1. Use action_function_symbol_table instead of function_symbol_table whenever instantiating them in GUI2. 20170402 20:01:17< celticminstrel> 2. In appropriate contexts, use the variant::execute_variant() function on the result of formulas. 20170402 20:01:33< celticminstrel> For example, you might add an action= key to canvas shapes. 20170402 20:02:14< celticminstrel> It would be evaluated in the context of the shape, but executed in the context of the canvas. 20170402 20:02:43< celticminstrel> Therefore, the use-case you wanted would be roughly as simple as action="set_var('base_rect', base_rect)" 20170402 20:02:46-!- markus_ [~mjs-de@x4db6c429.dyn.telefonica.de] has quit [Remote host closed the connection] 20170402 20:02:59< celticminstrel> I say "roughly" because there's currently no rect type. 20170402 20:03:11< celticminstrel> One could be trivially added though. 20170402 20:14:48-!- noy [~Noy@wesnoth/developer/noy] has joined #wesnoth-dev 20170402 20:21:21-!- ToBeFree [uid51591@wikimedia/ToBeFree] has quit [Quit: Connection closed for inactivity] 20170402 20:24:43-!- Greg-Boggs [~greg_bogg@2601:1c2:f00:9780:68a6:da81:bf55:d6d7] has joined #wesnoth-dev 20170402 20:29:04-!- noy [~Noy@wesnoth/developer/noy] has quit [Read error: Connection reset by peer] 20170402 20:30:23< celticminstrel> Waiting for Travis to pass, and then I need to remember how to tell MSVC to pass command-line arguments so I can do a quick test. 20170402 20:30:51-!- noy [~Noy@wesnoth/developer/noy] has joined #wesnoth-dev 20170402 20:36:19-!- StandYourGround [~StandYour@2602:30a:c02f:d050:acb5:6b34:9ea5:be13] has joined #wesnoth-dev 20170402 20:36:39-!- StandYourGround [~StandYour@2602:30a:c02f:d050:acb5:6b34:9ea5:be13] has quit [Client Quit] 20170402 20:39:37-!- boucman [~rosen@wesnoth/developer/boucman] has quit [Remote host closed the connection] 20170402 20:54:20-!- ToBeFree [uid51591@wikimedia/ToBeFree] has joined #wesnoth-dev 20170402 20:59:19-!- noy [~Noy@wesnoth/developer/noy] has quit [Read error: Connection reset by peer] 20170402 21:06:09-!- noy [~Noy@wesnoth/developer/noy] has joined #wesnoth-dev 20170402 21:14:43-!- JyrkiVesterinen [~JyrkiVest@87-100-157-75.bb.dnainternet.fi] has quit [Quit: .] 20170402 21:19:59-!- bumbadadabum [~bumbadada@wesnoth/developer/bumbadadabum] has quit [Read error: Connection reset by peer] 20170402 21:20:05-!- bumba [~bumbadada@wesnoth/developer/bumbadadabum] has joined #wesnoth-dev 20170402 21:29:16-!- noy [~Noy@wesnoth/developer/noy] has quit [Read error: Connection reset by peer] 20170402 21:29:46-!- noy [~Noy@wesnoth/developer/noy] has joined #wesnoth-dev 20170402 21:37:06-!- noy [~Noy@wesnoth/developer/noy] has quit [Read error: Connection reset by peer] 20170402 21:40:11< vultraz_iOS> i think i wanted you to add a rect type, yes 20170402 21:41:09< celticminstrel> That's really easy to do though. 20170402 21:41:34< vultraz_iOS> probably 20170402 21:41:41< vultraz_iOS> now that i understand the code more 20170402 21:42:57< celticminstrel> You need a formula_callable subclass, and possibly a formula function to create it. 20170402 21:44:44< celticminstrel> And if you want the function you'll probably want to not add it to the builtin functions. 20170402 21:44:55< celticminstrel> But you also might not need the function. 20170402 21:48:20-!- Greg-Boggs [~greg_bogg@2601:1c2:f00:9780:68a6:da81:bf55:d6d7] has quit [Remote host closed the connection] 20170402 21:49:28-!- atarocch [~atarocch@93.56.160.28] has quit [Ping timeout: 246 seconds] 20170402 22:21:32-!- RatArmy [~ratarmy@om126204192020.6.openmobile.ne.jp] has joined #wesnoth-dev 20170402 22:49:06-!- RatArmy [~ratarmy@om126204192020.6.openmobile.ne.jp] has quit [Read error: Connection reset by peer] 20170402 22:50:29-!- travis-ci [~travis-ci@ec2-107-20-121-44.compute-1.amazonaws.com] has joined #wesnoth-dev 20170402 22:50:30< travis-ci> wesnoth/wesnoth#13127 (wfl_action_callable - 7c5411c : Celtic Minstrel): The build has errored. 20170402 22:50:30< travis-ci> Build details : https://travis-ci.org/wesnoth/wesnoth/builds/217851278 20170402 22:50:30-!- travis-ci [~travis-ci@ec2-107-20-121-44.compute-1.amazonaws.com] has left #wesnoth-dev [] 20170402 22:59:48-!- irker012 [~irker@uruz.ai0867.net] has quit [Quit: transmission timeout] 20170402 23:05:24-!- Duthlet [~Duthlet@dslb-188-106-146-119.188.106.pools.vodafone-ip.de] has quit [Quit: leaving] 20170402 23:07:41-!- RatArmy [~ratarmy@om126204192020.6.openmobile.ne.jp] has joined #wesnoth-dev 20170402 23:11:39-!- Greg-Boggs [~greg_bogg@c-76-115-139-154.hsd1.or.comcast.net] has joined #wesnoth-dev 20170402 23:12:44-!- Greg-Boggs [~greg_bogg@c-76-115-139-154.hsd1.or.comcast.net] has quit [Remote host closed the connection] 20170402 23:18:42< celticminstrel> :| 20170402 23:21:27-!- irker691 [~irker@uruz.ai0867.net] has joined #wesnoth-dev 20170402 23:21:27< irker691> wesnoth: Celtic Minstrel wesnoth:wfl_action_callable ba9822a44ac5 / src/formula/callable_objects.cpp: Fixup unused variable https://github.com/wesnoth/wesnoth/commit/ba9822a44ac5904855009b7eedfce8a97598ec11 20170402 23:23:04< irker691> wesnoth: Celtic Minstrel wesnoth:wfl_action_callable 547ee7bcf27f / src/formula/function.cpp: Fixup recursion failure https://github.com/wesnoth/wesnoth/commit/547ee7bcf27fbcadcc60fe47bdc68c7f8f374217 20170402 23:23:04-!- RatArmy [~ratarmy@om126204192020.6.openmobile.ne.jp] has quit [Read error: Connection reset by peer] 20170402 23:25:43-!- mattsc [~mattsc@wesnoth/developer/mattsc] has quit [Quit: So long and thanks for all the fish.] 20170402 23:26:40-!- zookeeper [~lmsnie@wesnoth/developer/zookeeper] has quit [Ping timeout: 260 seconds] 20170402 23:30:30-!- nore [~ncourant@sas.eleves.ens.fr] has quit [Ping timeout: 256 seconds] 20170402 23:33:54-!- RatArmy [~ratarmy@om126204192020.6.openmobile.ne.jp] has joined #wesnoth-dev 20170402 23:35:37< vultraz_iOS> celticminstrel: i've been thinking more about this problem with enable_shared_from_this 20170402 23:37:13< vultraz_iOS> celticminstrel: what if we create a new class, formula_callable_value, that encapsulates the data necessary for variant to operate, and a member is stored in formula_callable. then instead of passing the entire object with 'this', we pass the value object. problem is, I'm not sure exactly what constitutes the value of a formula_callable :| 20170402 23:38:01< vultraz_iOS> the base class doesn't hold the value currently, it looks like 20170402 23:38:14< celticminstrel> I'm not really sure what you mean, but... that does give me the idea of "self" returning a proxy. 20170402 23:38:43< vultraz_iOS> formula_callable has set_value and get_value members 20170402 23:38:57< vultraz_iOS> but it leaves it up to derived classes to implement values 20170402 23:39:22< celticminstrel> I'm not sure if this is the same as your idea, but let me explain what I mean by a proxy. 20170402 23:39:31< celticminstrel> First, create a new class that inherits from formula_callable. 20170402 23:39:52< celticminstrel> It does nothing but forward to another formula_callable. 20170402 23:40:29< celticminstrel> Then the implementation of query_value returns an instance of that initialized from this. 20170402 23:40:31-!- travis-ci [~travis-ci@ec2-54-147-239-36.compute-1.amazonaws.com] has joined #wesnoth-dev 20170402 23:40:32< travis-ci> wesnoth/wesnoth#13130 (wfl_action_callable - 6366b13 : Celtic Minstrel): The build has errored. 20170402 23:40:32< travis-ci> Build details : https://travis-ci.org/wesnoth/wesnoth/builds/217859268 20170402 23:40:32-!- travis-ci [~travis-ci@ec2-54-147-239-36.compute-1.amazonaws.com] has left #wesnoth-dev [] 20170402 23:40:56< celticminstrel> Now that I've explained it though, I'm starting to doubt that it solves anything... 20170402 23:40:57< vultraz_iOS> not exactly the same as my idea but it could work 20170402 23:41:30< vultraz_iOS> my idea was to essentially do something similar to what i did with variants, except have a single "callable value" class with a boost::any member 20170402 23:41:57< celticminstrel> Ehh... 20170402 23:41:59< vultraz_iOS> (come to think of it, I could have done that with variant_value_base too. hm) 20170402 23:42:14< celticminstrel> TBH using boost::any here doesn't seem like the right choice. 20170402 23:42:52< vultraz_iOS> celticminstrel: the point would be that variant would be passed a shared_ptr generated from 'this', except 'this' is the value object not the formula_callable 20170402 23:44:00< celticminstrel> I'm not convinced it would solve the problem. And using boost::any almost certainly isn't the right choice here. 20170402 23:45:59< vultraz_iOS> why? 20170402 23:46:34< celticminstrel> boost::any isn't the right choice because this isn't a situation where we can have any type of value. 20170402 23:47:12< vultraz_iOS> but it would be 20170402 23:47:19< celticminstrel> Not really? 20170402 23:47:21< vultraz_iOS> if this was just a wrapper class to hold a value 20170402 23:47:52< celticminstrel> If what you want is a wrapper class to hold a value... don't write your own class, just use boost::any directly. But... that's really not what we need. 20170402 23:48:29< vultraz_iOS> the wrapper class is so there's something to inherit from enable_shared_from_this that has no derived classes 20170402 23:48:37< vultraz_iOS> you see? 20170402 23:48:42< vultraz_iOS> or am i not explaining it well 20170402 23:48:57-!- Greg-Boggs [~greg_bogg@2601:1c2:f00:9780:68a6:da81:bf55:d6d7] has joined #wesnoth-dev 20170402 23:49:04< celticminstrel> I'm not entirely clear on why it has to inherit from enable_shared_from_this in the first place. 20170402 23:49:30< vultraz_iOS> ...because we can't do shared_ptr<>(this)? 20170402 23:49:36< vultraz_iOS> we went over this yesterday 20170402 23:49:37< celticminstrel> I should probably take a look at the C++ standard to determine to what degree it mandates the implementation of shared_ptr's detection of enable_shared_from_this... 20170402 23:49:50-!- Greg-Boggs [~greg_bogg@2601:1c2:f00:9780:68a6:da81:bf55:d6d7] has quit [Remote host closed the connection] 20170402 23:49:56< celticminstrel> Well yes, we need enable_shared_from_this if we want to return this from the formula_callable. 20170402 23:50:11< celticminstrel> But once you start creating a new class to handle it, it's no longer clear whether you still need enable_shared_from_this. 20170402 23:50:23< vultraz_iOS> if variant takes a shared_ptr 20170402 23:50:24< vultraz_iOS> yes 20170402 23:51:34< vultraz_iOS> so instead of if(has_self_ && key == "self") { return variant(this); } we do if(has_self_ && key == "self") { return variant(value_->shared_from_this()); } 20170402 23:52:16< vultraz_iOS> the "value" wrapper can even hold a pointer to its parent 20170402 23:52:19< vultraz_iOS> or reference 20170402 23:53:11< vultraz_iOS> this way, we never create a shared_ptr of this 20170402 23:53:23< vultraz_iOS> formula_callable always manages the lifetime of its value 20170402 23:53:35< vultraz_iOS> er 20170402 23:53:37< vultraz_iOS> well, it owns it 20170402 23:53:59< vultraz_iOS> and we can get shared_ptrs to the value AND a link back to the parent without dealing with enable_shared_from_this problems in a base class 20170402 23:54:02< vultraz_iOS> celticminstrel: get it now? 20170402 23:54:13< celticminstrel> Not entirely. 20170402 23:54:22< vultraz_iOS> :| 20170402 23:54:29-!- Greg-Boggs [~greg_bogg@2601:1c2:f00:9780:68a6:da81:bf55:d6d7] has joined #wesnoth-dev 20170402 23:54:31< vultraz_iOS> what's not clear 20170402 23:54:42< celticminstrel> I think I do roughly get the outline of what you're proposing. 20170402 23:55:24< celticminstrel> But I'm not convinced it's a good idea. 20170402 23:56:09< vultraz_iOS> why? 20170402 23:56:36< celticminstrel> Okay, so I have N3242 which is a draft of the C++11 standard... possibly the final draft, given that it's dated 2011... 20170402 23:57:00< celticminstrel> is on page 520... 20170402 23:58:04< vultraz_iOS> I'd like to know your objections to my proposal 20170402 23:58:22< vultraz_iOS> how would it not work? 20170402 23:58:24< celticminstrel> One of the objections is that it seems overly complex. 20170402 23:58:29< celticminstrel> It may work. 20170402 23:59:09< vultraz_iOS> less complex than making sure we never do a certain method of shared_ptr conversion and "hoping" it doesn't break? 20170402 23:59:30< vultraz_iOS> er 20170402 23:59:31< vultraz_iOS> more --- Log closed Mon Apr 03 00:00:57 2017