--- Log opened Fri Apr 14 00:00:49 2017
20170414 00:09:18-!- SigurdFD [~SigurdFD@dynamic-acs-72-23-110-196.zoominternet.net] has joined #wesnoth-dev
20170414 00:27:21-!- Greg-Boggs [~greg_bogg@2601:1c2:f00:9780:d4a2:2690:5771:4d16] has quit [Remote host closed the connection]
20170414 00:43:44-!- Greg-Boggs [~greg_bogg@2601:1c2:f00:9780:d4a2:2690:5771:4d16] has joined #wesnoth-dev
20170414 01:00:43-!- Greg-Boggs [~greg_bogg@2601:1c2:f00:9780:d4a2:2690:5771:4d16] has quit [Remote host closed the connection]
20170414 01:01:24-!- Greg-Boggs [~greg_bogg@2601:1c2:f00:9780:d4a2:2690:5771:4d16] has joined #wesnoth-dev
20170414 01:06:10-!- Greg-Boggs [~greg_bogg@2601:1c2:f00:9780:d4a2:2690:5771:4d16] has quit [Ping timeout: 258 seconds]
20170414 01:49:22-!- irker284 [~irker@uruz.ai0867.net] has joined #wesnoth-dev
20170414 01:49:22< irker284> wesnoth: gfgtdf wesnoth:master 1adeb98dd817 / src/game_initialization/connect_engine.cpp: attempt to fix assertion in connect_engine https://github.com/wesnoth/wesnoth/commit/1adeb98dd81745931e315c0ab913f2a641ec332a
20170414 02:02:49-!- Greg-Boggs [~greg_bogg@2601:1c2:f00:9780:d4a2:2690:5771:4d16] has joined #wesnoth-dev
20170414 02:03:58-!- Greg-Boggs [~greg_bogg@2601:1c2:f00:9780:d4a2:2690:5771:4d16] has quit [Remote host closed the connection]
20170414 02:04:14-!- Greg-Boggs [~greg_bogg@2601:1c2:f00:9780:d4a2:2690:5771:4d16] has joined #wesnoth-dev
20170414 02:23:30-!- Greg-Boggs [~greg_bogg@2601:1c2:f00:9780:d4a2:2690:5771:4d16] has quit [Remote host closed the connection]
20170414 02:39:43-!- vultraz_iOS [uid24821@wesnoth/developer/vultraz] has joined #wesnoth-dev
20170414 02:41:08-!- stikonas [~gentoo@wesnoth/translator/stikonas] has quit [Quit: Konversation terminated!]
20170414 02:47:27< irker284> wesnoth: Charles Dang wesnoth:master f9004970c48a / src/storyscreen/interface.cpp: Story Viewer: don't only validate presence of [part] tags under [story] https://github.com/wesnoth/wesnoth/commit/f9004970c48a50264d7253dd7a018ddb2cba6f35
20170414 02:48:29< vultraz_iOS> celticminstrel: https://gna.org/bugs/index.php?25657
20170414 02:51:38< celticminstrel> vultraz_iOS: Any thoughts on my commit?
20170414 02:51:47< celticminstrel> The one on that branch
20170414 02:51:52< celticminstrel> frames_cleanup I think it was.
20170414 02:52:50< vultraz_iOS> `template<> template<>` o_o what dis
20170414 02:53:11< celticminstrel> If you have a template function defined in a template class, that's how you have to declare it out-of-line.
20170414 02:53:25< celticminstrel> Or a template class inside a template class, even.
20170414 02:54:37< vultraz_iOS> seems reasonable
20170414 02:54:57< celticminstrel> I need to fix its Travis build before merging though.
20170414 02:59:46< vultraz_iOS> i think a better question, though
20170414 03:00:04< vultraz_iOS> is why all those classes use different types for their parameters
20170414 03:00:30< celticminstrel> Well, frame_builder simply holds the raw data from the config.
20170414 03:00:50< celticminstrel> frame_parsed_parameters holds that data parsed into an animation.
20170414 03:01:02< celticminstrel> And frame_parameters holds that data resolved for a single frame.
20170414 03:01:09< celticminstrel> Something like that.
20170414 03:11:59< vultraz_iOS> celticminstrel: could you add DEPRECATED to global.hpp
20170414 03:12:12< celticminstrel> Probaly.
20170414 03:12:17< celticminstrel> ^probably
20170414 03:12:42< vultraz_iOS> i don't know all the flags
20170414 03:13:56< celticminstrel> Hmm. If I added DEPRECATED, you would have to use it like this:
20170414 03:14:07< celticminstrel> DEPRECATED(int f(), "reason") { ... }
20170414 03:14:21< vultraz_iOS> er..
20170414 03:14:23< vultraz_iOS> how come
20170414 03:14:31< celticminstrel> Because GCC requires the annotation to come at the end of the declaration.
20170414 03:14:48< vultraz_iOS> but that's not the format :/
20170414 03:14:53< celticminstrel> What?
20170414 03:15:54< celticminstrel> GCC's deprecated attribute is "int f() __attribute__((deprecated))"
20170414 03:16:19< vultraz_iOS> er
20170414 03:16:19< vultraz_iOS> wait
20170414 03:16:23< celticminstrel> I mean, I suppose I could test if it works at the beginning too...
20170414 03:16:24< vultraz_iOS> how would it look in actual usage
20170414 03:16:37< celticminstrel> Give me a minute.
20170414 03:17:27< vultraz_iOS> i thought you meant GCC wanted [[deprecated]]name("reason")(args) {...}
20170414 03:18:04< celticminstrel> DEPRECATED(decl, reason) would expand to decl __attribute__((deprecated))
20170414 03:21:28< celticminstrel> Oh hey, it does work in clang/GCC to put it at the beginning.
20170414 03:21:44< celticminstrel> And clang also accepts __declspec(deprecated) at the beginning, so I'd assume MSVC does too.
20170414 03:24:04-!- Greg-Boggs [~greg_bogg@c-76-115-139-154.hsd1.or.comcast.net] has joined #wesnoth-dev
20170414 03:25:55< celticminstrel> Doesn't look like there's a way to determine if clang supports it in C++11 format.
20170414 03:28:06< vultraz_iOS> you mean 14?
20170414 03:28:23< celticminstrel> The attribute syntax is C++11.
20170414 03:28:27< vultraz_iOS> ah
20170414 03:28:47-!- Greg-Boggs [~greg_bogg@c-76-115-139-154.hsd1.or.comcast.net] has quit [Ping timeout: 240 seconds]
20170414 03:28:48< vultraz_iOS> but the key is c++14. i assume if c++14 mode is enabled the C++11 syntax would work?
20170414 03:29:00< celticminstrel> Obviously?
20170414 03:29:27< celticminstrel> Anyway, deprecated has been supported in most compilers for a long, long time.
20170414 03:29:50< celticminstrel> Was probably supported in 1998 even.
20170414 03:30:17< celticminstrel> Though I myself wasn't programming in C++ until the 2000's...
20170414 03:30:20< vultraz_iOS> O_O
20170414 03:30:23< vultraz_iOS> 1998
20170414 03:30:46< celticminstrel> I don't know exactly when GCC and Microsoft decided to support it.
20170414 03:30:50< vultraz_iOS> i was barely born in 1998
20170414 03:31:13< celticminstrel> I also don't know if the other major compilers of the time supported it, like Borland or Intel or whatever they were.
20170414 03:31:43< celticminstrel> I guess Apply had its own compiler, because I would've been using that in my hacking on Glypha III...
20170414 03:31:47< celticminstrel> ^Apple
20170414 03:32:01< celticminstrel> Or wait, no, I wasn't. I was using CodeWarrior for that.
20170414 03:32:10< celticminstrel> No idea how I got a seemingly non-trial version of it though.
20170414 03:35:14< celticminstrel> (Also not sure that was in 1998, might've been several years later. And it was C, not C++.)
20170414 03:39:04< vultraz_iOS> trying to figure out why unit uses unique_ptrs for some strings o-O
20170414 03:39:19< celticminstrel> What strings?
20170414 03:39:45< vultraz_iOS> 	std::unique_ptr<std::string> usage_;
20170414 03:39:45< vultraz_iOS> 	std::unique_ptr<std::string> halo_;
20170414 03:39:45< vultraz_iOS> 	std::unique_ptr<std::string> ellipse_;
20170414 03:40:57< vultraz_iOS> seems to use this to have some 'null'...
20170414 03:41:05< vultraz_iOS> but... just have an empty string? or optional?
20170414 03:42:37< vultraz_iOS> celticminstrel: btw, unit::swap... could this be removed and simply use std::swap and restore the refcounts after?
20170414 03:43:37< celticminstrel> Oh hey. Is there a unit::operator= and if so what parameters does it take?
20170414 03:44:04< celticminstrel> A swap function is about as fundamental as assignment operator or copy construction.
20170414 03:44:11< celticminstrel> So I'd suggest leaving it.
20170414 03:44:22< vultraz_iOS> unit::swap is an implementation detail of operator=
20170414 03:44:23< celticminstrel> IIRC, std::swap will actually call it for you if it exists.
20170414 03:44:53< celticminstrel> If swap is an implementation detail of operator=, then there should be exactly one operator= that takes its parameter by value (no reference).
20170414 03:44:55< vultraz_iOS> then calls std::swap on every unit attribute (or so I'm assuming) except refcount
20170414 03:45:01< vultraz_iOS> yes
20170414 03:45:20< celticminstrel> Uh, wait, what?
20170414 03:45:31< celticminstrel> There's a swap, but operator= doesn't defer to it?
20170414 03:45:39< celticminstrel> The "best" way to implement operator= is:
20170414 03:45:55< celticminstrel> T& operator=(T other) {this->swap(other); return *this;}
20170414 03:46:15< vultraz_iOS> celticminstrel: https://github.com/wesnoth/wesnoth/blob/master/src/units/unit.cpp#L728-L804
20170414 03:47:03< vultraz_iOS> it's not an std::swap specialization (it could be)
20170414 03:47:12< vultraz_iOS> (perhaps should be)
20170414 03:47:46< celticminstrel> Okay yeah, that's how it should be.
20170414 03:48:02< vultraz_iOS> a std::swap specialization?
20170414 03:48:03< celticminstrel> ISTR that std::swap will call a.swap(b) if possible, so a swap specialization isn't necessary.
20170414 03:48:15< vultraz_iOS> I see
20170414 03:49:03< celticminstrel> I could be wrong however.
20170414 03:53:28 * celticminstrel tries a simple test case on Mac, finds it doesn't work.
20170414 03:53:59< celticminstrel> Swap does need access to the internals though.
20170414 03:54:07 * vultraz_iOS still wants to get to refactoring the intrusive_ptr from the unit_map at some point...
20170414 03:54:21< celticminstrel> So if you want to make a specialization, just make it defer to the member swap.
20170414 03:57:49< vultraz_iOS> specifically unit_map::iterator_base::get_shared_ptr seems like a good place for shared_from_this
20170414 03:58:30< vultraz_iOS> (name is misleading - it doesn't actually return a shared_ptr)
20170414 04:00:36< vultraz_iOS> anyway, later..
20170414 04:00:42< vultraz_iOS> that's complicated stuff..
20170414 04:00:44< vultraz_iOS> let's see..
20170414 04:00:55< vultraz_iOS> ah, you wanted to know why unit has a virtual dtor
20170414 04:00:56< vultraz_iOS> no idea
20170414 04:01:26< vultraz_iOS> nothing seems to inherit from it
20170414 04:02:29< celticminstrel> It looks like fake units are actually real units.
20170414 04:02:39< celticminstrel> ie, there's not fake_unit class or anything.
20170414 04:02:52< celticminstrel> ^no
20170414 04:05:56< celticminstrel> There's definitely nothing inheriting from it, anyway.
20170414 04:06:24< celticminstrel> I wonder if switching it to enable_shared_from_this would have performance implications...
20170414 04:06:42< celticminstrel> Actually, shared_ptr itself is theoretically slower than intrusive_ptr, because it's thread-safe.
20170414 04:07:07< celticminstrel> (intrusive_ptr's thread safety depends on the implementation of the refcounter.)
20170414 04:07:10< vultraz_iOS> isn't thread-safe good
20170414 04:07:13< celticminstrel> Yes.
20170414 04:07:22< celticminstrel> But it's also slower.
20170414 04:07:46< celticminstrel> Whether it's sufficiently slower to matter though, I have no idea. I'd assume not unless proven otherwise, TBH.
20170414 04:17:52< vultraz_iOS> hmm
20170414 04:18:03< vultraz_iOS> actually, the unit_map interface might mean it's easy to switch to shared_ptrs...
20170414 04:18:19< vultraz_iOS> * and get_shared_ptr do the same thing..
20170414 04:18:23< vultraz_iOS> could just change to shared_from_this..
20170414 04:20:03-!- SigurdFD [~SigurdFD@dynamic-acs-72-23-110-196.zoominternet.net] has quit []
20170414 04:21:27< celticminstrel> Don't forget that shared_from_this is technically unsafe.
20170414 04:21:47< celticminstrel> In that if the unit wasn't originally created for a shared_ptr, it'll do something bad.
20170414 04:22:27< celticminstrel> If you're going to do it, I'd suggest inheriting privately from enable_shared_from_this.
20170414 04:22:33< celticminstrel> And declare unit_map as a friend class.
20170414 04:27:39< celticminstrel> (And/or the unit map iterator.)
20170414 04:37:42< irker284> wesnoth: Celtic Minstrel wesnoth:frame_cleanup 90d0e8f0931b / src/units/ (frame.cpp frame.hpp): Templatize the unit animation frame parameters https://github.com/wesnoth/wesnoth/commit/90d0e8f0931b9b9038392fc62dc513f858174804
20170414 04:38:16< celticminstrel> We'll see if Travis builds it now...
20170414 04:38:32< celticminstrel> Last time it gave up because of too many errors, so there could still be more problems.
20170414 04:39:20< celticminstrel> Just FTR, the only changes were adding some .str() in the frame_builder constructor, adding template<> to the basic_frame_parameters constructors, and swapping the order of auto_hflip and auto_vflip in the declaration.
20170414 05:03:09-!- Greg-Boggs [~greg_bogg@2601:1c2:f00:9780:d4a2:2690:5771:4d16] has joined #wesnoth-dev
20170414 05:08:49-!- Greg-Boggs [~greg_bogg@2601:1c2:f00:9780:d4a2:2690:5771:4d16] has quit [Ping timeout: 258 seconds]
20170414 05:53:58< vultraz_iOS> celticminstrel: I can't seem to find where the alpha_ member in unit is used
20170414 05:57:55< vultraz_iOS> it doesn't appear to do anything
20170414 05:58:16< vultraz_iOS> there's alpha in unit_type..
20170414 05:58:27< vultraz_iOS> and the member in unit copies it...
20170414 05:58:31< vultraz_iOS> in advance_to
20170414 05:58:43< vultraz_iOS> but the unit::alpha function appears unused
20170414 05:58:54< vultraz_iOS> and the member is only used to set data in the class, not used anywhere
20170414 05:59:25< vultraz_iOS> can't really find where it's used in unit_type either
20170414 06:00:38< vultraz_iOS> there's no such key documented on the wiki
20170414 06:01:16< vultraz_iOS> celticminstrel: do remove?
20170414 06:04:38< celticminstrel> Well, the best way to test if it's unused is to comment it out and see what fails... or you already did that?
20170414 06:06:02< vultraz_iOS> just grep, but trying that now
20170414 06:06:19-!- atarocch [~atarocch@93.56.160.28] has joined #wesnoth-dev
20170414 06:08:29< vultraz_iOS> i just realized something
20170414 06:10:01< vultraz_iOS> when generating a unit from a type, the unit basically copies all its data from the type 
20170414 06:10:07< vultraz_iOS> why :/
20170414 06:10:47< vultraz_iOS> instead of... i dunno... keeping a unit_type member and referring to it directly?
20170414 06:11:18< vultraz_iOS> i mean, it already keeps a const ptr to a unit_type
20170414 06:11:22< vultraz_iOS> so why all the copying?
20170414 06:13:27< vultraz_iOS> celticminstrel: ok, that aside for a second, the alpha getter in unit isn't used at all and the alpha getter in unit_type is only used to set unit::alpha_ in unit::advance_to
20170414 06:13:59< vultraz_iOS> and as far as I can see, the alpha_ member in either class doesn't really do anything besides being passed around or written to a config
20170414 06:14:59-!- Kwandulin [~Kwandulin@p200300760F6D80479D1E6D80CA3C1CA3.dip0.t-ipconnect.de] has joined #wesnoth-dev
20170414 06:27:55< celticminstrel> It needs to copy because all its stats can be modified independently of the type.
20170414 06:28:27< celticminstrel> Plus the type can change.
20170414 06:37:50< celticminstrel> Oh yeah, you could check git blame to see if the alpha member ever did anything.
20170414 06:39:47< vultraz_iOS> I don't know how to use git blak e
20170414 06:39:49< vultraz_iOS> Bla,em
20170414 06:39:50< vultraz_iOS> Blame
20170414 06:40:06< celticminstrel> You can use it from github.
20170414 06:43:06< vultraz_iOS> ooo
20170414 06:44:47< celticminstrel> And there's even the "view blame before this change" option.
20170414 06:46:29< vultraz_iOS> doesn't appear to have ever done anything
20170414 06:47:27< celticminstrel> So the commit that added it didn't make it do anything?
20170414 06:47:32< celticminstrel> Weird.
20170414 06:47:39< vultraz_iOS> oh, i need to go that far back
20170414 06:48:07< celticminstrel> Well, if it doesn't do anything and never did anything and isn't documented, I don't see any reason to not remove it.
20170414 06:49:24< vultraz_iOS> the alpha member was added to unit 10 years ago
20170414 06:50:06< vultraz_iOS> or 
20170414 06:50:06< vultraz_iOS> well
20170414 06:50:10< vultraz_iOS> last modified 
20170414 06:50:59< vultraz_iOS> what the hell is an FPU-less machine 
20170414 06:51:09< vultraz_iOS> https://github.com/wesnoth/wesnoth/commit/81ffd3dc9910107522b391b07338381b76180ec5
20170414 06:51:33< celticminstrel> A machine that does not natively support floating-point calculations.
20170414 06:51:37< celticminstrel> The NES for example.
20170414 06:52:55< vultraz_iOS> do those even exist today
20170414 06:53:11< celticminstrel> Probably.
20170414 06:53:12< vultraz_iOS> and do we even support them
20170414 06:53:18< celticminstrel> But I wouldn't expect them to be used in personal computers.
20170414 06:53:35< vultraz_iOS> so i wonder if this fixed-point code can be removed
20170414 06:53:54< celticminstrel> Eh.
20170414 06:54:29< celticminstrel> Fixed point does have some advantages over floating-point though.
20170414 06:54:52< vultraz_iOS> this alpha stuff has been here since the beginning
20170414 06:54:57< vultraz_iOS> I can trace it back 14 years
20170414 06:55:06< celticminstrel> For example, calculations are exact rather than approximate, provided the result actually fits.
20170414 06:55:30< vultraz_iOS> going to remove
20170414 06:55:34< celticminstrel> Alpha?
20170414 06:55:41< vultraz_iOS> yes
20170414 06:55:46< celticminstrel> Okay.
20170414 06:55:52< vultraz_iOS> it's not used for anything in either class except being written and calculated..
20170414 06:55:57< celticminstrel> Don't remove fixed point without getting multiple opinions.
20170414 06:56:07< vultraz_iOS> it's serialized to a config but I can't find any place where an "alpha" key is read from a config
20170414 06:56:11< celticminstrel> Calculated how?
20170414 06:56:24< vultraz_iOS> well, besides here
20170414 06:56:26< vultraz_iOS> 	const config::attribute_value & alpha_blend = cfg_["alpha"];
20170414 06:56:26< vultraz_iOS> 	if(alpha_blend.empty() == false) {
20170414 06:56:26< vultraz_iOS> 		alpha_ = ftofxp(alpha_blend.to_double());
20170414 06:56:26< vultraz_iOS> 	}
20170414 06:56:35< vultraz_iOS> (in unit_type)
20170414 06:57:29< celticminstrel> The expected function of that key isn't the responsibility of the unit class now, anyway. It's part of the animations system.
20170414 06:57:35< vultraz_iOS> indeed
20170414 06:57:40< vultraz_iOS> stuff like
20170414 06:57:41< vultraz_iOS> , highlight_ratio_(cfg[frame_string + "alpha"])
20170414 06:57:48< vultraz_iOS> probably handles what this used to do
20170414 06:57:51< celticminstrel> I think the fixed point code might've been implemented because of OpenPandora or something?
20170414 06:58:13< celticminstrel> Is highlight_ratio the thing used for shadows?
20170414 06:59:02< vultraz_iOS> dunno
20170414 07:00:31< celticminstrel> To make them semitransparent.
20170414 07:00:47< celticminstrel> Fading in and out.
20170414 07:02:15< vultraz_iOS> looks like an alpha  hey
20170414 07:02:17< vultraz_iOS> key
20170414 07:03:04< celticminstrel> ???
20170414 07:03:22< vultraz_iOS> the shadow transparency us controlled by an alpha key
20170414 07:03:27< vultraz_iOS> in the animations
20170414 07:04:11< celticminstrel> Looks like the alpha key is loaded into highlight_ratio.
20170414 07:05:15< irker284> wesnoth: Celtic Minstrel wesnoth:frame_cleanup 46f81f7f0f1e / src/units/ (frame.cpp frame.hpp): Templatize the unit animation frame parameters https://github.com/wesnoth/wesnoth/commit/46f81f7f0f1e17cb350802330c7409d676c8a2b7
20170414 07:05:34< celticminstrel> Looks like my last fix worked, apart from the fact that I forgot to save before committing.
20170414 07:06:05< celticminstrel> One nice/annoying difference with XCode is that it always saves your work pretty much immediately (or as soon as you're idle for a moment).
20170414 07:06:15-!- celticminstrel is now known as celmin|Zzzzzz
20170414 07:10:35< irker284> wesnoth: Charles Dang wesnoth:master 1310dc62f3cd / src/units/ (types.cpp types.hpp unit.cpp unit.hpp): Removed alpha key from unit and unit_type https://github.com/wesnoth/wesnoth/commit/1310dc62f3cd06061de6c834828796be4fb69fab
20170414 07:22:10< vultraz_iOS> gah
20170414 07:22:16< vultraz_iOS> this unit_map code is confusing
20170414 07:22:22< vultraz_iOS> why does unit_pod keep a refcount too
20170414 07:53:17-!- Kwandulin [~Kwandulin@p200300760F6D80479D1E6D80CA3C1CA3.dip0.t-ipconnect.de] has quit [Ping timeout: 252 seconds]
20170414 08:21:14-!- janebot [~Gambot@unaffiliated/gambit/bot/gambot] has quit [Remote host closed the connection]
20170414 08:21:20-!- janebot [~Gambot@unaffiliated/gambit/bot/gambot] has joined #wesnoth-dev
20170414 08:21:23-!- zookeeper [~lmsnie@wesnoth/developer/zookeeper] has joined #wesnoth-dev
20170414 08:27:54-!- Kwandulin [~Kwandulin@p200300760F6D80479D1E6D80CA3C1CA3.dip0.t-ipconnect.de] has joined #wesnoth-dev
20170414 08:42:13-!- mjs-de [~mjs-de@x4db608a0.dyn.telefonica.de] has joined #wesnoth-dev
20170414 08:46:11< zookeeper> vultraz_iOS, [unit_type] used to support an alpha= key?
20170414 08:47:03< zookeeper> (no, i don't mind it being removed, surely no one even knew of it and it's not needed for anything anyway)
20170414 09:03:39-!- Kwandulin [~Kwandulin@p200300760F6D80479D1E6D80CA3C1CA3.dip0.t-ipconnect.de] has quit [Read error: Connection reset by peer]
20170414 09:15:11-!- mjs-de [~mjs-de@x4db608a0.dyn.telefonica.de] has quit [Remote host closed the connection]
20170414 09:45:39-!- Kwandulin [~Kwandulin@p200300760F6D80470CE10DFFF5655DDB.dip0.t-ipconnect.de] has joined #wesnoth-dev
20170414 10:04:28< vultraz_iOS> zookeeper: apparently
20170414 10:04:48< vultraz_iOS> zookeeper: I traced the presence of these keys back over a decade
20170414 10:15:35-!- prkc [~prkc@e9.85.7a9f.ip4.static.sl-reverse.com] has quit [Remote host closed the connection]
20170414 10:17:36< zookeeper> fun
20170414 10:31:51-!- esr [~esr@wesnoth/developer/esr] has quit [Quit: WeeChat 1.4]
20170414 10:39:25-!- esr [~esr@wesnoth/developer/esr] has joined #wesnoth-dev
20170414 10:46:05-!- gfgtdf [~chatzilla@x4e363021.dyn.telefonica.de] has joined #wesnoth-dev
20170414 10:46:29< gfgtdf> 20170414 03:39:04< vultraz_iOS> trying to figure out why unit uses unique_ptrs for some strings o-O
20170414 10:47:20< gfgtdf> vultraz_iOS: they are just optional, the previously were atribute_values and changing them to unique_ptrs/optional came closer to th erepvious behaviour
20170414 10:47:53< gfgtdf> vultraz_iOS: which doesn't work that just std::strign dwouldn't work aswell
20170414 10:48:02< irker284> wesnoth: Charles Dang wesnoth:master 8a1263c8b5ad / src/synced_commands.cpp: Don't exit to desktop if using debug unit command with invalid type name (FR #25 https://github.com/wesnoth/wesnoth/commit/8a1263c8b5ad5d7f9df3a936fb8973bf74daef4a
20170414 10:48:39< gfgtdf> s/doesn't work/doesn't mean
20170414 10:49:10< vultraz_iOS> I'd prefer we used optional since it makes it clearer what it's for
20170414 10:49:30< vultraz_iOS> but im not sure how just an empty string doesn't suffice
20170414 10:49:58< vultraz_iOS> oh blah i meant to day titlescreen in that commit message
20170414 10:49:59< vultraz_iOS> fuck
20170414 11:13:28-!- stikonas [~gentoo@wesnoth/translator/stikonas] has joined #wesnoth-dev
20170414 11:13:44< Kwandulin> Wait, when did that change? I am having a side with team_name=goodguys, badguys, so it is allied with the player and the enemies and thus can't be attacked (former behaviour). Now that side is attacked by both. Wat do
20170414 11:13:54-!- atarocch [~atarocch@93.56.160.28] has quit [Remote host closed the connection]
20170414 11:14:31< vultraz_iOS> uh..
20170414 11:14:32< vultraz_iOS> weird
20170414 11:14:46< vultraz_iOS> gfgtdf: know anything about this? ^
20170414 11:15:44< zookeeper> great, more breakage
20170414 11:17:15< vultraz_iOS> break ALL THE THINGS \ o /
20170414 11:21:12< Kwandulin> Ah, seems it's just in 1.13.6
20170414 11:21:21< Kwandulin> (missclicked on 1.13.6)
20170414 11:21:24< Kwandulin> seems to work in 1.13.7
20170414 11:21:28< vultraz_iOS> ok good
20170414 11:23:48< zookeeper> yay
20170414 11:26:01-!- stikonas [~gentoo@wesnoth/translator/stikonas] has quit [Ping timeout: 255 seconds]
20170414 11:26:04-!- stikonas_ [~gentoo@wesnoth/translator/stikonas] has joined #wesnoth-dev
20170414 11:36:30-!- atarocch [~atarocch@109.112.23.36] has joined #wesnoth-dev
20170414 11:49:15< irker284> wesnoth: gfgtdf wesnoth:master e4926328f92a / src/ (8 files in 5 dirs): use std::scoped_ptr where appropriate 1 https://github.com/wesnoth/wesnoth/commit/e4926328f92a443900853f9570d3c98bbf50ddb7
20170414 11:49:17< irker284> wesnoth: gfgtdf wesnoth:master 4bdfc41c0682 / src/gui/ (10 files in 2 dirs): use std::scoped_ptr where appropriate 2 (gui2 code) https://github.com/wesnoth/wesnoth/commit/4bdfc41c068251dabc16ee7115466ac737446d2e
20170414 11:49:19< irker284> wesnoth: gfgtdf wesnoth:master 25d7110c8c0f / src/ (9 files in 4 dirs): use std::scoped_ptr where appropriate 3 (filesystem code) https://github.com/wesnoth/wesnoth/commit/25d7110c8c0f9563f6f3ae3a8b4c1bab1adb4fca
20170414 11:50:56-!- stikonas_ is now known as stikonas
20170414 11:56:51< vultraz_iOS> :O a new ptr!
20170414 11:58:01< vultraz_iOS> oh you're just using unique_ptrs
20170414 12:01:44< gfgtdf> vultraz_iOS: aww yes tyop
20170414 12:01:59< gfgtdf> stull uses to the old boost name 
20170414 12:02:06< gfgtdf> used*
20170414 12:02:14< gfgtdf> used*
20170414 12:02:43< gfgtdf> to late for ahncging now i guess
20170414 12:02:47< gfgtdf> for changing*
20170414 12:03:17< vultraz_iOS> it's ok
20170414 12:03:56< vultraz_iOS> gfgtdf: good work tho, that's a lot of cleanip :)
20170414 12:03:57< vultraz_iOS> cleanup
20170414 12:05:51< gfgtdf> while doing this i notoiced a few lined that look liek memeory leak, i didnt chenge them becasue i wasn't sure though
20170414 12:06:08< gfgtdf> first one: https://github.com/wesnoth/wesnoth/blob/master/src/addon/manager.cpp#L235
20170414 12:06:40< gfgtdf> second one: https://github.com/wesnoth/wesnoth/blob/master/src/game_initialization/create_engine.cpp#L700
20170414 12:07:08< gfgtdf> third one: https://github.com/wesnoth/wesnoth/blob/e4926328f92a443900853f9570d3c98bbf50ddb7/src/game_initialization/lobby_info.cpp#L194
20170414 12:10:33< gfgtdf> matthiaskrgr: coudl ypoui please test the follwing cases for memeory leaks: 
20170414 12:10:42< gfgtdf> 1) upload an addonn that uses an .ign file
20170414 12:11:13< gfgtdf> 2) play in mp an usewr made scenario made via editor (from /editor/scenarios/)
20170414 12:12:15< gfgtdf> 3) join the mp server with multiple hwne multiple games are open/running.
20170414 12:22:14-!- Kwandulin [~Kwandulin@p200300760F6D80470CE10DFFF5655DDB.dip0.t-ipconnect.de] has quit [Ping timeout: 252 seconds]
20170414 12:23:47-!- Kwandulin [~Kwandulin@p200300760F6D80670CE10DFFF5655DDB.dip0.t-ipconnect.de] has joined #wesnoth-dev
20170414 12:43:11< gfgtdf> (3) actualyl means be oin the mp server lobby while anoter game ends/closes
20170414 12:52:10< irker284> wesnoth: Maxwell Paul Brickner wesnoth:master de76ba017f94 / README.md: Updated links to wesnoth.org to use https https://github.com/wesnoth/wesnoth/commit/de76ba017f94bc14ea19cf55478d7e593277da1b
20170414 12:52:12< irker284> wesnoth: Wedge009 wesnoth:master ade221e3030e / README.md: Merge pull request #981 from mbrickn/patch-1 https://github.com/wesnoth/wesnoth/commit/ade221e3030ea91e0c8e3c507ffebe4304af518d
20170414 12:52:51-!- JyrkiVesterinen [~JyrkiVest@87-92-12-90.bb.dnainternet.fi] has joined #wesnoth-dev
20170414 13:50:56-!- Kwandulin [~Kwandulin@p200300760F6D80670CE10DFFF5655DDB.dip0.t-ipconnect.de] has quit [Read error: Connection reset by peer]
20170414 13:52:41-!- JyrkiVesterinen [~JyrkiVest@87-92-12-90.bb.dnainternet.fi] has quit [Quit: .]
20170414 13:57:50-!- atarocch [~atarocch@109.112.23.36] has quit [Ping timeout: 268 seconds]
20170414 14:09:16-!- atarocch [~atarocch@109.112.23.36] has joined #wesnoth-dev
20170414 14:28:40-!- atarocch [~atarocch@109.112.23.36] has quit [Ping timeout: 268 seconds]
20170414 14:29:31-!- JyrkiVesterinen [~JyrkiVest@87-92-12-90.bb.dnainternet.fi] has joined #wesnoth-dev
20170414 14:30:32< irker284> wesnoth: Charles Dang wesnoth:master ebeeae38724b / src/ (scripting/lua_unit.cpp units/unit.cpp units/unit.hpp): Unit: refactor upkeep handling to use a visitor pattern https://github.com/wesnoth/wesnoth/commit/ebeeae38724b9bbe6c7de4d29fa92491030153b7
20170414 14:31:50-!- mkdroid [~null@unaffiliated/matthiaskrgr] has joined #wesnoth-dev
20170414 14:32:36-!- mkdroid [~null@unaffiliated/matthiaskrgr] has quit [Client Quit]
20170414 14:32:45 * vultraz_iOS now knows how to use the visitor pattern
20170414 14:45:17-!- Kwandulin [~Kwandulin@p200300760F6D8067C106474B65342408.dip0.t-ipconnect.de] has joined #wesnoth-dev
20170414 14:53:13-!- oldlaptop [~quassel@45.63.78.126] has quit [Remote host closed the connection]
20170414 15:17:49< celmin|Zzzzzz> Um. GNA 25619 is a bug, not a feature request.
20170414 15:18:01< celmin|Zzzzzz> (I just changed the ticket too.)
20170414 15:21:27< vultraz_iOS> I'll change the lua_pushscreen later
20170414 15:21:35< vultraz_iOS> I'm watching the Star Wars livestream
20170414 15:21:42< celmin|Zzzzzz> I assume you mean pushstring.
20170414 15:22:10< vultraz_iOS> Yes
20170414 15:22:12< celmin|Zzzzzz> I can't quite remember, but pushstring might've been specifically for string constants.
20170414 15:24:30< JyrkiVesterinen> No, it's not. "Lua makes (or reuses) an internal copy of the given string, so the memory at s can be freed or reused immediately after the function returns."
20170414 15:24:35< celmin|Zzzzzz> Hmm. Not happy with the latest error on frame_cleanup... it implies I need to move the constructors to the header...
20170414 15:24:45< celmin|Zzzzzz> JyrkiVesterinen: Not what?
20170414 15:24:59< JyrkiVesterinen> It's not "specifically for string constants".
20170414 15:25:04< celmin|Zzzzzz> Ah, okay.
20170414 15:25:19< JyrkiVesterinen> The string you push can be mutable, and you can even destroy it immediately after pushing.
20170414 15:29:33-!- Kwandulin [~Kwandulin@p200300760F6D8067C106474B65342408.dip0.t-ipconnect.de] has quit [Read error: Connection reset by peer]
20170414 15:30:50-!- Kwandulin [~Kwandulin@p200300760F6D8067C106474B65342408.dip0.t-ipconnect.de] has joined #wesnoth-dev
20170414 15:49:23< gfgtdf> zookeeper: do sighted events also for for locations cleared woith [lift_fog] ?
20170414 15:49:34< gfgtdf> s/for for/fire for
20170414 15:49:54< zookeeper> i don't know. any reason to suspect they wouldn't?
20170414 15:51:22< gfgtdf> zookeeper: it seems liek the function in vision.cpp that fire those events all require a 'viewer' unit
20170414 15:51:40< gfgtdf> zookeeper: since we have no 'viewer' unit here i suspect that they are not called
20170414 15:51:56< zookeeper> i see
20170414 15:53:00< zookeeper> i guess i can try to test that
20170414 15:53:12< zookeeper> unless you want to? :p
20170414 15:56:49< zookeeper> well, wrote a test case, checking...
20170414 15:57:55< gfgtdf> zookeeper: i wonder, if not shodul we fix that? and shodul we provice a filter for the 'viewing side' other than [filter_second] ?
20170414 15:58:56< zookeeper> err, except of course i wasn't thinking when i wrote this... just a moment :p
20170414 15:59:18-!- JyrkiVesterinen [~JyrkiVest@87-92-12-90.bb.dnainternet.fi] has quit [Quit: .]
20170414 16:00:54-!- Greg-Boggs [~greg_bogg@173.240.241.83] has joined #wesnoth-dev
20170414 16:02:36< zookeeper> indeed, a sighted event doesn't seem to fire if the unit is revealed because of a [lift_fog] that gets executed
20170414 16:03:40-!- Greg-Boggs [~greg_bogg@173.240.241.83] has quit [Remote host closed the connection]
20170414 16:03:47-!- Greg-Boggs [~greg_bogg@173.240.241.83] has joined #wesnoth-dev
20170414 16:05:16< zookeeper> yeah, that should be fixed. and i don't think there's a lot of need for other kinds of side filters
20170414 16:06:59< zookeeper> maybe if we had some kind of new event to supercede sighted then it could be designed differently, but i don't feel like we should tack on new special stuff onto sighted events. it's going to be very very rare that you'd want an event to fire when a unitless side sights something, after all.
20170414 16:08:09< vultraz_iOS> celmin|Zzzzzz: ok, so i should use lua_pushlstring here?
20170414 16:09:50< vultraz_iOS> celmin|Zzzzzz: actually lua_pushlstring doesn't work
20170414 16:09:56< vultraz_iOS> it still wants a c_str
20170414 16:10:09< vultraz_iOS> (cannot convert to const char*)
20170414 16:10:12< vultraz_iOS> lua_push works, though
20170414 16:10:43< vultraz_iOS> but i don't know if i can merge these statements...
20170414 16:10:53< vultraz_iOS> since my new behavior means a string is always returned
20170414 16:10:55< vultraz_iOS> not an int
20170414 16:14:51< celmin|Zzzzzz> vultraz_iOS: You pass .c_str() and .length() / .size()
20170414 16:14:57< celmin|Zzzzzz> (One of those, not the quotient.)
20170414 16:15:00< vultraz_iOS> .........what?
20170414 16:15:11< celmin|Zzzzzz> lua_pushlstring(s.c_str(), s.size())
20170414 16:15:18< vultraz_iOS> oh
20170414 16:15:29< celmin|Zzzzzz> lua_push is probably fine too, that's a Wesnoth template function.
20170414 16:15:37< celmin|Zzzzzz> It probably compiles down to the same thing.
20170414 16:16:06-!- mkdroid [~null@unaffiliated/matthiaskrgr] has joined #wesnoth-dev
20170414 16:16:09< gfgtdf> zookeeper: ok
20170414 16:16:36< vultraz_iOS> i meant if both branches use pua_push, can i combine then
20170414 16:16:46< vultraz_iOS> im not sure since that would be using a string for the int value 
20170414 16:17:09< celmin|Zzzzzz> If you combined the branches you'd be pushing a string for the int value, yes.
20170414 16:17:25< vultraz_iOS> and i assume that would cause problems
20170414 16:18:05< celmin|Zzzzzz> I don't think it would cause many problems. Lua does implicitly convert between string and number, but I think it might still be surprising - type(u.upkeep) would return string, not number.
20170414 16:18:27< celmin|Zzzzzz> Other than that I think it would work exactly the same for Lua users.
20170414 16:18:39< zookeeper> gfgtdf, i was under the impression that sighted events will currently fire in some situations where there is no "viewer unit", in which case it's just some random (closest?) unit on the viewing side that gets chosen as the secondary unit. so do you think this is a problem only with [lift_fog], or more generally elsewhere too?
20170414 16:19:36< irker284> wesnoth: Charles Dang wesnoth:master 234929683681 / src/scripting/lua_unit.cpp: Small improvement to ebeeae3 https://github.com/wesnoth/wesnoth/commit/234929683681ede82919a7b7e2d2dbf3d237d504
20170414 16:19:56< celmin|Zzzzzz> gfgtdf: Iterate through unit map, check units on the viewing side, pick the first one you find that can see the revealed unit.
20170414 16:20:01< gfgtdf> zookeeper: i actuall though there is always a viewer unit, ofcourse that one doesn always need to be moving, for exampel whena unit is recuited it is the viewer unit for shodul crearing done by recruioting
20170414 16:21:23< zookeeper> gfgtdf, yeah, there _should_ always be a viewer unit even if it's not a unit which causes the side to see the sighted unit.
20170414 16:22:26< gfgtdf> hmm that woudl fail is the side had no units
20170414 16:22:47-!- celmin|Zzzzzz is now known as celticminstrel
20170414 16:23:14< zookeeper> sure, but as said why would you write an event to fire when a unitless side sights some other side's unit? in theory sure that would fail, but it doesn't seem important here
20170414 16:23:22-!- boucman [~rosen@wesnoth/developer/boucman] has joined #wesnoth-dev
20170414 16:23:49< zookeeper> anyway, i don't know what the underlying reason is why sighted events don't get fired when a unit is revealed by [lift_fog]
20170414 16:24:49< vultraz_iOS> celticminstrel: about the unit map, btw... there seems to be two sources of refcounting..
20170414 16:24:56< vultraz_iOS> one in the map and one for the unit itself
20170414 16:24:59< vultraz_iOS> this confuses me
20170414 16:25:18< gfgtdf> vultraz_iOS: ye i agree
20170414 16:25:48< zookeeper> gfgtdf, okay, i tried what happens if i just remove fog from a side with [modify_side]... and no sighted events fire in that case either.
20170414 16:27:11< zookeeper> i guess i need to re-check what the intention for sighted event behavior in those kinds of cases was since i might be misremembering it, but afk for a bit first ->
20170414 16:28:03< gfgtdf> ok
20170414 16:30:11< vultraz_iOS> gfgtdf: unless im misunderstanding the point of the code.. shouldn't we be able to use a share_ptr for unit and eliminate the refcounting in unit_map - or if it's needed, forward it to the shared_ptr's refcount?
20170414 16:31:06< vultraz_iOS> with use_count()
20170414 16:32:04< gfgtdf> vultraz_iOS: i dont really see how replacign instrusuve_ptr with shard_ptr will make the other counter unneeded.
20170414 16:32:16< vultraz_iOS> hm
20170414 16:32:30< vultraz_iOS> then i guess i don't fully understand how the map works
20170414 16:32:44< vultraz_iOS> does the map refcount have to do with whether the ptr is in the map at all?
20170414 16:33:23< gfgtdf> originaly onyl the map refounted, it did that to make sure that the iterators are never invalidated
20170414 16:33:48< gfgtdf> so that you coudl so for(unit& u : unit_map) {do stuff with u}
20170414 16:34:12< gfgtdf> without worring about possibel wml code addng/rmeoving units
20170414 16:34:15< gfgtdf> form fired events
20170414 16:34:48< gfgtdf> then iceiceice added a refount to units here https://github.com/wesnoth/wesnoth/commit/9e7dc5ba008298d823c13cff214f7477601b5efd
20170414 16:36:05< gfgtdf> accorign to the commitmessage the intention was to make it easier to move units from recall/map/preivvate to each other witohut copying it
20170414 16:36:31< vultraz_iOS> ok
20170414 16:36:32< gfgtdf> he also intended to remove the other reffcount in favour of that but somhow didnt managed to do it.
20170414 16:36:56< vultraz_iOS> he intended to remove the unit_map refcount ?
20170414 16:37:00< gfgtdf> how sure whethe its becasue it too hard/imposible  or becasue he had other things to do
20170414 16:37:28< gfgtdf> accoring to the commitmessage ^ that was how original plan when adding the new recount to units.
20170414 16:37:33-!- turupawn [~turupawn@190.92.33.32] has joined #wesnoth-dev
20170414 16:37:40< gfgtdf> s/how/his
20170414 16:38:16< vultraz_iOS> hmm
20170414 16:38:20< vultraz_iOS> how can we do this
20170414 16:41:15< gfgtdf> hmm not sure
20170414 16:42:06< vultraz_iOS> i don't particularly understand *why* the unit refcount couldn't just replace the map refcount immediately
20170414 16:42:08< vultraz_iOS> in that commit
20170414 16:42:15< vultraz_iOS> basically
20170414 16:42:27< vultraz_iOS> what does that refcount ensure now that the unit refcount does not
20170414 16:43:18< gfgtdf> that iterators are not invalidated
20170414 16:43:39< celticminstrel> I don't quite understand wedge009's PR...
20170414 16:43:44< gfgtdf> so if you have like iterator_type a = unit_map_.find(id);
20170414 16:44:03< gfgtdf> you cna always to a++ without crashing even if the underlyign unit was smohow deleted.
20170414 16:45:19< vultraz_iOS> I see...
20170414 16:45:38< vultraz_iOS> ok let's take this in steps
20170414 16:45:45< vultraz_iOS> see if we change the unit refcounts to shared_ptr
20170414 16:46:25< vultraz_iOS> then work on removing the unit_pod
20170414 16:47:31< zookeeper> gfgtdf, ok, so EventWML doesn't really say anything about firing of sighted events due to actions other than unit recruit/movement/leveling/etc.
20170414 16:47:34< zookeeper> considering that sighted events do trigger even in the absense of fog/shroud when units are for example recruited, i think they should trigger even when [lift_fog] is used or fog is removed entirely from a side, and so on.
20170414 16:48:00< zookeeper> i don't know if that's hard to do though
20170414 16:49:42< gfgtdf> hmm whena unit is sighed becasue it is recuits (not beacause other are recruited) does the game specifiy a sighting unit (for [filter_secoind]) ?
20170414 16:50:16< gfgtdf> vultraz_iOS: changing the unit refocunt to shared_ptr shodul be rather easy, but i don't really see the advantage
20170414 16:52:19< gfgtdf> vultraz_iOS: its prpably just changin the unit_ptr typedef  from intrusove to shated + removing  then unused code.
20170414 16:52:21< zookeeper> gfgtdf, "In all cases where the acting unit is sighted, a (single) secondary unit is chosen from the sighting team. This choice should be considered arbitrary, but units within their sight range of the acting unit are chosen in preference to units further away." presumably applies in that case
20170414 16:53:36-!- JyrkiVesterinen [~JyrkiVest@87-92-12-90.bb.dnainternet.fi] has joined #wesnoth-dev
20170414 16:54:09< vultraz_iOS> gfgtdf: not really an improvement except we get thread safety (not needed r/n) and consistency with other code
20170414 16:54:55< gfgtdf> vultraz_iOS: thread saveftey also impled perormfnac eloss since threading sync doesn't come for free (not sure if it matters here)
20170414 16:55:37< gfgtdf> s/perormfnac eloss/performance loss
20170414 16:56:13< JyrkiVesterinen> The performance loss caused by thread safety is small. I wouldn't worry about it.
20170414 16:57:21< gfgtdf> zookeeper: rigth i just saw that in the code, i'm a bit surpised
20170414 16:57:23-!- oldlaptop [~quassel@45.63.78.126] has joined #wesnoth-dev
20170414 16:58:06 * celticminstrel is still dubious of vultraz_iOS's unit map work...
20170414 16:58:36< celticminstrel> Personally I'd think removing the unit_pod first would be better?
20170414 16:59:16< celticminstrel> zookeeper, gfgtdf: I forget, do sighted events fire once per unit sighted?
20170414 16:59:57< zookeeper> what do you mean? like, can the same event fire multiple times for the same unit, if it is revealed, hidden, revealed again, etc?
20170414 17:00:13< celticminstrel> I mean if multiple units are revealed at the same time.
20170414 17:00:47< zookeeper> yeah, once per unit
20170414 17:04:16< celticminstrel> vultraz_iOS: So I get the impression that unit_ptr is "unit*" while unit_pod is "unit**"
20170414 17:04:27< vultraz_iOS> ptr to ptr?
20170414 17:04:31< celticminstrel> ie it's reference-counting the unit and also reference-counting the pointer to the unit.
20170414 17:04:33< celticminstrel> Yes.
20170414 17:05:37< vultraz_iOS> alright, so what does that tell us
20170414 17:05:51< celticminstrel> Hmm, is it safe to store iterators into a std::map?
20170414 17:06:21< vultraz_iOS> alright, this is not as simple as I hoped
20170414 17:06:46-!- JyrkiVesterinen_ [~JyrkiVest@87-92-12-90.bb.dnainternet.fi] has joined #wesnoth-dev
20170414 17:07:22< vultraz_iOS> any action causes a crash :P
20170414 17:07:34< celticminstrel> Aha, so associative containers don't invalidate any iterators on insert. I didn't know that.
20170414 17:07:47< gfgtdf> vultraz_iOS: after replacing with shared_ptr or rmeoving pos count ?
20170414 17:07:56< vultraz_iOS> replacing shared_ptr
20170414 17:08:07< celticminstrel> I guess that makes sense, since a binary tree is similar to a linked list.
20170414 17:08:15< celticminstrel> In that each node stores pointers to the next node(s).
20170414 17:08:47< celticminstrel> This doesn't actually mention unordered maps... so those might invalidate iterators...
20170414 17:09:20-!- JyrkiVesterinen [~JyrkiVest@87-92-12-90.bb.dnainternet.fi] has quit [Ping timeout: 260 seconds]
20170414 17:09:20-!- JyrkiVesterinen_ is now known as JyrkiVesterinen
20170414 17:09:29< vultraz_iOS> https://github.com/Vultraz/wesnoth/commit/3fa46418899bddfc22ee8f44a0890b823c513250 basic stuff
20170414 17:09:31< vultraz_iOS> now to debug..
20170414 17:11:57< vultraz_iOS> #0 0x10dbf67	std::_Rb_tree<map_location, std::pair<map_location const, bool>, std::_Select1st<std::pair<map_location const, bool> >, std::less<map_location>, std::allocator<std::pair<map_location const, bool> > >::_M_erase(std::_Rb_tree_node<std::pair<map_location const, bool> >*) () (??:??)
20170414 17:11:57< vultraz_iOS> #1 0x167d3bc	?? () (??:??)
20170414 17:12:01< vultraz_iOS> well this tells me nothing...
20170414 17:12:36< gfgtdf> ye not really descriptive
20170414 17:12:53< gfgtdf> did you cimpile in debug build ?
20170414 17:13:05< JyrkiVesterinen> Probably a broken stack trace. A debug build would likely produce a useful stack.
20170414 17:13:15< gfgtdf> i actuall also have a small clenaup of the unit_map stuff currently, but it shodul cinflict with your i tink
20170414 17:13:16-!- mkdr0id [~null@unaffiliated/matthiaskrgr] has joined #wesnoth-dev
20170414 17:13:17< celticminstrel> So unordered maps/sets do invalidate iterators unpredictably on insert but never on erase.
20170414 17:14:17 * vultraz_iOS make debug build
20170414 17:15:00< celticminstrel> Hmm. I suspect shared_ptr can't be used here.
20170414 17:15:20< celticminstrel> Or wait.
20170414 17:15:26< celticminstrel> Adding a unit to the map copies it?
20170414 17:15:39< celticminstrel> Okay, then shared_ptr should be safe.
20170414 17:16:24< celticminstrel> But vultraz's crash was during erase, huh...
20170414 17:17:34-!- mkdroid [~null@unaffiliated/matthiaskrgr] has quit [Ping timeout: 260 seconds]
20170414 17:18:05< celticminstrel> Several functions call erase - move, replace, extract, etc.
20170414 17:18:49< gfgtdf> celticminstrel: actuall i'm abnotu to remove the 'ading a unit to the map coped it' thing, meaing replace all cases of unit_map::add with unit_map::insert
20170414 17:18:57< gfgtdf> i'm about to*
20170414 17:19:09< celticminstrel> Well, that would conflict with what Vultraz is doing then.
20170414 17:19:26< celticminstrel> Or, wait, if you're removing add altogether, I guess it wouldn't.
20170414 17:22:52< gfgtdf> i'm trying to see what temporary_unit_placer is used for, anyone knows ?
20170414 17:26:34-!- mkdr0id [~null@unaffiliated/matthiaskrgr] has quit [Ping timeout: 240 seconds]
20170414 17:27:19< celticminstrel> gfgtdf: Looks like it's not used.
20170414 17:27:51< gfgtdf> celticminstrel: ok this means i don't think about what to do with the .add call in it
20170414 17:32:30< vultraz_iOS> huh..
20170414 17:32:34< vultraz_iOS> now it says it's this?
20170414 17:32:48< vultraz_iOS> #0 0x1522445	std::swap<halo::halo_record*>(__a=@0x22fbcce8: 0x0, __b=@0xfeeeff12: <error reading variable>) (C:/TDM-GCC-32/lib/gcc/mingw32/5.1.0/include/c++/bits/move.h:186)
20170414 17:33:12< celticminstrel> Probably varies depending on what you do?
20170414 17:39:02< vultraz_iOS> well definitely different when creating a unit
20170414 17:39:08< vultraz_iOS> but this is when moving a unit..
20170414 17:40:24< vultraz_iOS> god linking with a debug takes forever..
20170414 17:48:08< vultraz_iOS> well, it's definitely something to do with something invalid relating to ptrs..
20170414 17:48:12< vultraz_iOS> and the halo manager..
20170414 17:50:00< vultraz_iOS> but why is that so..
20170414 17:50:28< celticminstrel> Is shared_from_this being called an an unshared unt?
20170414 17:50:48< celticminstrel> Oh BTW, did you inherit privately from enable_shared_from_this like I recommended?
20170414 17:50:53< vultraz_iOS> doesn't work
20170414 17:51:02< celticminstrel> Why not?
20170414 17:51:15< vultraz_iOS> "has inaccessible base" when 'new unit' is used
20170414 17:51:42< celticminstrel> ...
20170414 17:51:55< vultraz_iOS> what?
20170414 17:51:59< celticminstrel> That doesn's make sense?
20170414 17:52:17< celticminstrel> Having a private base doesn't prevent instantiating the class...
20170414 17:52:51< celticminstrel> Was this error in the unit map somewhere or in some random location in the code?
20170414 17:53:04< vultraz_iOS> some random location
20170414 17:54:02< vultraz_iOS> hmmm
20170414 17:54:07< vultraz_iOS> this is the unit mover...
20170414 17:54:19< vultraz_iOS> which uses fake_unit_ptr..
20170414 17:54:22< vultraz_iOS> which wraps a unit_ptr..
20170414 17:55:04< vultraz_iOS> but ..
20170414 17:55:05< vultraz_iOS> hm..
20170414 17:55:09< vultraz_iOS> this shouldn't do anything?
20170414 17:55:11< celticminstrel> So...
20170414 17:55:28< celticminstrel> When you got "has inaccessible base"...
20170414 17:55:42< celticminstrel> Was it something like "unit* u = new unit" or more like "unit_ptr u(new unit)"?
20170414 17:56:14< vultraz_iOS> latter
20170414 17:56:24< celticminstrel> Okay, so then declaring shared_ptr a friend might fix that.
20170414 17:56:37< vultraz_iOS> hmm..
20170414 17:56:42< celticminstrel> But anyway that's probably not the priority right now.
20170414 17:56:57< vultraz_iOS> this is interesting
20170414 17:57:14< vultraz_iOS> so fake_unit_ptr::operator-> just returns the value
20170414 17:57:18< vultraz_iOS> even though the comments say deref..
20170414 17:57:32< celticminstrel> ???
20170414 17:57:34< vultraz_iOS> before it would just be an intrusive_ptr being returned..
20170414 17:58:06< celticminstrel> Oh.
20170414 17:58:18< vultraz_iOS> https://github.com/wesnoth/wesnoth/blob/master/src/fake_unit_ptr.hpp#L49-L50
20170414 17:58:36< celticminstrel> That's how you overload operator->, yes.
20170414 17:58:41< vultraz_iOS> this class confuses me
20170414 17:58:57< vultraz_iOS> why are .get() and * returning different things...
20170414 17:59:03< celticminstrel> The return value must be either a raw pointer or a class that itself overloads operator->
20170414 17:59:06< vultraz_iOS> oh wait
20170414 17:59:07< vultraz_iOS> dur
20170414 17:59:09< vultraz_iOS> of course..
20170414 17:59:22< celticminstrel> Oh wow, github syntax hiliting failed on lin 66. XD
20170414 17:59:25< celticminstrel> ^line
20170414 17:59:53< vultraz_iOS> so is it correct to return the shared_ptr here?
20170414 18:00:25< celticminstrel> Definitely don't return raw pointers to something that might be in a shared_ptr.
20170414 18:02:59< vultraz_iOS> ok let's do something different..
20170414 18:03:07< vultraz_iOS> try recruiting..
20170414 18:03:23< vultraz_iOS> the unit appears... but the game crashes before it registers on the map
20170414 18:04:02< vultraz_iOS> https://pastebin.com/ueiNTKSn
20170414 18:04:16< vultraz_iOS> eehhh??
20170414 18:04:56< JyrkiVesterinen> Maybe double free?
20170414 18:04:57< vultraz_iOS> why in hell is clear being called
20170414 18:05:43< gfgtdf> vultraz_iOS: becasue unit_maps dtor is called
20170414 18:05:48< gfgtdf> destructor*
20170414 18:05:50< vultraz_iOS> yes
20170414 18:06:39< vultraz_iOS> hmm
20170414 18:06:41< vultraz_iOS> i have an idea..
20170414 18:13:45-!- turupawn [~turupawn@190.92.33.32] has quit [Ping timeout: 268 seconds]
20170414 18:14:19< vultraz_iOS> hmm
20170414 18:14:37< vultraz_iOS> i thought maybe the reset calls in unit_map::clear were causing double free but it still crashes..
20170414 18:15:10< celticminstrel> reset calls shouldn't cause double free.
20170414 18:15:23< celticminstrel> When you reset, the pointer becomes null, so a second reset is a no-op.
20170414 18:15:26< vultraz_iOS> well the map they're in is then cleared
20170414 18:15:48< celticminstrel> Is it clearing the ordered or unordered map?
20170414 18:16:06< celticminstrel> Or wait, I guess it'd be both.
20170414 18:16:10< JyrkiVesterinen> If the same pointer ever gets managed by two shared_ptrs at the same time, then resetting/destroying the first one frees the pointer, and resetting/destroying the second one causes a double free.
20170414 18:16:17< vultraz_iOS> both
20170414 18:17:41< vultraz_iOS> perhaps i should check use_count
20170414 18:42:41-!- Greg-Boggs [~greg_bogg@173.240.241.83] has quit [Remote host closed the connection]
20170414 18:43:07-!- Greg-Boggs [~greg_bogg@173.240.241.83] has joined #wesnoth-dev
20170414 18:43:21< vultraz_iOS> i can't figure this out
20170414 18:46:43-!- atarocch [~atarocch@2.43.52.169] has joined #wesnoth-dev
20170414 18:48:17-!- Greg-Boggs [~greg_bogg@173.240.241.83] has quit [Ping timeout: 268 seconds]
20170414 18:55:03< vultraz_iOS> well..
20170414 18:55:16< vultraz_iOS> adding the check for use_count() > 0 now takes me back to the damn halo stuff
20170414 18:56:38< vultraz_iOS> wait a minute
20170414 18:56:39< vultraz_iOS> temp_unit_ptr_ = fake_unit_ptr(unit_ptr(new unit(*u)), resources::fake_units);
20170414 18:57:01< vultraz_iOS> so what does that expand to...
20170414 18:57:54-!- turupawn [~turupawn@179.49.119.217] has joined #wesnoth-dev
20170414 18:57:54< vultraz_iOS> temp_unit_ptr is a fake_unit_ptr whose ctor is passed a new shared_ptr created from dereferencing another shared_ptr..
20170414 18:57:57< vultraz_iOS> celticminstrel: is this safe?
20170414 18:59:04 * vultraz_iOS tries shared_from_this
20170414 19:01:50< vultraz_iOS> nope, doesn't fix it..
20170414 19:03:33< vultraz_iOS> the ctors takes that shared_ptr by reference..
20170414 19:03:47< vultraz_iOS> but then copies it internally...
20170414 19:04:03< celticminstrel> Copying a shared_ptr is fine.
20170414 19:04:26< celticminstrel> And std::make_shared<unit>(*ptr) would also be fine.
20170414 19:05:29< vultraz_iOS> hmmm
20170414 19:05:36< vultraz_iOS> there's a reset call in this class..
20170414 19:05:40< vultraz_iOS> let's check for validness...
20170414 19:08:36< vultraz_iOS> nope...
20170414 19:10:58< vultraz_iOS> let's remove the reset completely
20170414 19:11:46< vultraz_iOS> 	// The fake_unit class exists for this one line, which removes the
20170414 19:11:46< vultraz_iOS> 	// fake_unit from the managers's fake_units_ dequeue in the event of an
20170414 19:11:46< vultraz_iOS> 	// exception.
20170414 19:11:47< vultraz_iOS> whut
20170414 19:17:58-!- stikonas [~gentoo@wesnoth/translator/stikonas] has quit [Quit: Konversation terminated!]
20170414 19:18:57-!- stikonas [~gentoo@wesnoth/translator/stikonas] has joined #wesnoth-dev
20170414 19:18:58< vultraz_iOS> yeah i can't figure this out
20170414 19:19:28< celticminstrel> I suggest abandoning it for now. The intrusive_ptr works, and there are more important things to worry about.
20170414 19:19:50-!- irker284 [~irker@uruz.ai0867.net] has quit [Quit: transmission timeout]
20170414 19:24:22-!- stikonas [~gentoo@wesnoth/translator/stikonas] has quit [Remote host closed the connection]
20170414 19:28:29-!- irker825 [~irker@uruz.ai0867.net] has joined #wesnoth-dev
20170414 19:28:29< irker825> wesnoth: Celtic Minstrel wesnoth:frame_cleanup 242afba609d0 / src/units/ (animation_component.cpp drawer.cpp frame.cpp frame.hpp): Templatize the unit animation frame parameters https://github.com/wesnoth/wesnoth/commit/242afba609d0f82dc38ed484f56ccebdff886a66
20170414 19:31:58< celticminstrel> vultraz_iOS: Please review again.
20170414 19:34:55-!- turupawn [~turupawn@179.49.119.217] has quit [Read error: No route to host]
20170414 19:36:05< celticminstrel> Would be nice if others also took a look - gfgtdf or Jyrki maybe?
20170414 19:36:30< JyrkiVesterinen> I'm not going to take a look tonight. I'll go to bed soon.
20170414 19:37:56< celticminstrel> And just FTR vultraz_iOS, don't merge it.
20170414 19:38:01< celticminstrel> Just review.
20170414 19:39:59< vultraz_iOS> Looks alright st first glance
20170414 19:40:14< celticminstrel> Good, good.
20170414 19:40:34< celticminstrel> Unfortunately it breaks everything, so I need to fix that before merging.
20170414 19:41:49< vultraz_iOS> I'll look more later
20170414 19:42:37< gfgtdf> made pr #985 for less unti coypign how it doens conflict with vultraz work.
20170414 19:43:02< gfgtdf> unit*
20170414 19:43:29< celticminstrel> gfgtdf: Mind looking at my commit that I jsut pushed there?
20170414 19:43:57< celticminstrel> ^just
20170414 19:46:33< gfgtdf> celticminstrel: still don't think it build it looks liek at multipl places, for examepl here https://github.com/wesnoth/wesnoth/commit/242afba609d0f82dc38ed484f56ccebdff886a66#diff-5eac1472101c1f3f68f6c9ce39068934R345 is lacks the base:: specifier.
20170414 19:48:19< gfgtdf> celticminstrel: i wonder why you refer to the base class sometimes as base:: and sometimes explicitly as builder_frame_parameters::
20170414 19:48:35< celticminstrel> I used an alias for the constructor where it was referenced a lot.
20170414 19:48:43< celticminstrel> But didn't make one for the builder functions.
20170414 19:49:10< gfgtdf> celticminstrel: you could also just put the alias just in the header class definition if you want to.
20170414 19:49:18< celticminstrel> The line you reference is in frame_parsed_parameters which doesn't have all the builder functions, so only duration is hidden.
20170414 19:49:24< celticminstrel> I did consider that actually.
20170414 19:49:45< celticminstrel> I also used the qualifiers even where it wasn't necessary.
20170414 19:50:05< celticminstrel> Only the ones that match the name of the builder function need to be qualified.
20170414 19:50:47< gfgtdf> hmm right
20170414 19:51:15< celticminstrel> But IMO it looks better for uniformity to qualify all of them.
20170414 19:52:21< gfgtdf> i perosnalyl woudl problaly just left the trainilg underscores so that no name conflics appear.
20170414 19:52:35< celticminstrel> I could do that.
20170414 19:52:46< celticminstrel> But if I do that, then the commit touches many more files.
20170414 19:52:59< celticminstrel> Because suddenly everywhere frame_parsed_parameters is used I need to add the underscores.
20170414 19:53:20< celticminstrel> Uh.
20170414 19:53:24< celticminstrel> frame_parameters, I mean.
20170414 19:54:47-!- stikonas [~gentoo@wesnoth/translator/stikonas] has joined #wesnoth-dev
20170414 19:57:07< celticminstrel> gfgtdf: What about retaining add() / replace() but making them take a unit_ptr instead of a unit&?
20170414 19:58:11< gfgtdf> celticminstrel: i doint remove add/replace yet since there are still2 cases that use it (in editor/ and in temporariy_unit_placer)
20170414 19:59:16< celticminstrel> I meant maybe they should be changed to take a pointer.
20170414 19:59:51< celticminstrel> Or maybe an overload of insert added that takes a location.
20170414 19:59:59< celticminstrel> insert(loc, unitptr)
20170414 20:00:05< gfgtdf> celticminstrel: that that woudl make sense
20170414 20:00:30< celticminstrel> I mean I guess it's not a big deal that you have to set the location manually now, but...
20170414 20:03:13< irker825> wesnoth: Celtic Minstrel wesnoth:frame_cleanup 06c9e59065de / src/units/ (animation_component.cpp drawer.cpp frame.cpp frame.hpp): Templatize the unit animation frame parameters https://github.com/wesnoth/wesnoth/commit/06c9e59065de3dc43bdcc48efa42abd910d4a7c9
20170414 20:03:16< celticminstrel> ^ just fixing a -Wreorder
20170414 20:04:35-!- clavi [~clavi@v22017034422546657.goodsrv.de] has quit [Quit: ZNC - http://znc.in]
20170414 20:09:04-!- clavi [~clavi@v22017034422546657.goodsrv.de] has joined #wesnoth-dev
20170414 20:09:53-!- JyrkiVesterinen [~JyrkiVest@87-92-12-90.bb.dnainternet.fi] has quit [Quit: .]
20170414 20:38:05-!- atarocch [~atarocch@2.43.52.169] has quit [Ping timeout: 240 seconds]
20170414 20:38:29-!- atarocch [~atarocch@2.43.52.169] has joined #wesnoth-dev
20170414 20:39:14< wedge009> celticminstrel: https://gna.org/bugs/?14503
20170414 20:40:20< gfgtdf> vultraz_iOS: the gui2 lobby has problem displaying games that are played with a lot of modifications.
20170414 20:41:13< gfgtdf> vultraz_iOS: basically it puts all modificatiosn in in one line so that the gameslists gets a horizonal scrollbar if a game has many modifications
20170414 20:42:06< gfgtdf> vultraz_iOS: this is specialyl a problem becasue it's then no longer possible to see the 'join' button and the map/name images at the same time
20170414 20:42:53-!- Greg-Boggs [~greg_bogg@173.240.241.83] has joined #wesnoth-dev
20170414 20:43:59< celticminstrel> Ah, I get it now.
20170414 20:49:47-!- stikonas [~gentoo@wesnoth/translator/stikonas] has quit [Remote host closed the connection]
20170414 20:50:24-!- stikonas [~gentoo@wesnoth/translator/stikonas] has joined #wesnoth-dev
20170414 20:56:41-!- Kwandulin [~Kwandulin@p200300760F6D8067C106474B65342408.dip0.t-ipconnect.de] has quit [Quit: Miranda IM! Smaller, Faster, Easier. http://miranda-im.org]
20170414 21:14:10-!- atarocch [~atarocch@2.43.52.169] has quit [Remote host closed the connection]
20170414 21:39:09-!- wedge009 [~Thunderbi@60-241-236-92.static.tpgi.com.au] has quit [Quit: wedge009]
20170414 21:41:29-!- wedge009 [~Thunderbi@60-241-236-92.static.tpgi.com.au] has joined #wesnoth-dev
20170414 21:43:25-!- stikonas [~gentoo@wesnoth/translator/stikonas] has quit [Remote host closed the connection]
20170414 21:44:27-!- stikonas [~gentoo@wesnoth/translator/stikonas] has joined #wesnoth-dev
20170414 21:45:17< irker825> wesnoth: gfgtdf wesnoth:master c4625197d7c6 / src/ (6 files in 4 dirs): less unit copying https://github.com/wesnoth/wesnoth/commit/c4625197d7c6d0f37f11a0cc3d442204640b5582
20170414 22:07:47-!- vultraz_iOS [uid24821@wesnoth/developer/vultraz] has quit [Quit: Connection closed for inactivity]
20170414 22:09:05-!- zookeeper [~lmsnie@wesnoth/developer/zookeeper] has quit [Ping timeout: 252 seconds]
20170414 22:23:13-!- boucman [~rosen@wesnoth/developer/boucman] has quit [Remote host closed the connection]
20170414 22:35:44-!- stikonas [~gentoo@wesnoth/translator/stikonas] has quit [Remote host closed the connection]
20170414 22:36:46-!- stikonas [~gentoo@wesnoth/translator/stikonas] has joined #wesnoth-dev
20170414 23:12:09-!- gfgtdf [~chatzilla@x4e363021.dyn.telefonica.de] has quit [Read error: Connection reset by peer]
20170414 23:50:49-!- Greg-Boggs [~greg_bogg@173.240.241.83] has quit [Remote host closed the connection]
20170414 23:51:26-!- Greg-Boggs [~greg_bogg@173.240.241.83] has joined #wesnoth-dev
20170414 23:55:48-!- Greg-Boggs [~greg_bogg@173.240.241.83] has quit [Ping timeout: 260 seconds]
--- Log closed Sat Apr 15 00:00:53 2017