Times are UTC Toggle Colours
09:14:13 *** Alberth has joined #openttd.dev 09:14:13 *** ChanServ sets mode: +v Alberth 11:16:32 <fonsinchen> I'm going to rewrite the smallstack without using Pool. It's impossible to verify that it's used correctly like this. 11:17:19 *** DorpsGek changes topic to "OpenTTD Dev Channel || Latest SVN: r26341 || Logs: http://webster.openttdcoop.org/?channel=openttd.dev || Voice (talk-right) upon request via #openttd; make sure you are registered to NickServ before asking" 11:31:18 <planetmaker> I think we should also allow full svn check-outs and not fail version checks when building tags (releases) from those: http://devs.openttd.org/~planetmaker/patches/index.php?source=svn_version_tags.diff 11:36:30 <fonsinchen> Can we be sure that those are enough '../'? 11:37:37 <planetmaker> I only add a check for yet another level higher 11:37:54 <planetmaker> ../ in / is the same as / 11:40:48 <fonsinchen> It will fail if the user puts a partial non-svn checkout of openttd into an unrelated svn checkout. That's a fairly pathetic case, thouh. 11:41:49 <LordAro> i think that goes into the realms of "stupid user is stupid" 11:42:06 <planetmaker> that already can fail 11:42:17 <planetmaker> but that's indeed a rather pathetic case, I think 11:42:41 <planetmaker> it's more that I could not build a release version from my existing svn checkout 11:43:41 <planetmaker> though a complete svn checkout is also somewhat pathetic, I think there are more users who use that (even somewhat legitimately) than those who have a .svn two levels higher than their openttd svn checkout 11:43:44 <fonsinchen> But now it can fail on two levels up from openttd's root dir. We could (semi-) fix that by checking for something specific to openttd in that directory. 11:44:21 <fonsinchen> Whatever, if it helps, it's OK for me. 11:45:05 <planetmaker> well, it's still guarded by the other conditions which still must be fulfilled: && [ -n "`LC_ALL=C svn info $ROOT_DIR/.. | grep '^URL:.*tags$'`" ] 11:45:29 <planetmaker> thus you must be in a dir which has 'tags' in the path 11:45:41 <planetmaker> which is pretty openttd-specific for the releases 11:50:46 <planetmaker> I actually would want to know from Rubidium why a "full svn checkout (is) discouraged" 11:51:02 <planetmaker> except that there's of course little reason to have that for most people 12:32:25 *** Supercheese has quit IRC 12:32:57 *** Supercheese has joined #openttd.dev 12:32:57 *** ChanServ sets mode: +v Supercheese 12:36:55 *** frosch123 has joined #openttd.dev 12:36:55 *** ChanServ sets mode: +v frosch123 13:05:23 *** Zuu has joined #openttd.dev 13:05:23 *** ChanServ sets mode: +v Zuu 13:05:42 *** Zuu has left #openttd.dev 13:07:34 <Rubidium> planetmaker: you really need the 131 other non-1.3.3/1.4.0-beta4 subversion checkouts? Of which probably half don't even compile? 13:07:34 <Rubidium> generally, if you want to check whether something was backported, the stable branch is better anyhow 13:08:40 <planetmaker> "need" is relative. But yes, I have many of those and I sometimes use one or the other to check something 13:08:51 <planetmaker> difficult if it doesn't compile, though, indeed 13:11:28 * Rubidium only has the .0 release and stable branches of anything < 1.3.3 13:12:09 <Alberth> just download the source distribution? 13:13:20 <planetmaker> ok, is there any problem other than "I don't use it"? 13:40:40 <planetmaker> but I really don't get why it's now a bad idea, Rubidium 14:05:37 <fonsinchen> In fact the smallstack leaks. In its destructor it only deletes the first item beyond the head, not the full stack. 14:06:10 <planetmaker> oi. 14:07:52 <fonsinchen> You need a pretty complex game with lots of nested conditional orders for this to be a problem though. 14:10:48 <planetmaker> hm, this behaviour is new to me... is there a max number of open windows? 14:12:19 <planetmaker> I cannot open more than 9 concurrent vehicle windows 14:12:39 <fonsinchen> That's not new. I've run into that quite a few times. 14:14:47 <planetmaker> so if the conditional orders are required for a desync based on the smallstack leak, it's not the reason in the game we had running on welcome server 14:14:57 <planetmaker> I didn't find a train with conditional orders in any company 14:15:59 <fonsinchen> I can't imagine it causes a desync. It could cause a crash when the 64k smallstack items run out. 14:16:00 <frosch123> planetmaker: there is an advanced setting to limit the amount of non-sticky windows 14:17:37 <planetmaker> wow, another setting I can't remember :D 14:17:58 <frosch123> before 0.7 or so there was a fixed amount of windows 14:18:16 <frosch123> it was then changed to a configurable soft limit 14:18:58 <planetmaker> that's long ago :) 14:20:49 <frosch123> fonsinchen: does the order of items in the pool matter somehow? 14:21:26 <frosch123> if the server has more than the clients, the clients will assign different ids than the server 14:21:59 <fonsinchen> I don't quite get that ... 14:22:24 <fonsinchen> The server can assign different IDs than the clients without that being a desync? 14:23:15 <fonsinchen> But the order of items doesn't matter. It's basically a linked list with custom new/delete. 14:24:23 <frosch123> ah, i guess even the leaked items will still be saved, so also exist on the clients after load 14:24:42 <frosch123> or is that pool not saved? 14:26:23 <fonsinchen> It's not saved 14:26:32 <fonsinchen> At least I hope so 14:26:48 <fonsinchen> I'm currently rewriting it to not use a pool at all. 15:41:29 <fonsinchen> https://github.com/ulfhermann/openttd/commit/854b855a593c70da9bdb3a281741239998856e27 ... 15:41:43 <fonsinchen> https://github.com/ulfhermann/openttd/commit/29a86a3d71c80ec0d2062cc4718f916bb0390490 15:41:54 <fonsinchen> To fix the smallstack problem 15:42:06 <frosch123> he, i just thought earlier to write a mutex lockguard 15:42:56 <frosch123> rename ThreadMutexLocker to TheadMutexLockGuard? afaik "lock guard" is the common name for that 15:43:24 <frosch123> yeah, there is a std::lock_guard in c++11 15:45:40 <fonsinchen> Qt has MutexLocker 15:45:49 <fonsinchen> QMutexLocker that is 15:46:03 <frosch123> ok, i never really used qt 15:46:50 <LordAro> ew, Qt 15:46:56 <LordAro> woo, c++11 15:46:58 <LordAro> :) 15:47:49 <frosch123> do you have a highlight on c++11 ? 15:48:07 <fonsinchen> I think he has a highlight on Qt :) 15:48:55 <frosch123> really? but qt rarely refers to actual girls 15:49:30 <fonsinchen> That's why he says "ew" everytime he sees it. 15:52:23 <Rubidium> planetmaker: what percentage of users will need a full checkout? They generally only need one version, so generally... it's discouraged 15:52:52 <frosch123> fonsinchen: to nitpick: the smallstack is only "thread-safe", not "reentrant" 15:53:14 <fonsinchen> thread-safe is more than reentrant 15:53:15 <planetmaker> Rubidium, yes, discourage. But have it fail to deliver a usable source? 15:53:30 <frosch123> what? reentrant is more than thread-safe 15:53:32 <fonsinchen> reentrant means you can use different objects of it in different threads 15:53:34 <Rubidium> planetmaker: that's because subversion changed its format 15:53:43 <Rubidium> ... of working copies 15:53:46 <fonsinchen> thread-safe means you can use the same object concurrently 15:54:04 <frosch123> reentrant means you can interrupt any thread at any time, and use the thing 15:54:21 <Rubidium> planetmaker: and when I made the 'fix' I genuinely believed I was more or less the only one needing it for the tags as you can see in the configure comment 15:55:47 <planetmaker> yes, I do see that. And I agree the amount of people who have some reason to use it is about as limited as the people in this channel, rather less. Yet... 15:56:59 <planetmaker> ... does it hurt anywhere? 15:57:17 <planetmaker> currently, if someone got full svn, s/he will need to checkout parts of it again 15:57:51 <Rubidium> for what it's worth, I have a "shallow" / mostly "empty" checkout of tags, so I only needed it to look one path higher. From that I usually remove "old" tags after a few weeks. 15:58:19 <Rubidium> the branches are just separate checkouts 15:58:23 <fonsinchen> reentrancy is kind of a convoluted explanation of the concept. You can of course interrupt any call to any method of SmallStack and later continue it if you don't modify the same smallstack in between. 15:58:45 <fonsinchen> thread-safe means you can also do that while using the same smallstack. 15:59:06 <planetmaker> well, I have one svn, where I usually only update trunk. or in the tags dir what I need 15:59:10 <fonsinchen> So my implementation is reentrant, but not thread-safe. 15:59:42 <frosch123> i thought the reverse :p i learned "mutexes make code thread-safe, but never reentrant" 15:59:52 <Rubidium> planetmaker: feel free to add an extra level, though be sure to not copy the "(tags)" comment as that won't be right 16:00:22 <fonsinchen> Well, I made the calls to the underlying pool thread safe with mutexes 16:00:26 <frosch123> mutexes protect other threads from interfering, but not the same thread from interfering with itself 16:00:41 <frosch123> but well, does not matter 16:02:02 <LordAro> fonsinchen, frosch123: D: 16:02:07 <LordAro> but no, i've just been lurking 16:07:48 <frosch123> + if (index < Tmax_size) { <- shouldn't that be an assert in Create? 16:08:09 <frosch123> Push seems to just ignore the push when the pool is filled 16:08:17 <fonsinchen> See the documentation of Push 16:08:21 <frosch123> hmm, yeah :p 16:08:46 <fonsinchen> it is in fact possible to create pathetic cases with more than 64k branches in conditional orders. 16:08:58 <fonsinchen> I've decided to ignore that then. 16:11:26 <fonsinchen> You're probably right in that I shouldn't call this reentrant 16:12:22 <fonsinchen> The definitions of reentrancy differ, though: http://qt-project.org/doc/qt-5/threads-reentrancy.html 16:14:32 <Rubidium> as long as the function doesn't have an internal state it's reentrant by definition, right? 16:14:42 <Rubidium> e.g. max/min/abs is reentrant 16:15:14 <frosch123> "Note: Terminology in the multithreading domain isn't entirely standardized. POSIX uses definitions of reentrant and thread-safe that are somewhat different for its C APIs. When using other object-oriented C++ class libraries with Qt, be sure the definitions are understood." <- from that page :) 16:15:47 <frosch123> so, qt's definiton of reentrant is very different to the posix one 16:16:43 <fonsinchen> apparently 16:16:57 <fonsinchen> Do we have a better name for what they call reentrant? 16:17:35 <Rubidium> IMHO reentrant functions are functions that do not modify (global) state 16:18:04 <frosch123> that's the posix definition 16:18:36 <frosch123> hmm, kind of both :p 16:18:39 <Rubidium> e.g. anything that touches errno isn't reentrant, since if you call it from two threads... even when they are utterly different (e.g. writing file A and reading file B), because upon error you wouldn't know what error you actually got 16:20:21 <frosch123> fonsinchen: diff looks fine 16:24:44 *** DorpsGek changes topic to "OpenTTD Dev Channel || Latest SVN: r26342 || Logs: http://webster.openttdcoop.org/?channel=openttd.dev || Voice (talk-right) upon request via #openttd; make sure you are registered to NickServ before asking" 16:25:19 *** DorpsGek changes topic to "OpenTTD Dev Channel || Latest SVN: r26343 || Logs: http://webster.openttdcoop.org/?channel=openttd.dev || Voice (talk-right) upon request via #openttd; make sure you are registered to NickServ before asking" 16:39:44 <planetmaker> http://bugs.openttd.org/task/5907 <-- any objects? That makes sense to me and I didn't find anything which is negatively impacted 16:39:51 <planetmaker> *objections 16:43:42 <frosch123> i guess trees are often enough on shores to justify that opimisation 17:03:59 *** DorpsGek changes topic to "OpenTTD Dev Channel || Latest SVN: r26344 || Logs: http://webster.openttdcoop.org/?channel=openttd.dev || Voice (talk-right) upon request via #openttd; make sure you are registered to NickServ before asking" 17:29:04 <fonsinchen> YES. I finally can reproduce FS#5898 17:29:17 <planetmaker> https://bugs.openttd.org/task/5906 <-- makes sense to me, too 17:33:26 <frosch123> planetmaker: keep the stuff in InvalidateDimensionCache in the same order as in the defintion and constructions below? 17:33:33 <frosch123> that makes it easier to see that they are complete 17:34:39 <frosch123> it looks like whenever NWidgetLeaf::InvalidateDimensionCache(); is called, also NWidgetScrollbar::InvalidateDimensionCache(); is called 17:34:55 <frosch123> so, only calling one is likely wrong 17:35:02 <frosch123> is there maybe a more generic method to call? 17:35:15 <planetmaker> same order definitely makes sense 17:35:41 <frosch123> isn't ReInitAllWindows called somewhere? 17:35:53 <frosch123> if the sprite sizes really change, then reinit should be called 17:36:11 <planetmaker> the bug is real, though 17:36:21 <frosch123> but well, i cannot survey all call paths 17:36:22 <planetmaker> that sprite's width doesn't change 17:36:32 <planetmaker> ok, I shall look 17:36:36 <frosch123> that does not say that the fix is correct :p 17:36:49 <frosch123> the hunk in widget.cpp is ceratinly correct 17:37:04 <frosch123> i would claim that gfxinit is incorrect, though i do not know whether just calling the scrollbar thingie is enough 17:37:12 <frosch123> or whtehr it should be reinitallwindiows 17:37:23 <frosch123> or whether calling the latter would cause more troubles 17:38:50 <frosch123> anyway, it's the same patching place as fsä5867 17:39:14 <frosch123> i don't think that adding random functions to that place is fixing anything in long term 17:40:05 <planetmaker> hm 17:42:30 *** DorpsGek changes topic to "OpenTTD Dev Channel || Latest SVN: r26345 || Logs: http://webster.openttdcoop.org/?channel=openttd.dev || Voice (talk-right) upon request via #openttd; make sure you are registered to NickServ before asking" 17:45:25 *** DorpsGek changes topic to "OpenTTD Dev Channel || Latest SVN: r26346 || Logs: http://webster.openttdcoop.org/?channel=openttd.dev || Voice (talk-right) upon request via #openttd; make sure you are registered to NickServ before asking" 18:37:01 <fonsinchen> https://github.com/ulfhermann/openttd/commit/9231d2046001bb421e27183b83ea2bf5e8ad1ec8 for FS#5898 18:37:41 <fonsinchen> Basically I'm moving the thread handling into the jobs because when a saveload fails it nulls all pointers and the schedule won't know its jobs anymore 18:38:20 <fonsinchen> However, we can still get hold of them on pool clean later, so that's no real problem. 18:40:42 <frosch123> looks fine to me 18:43:10 *** DorpsGek changes topic to "OpenTTD Dev Channel || Latest SVN: r26347 || Logs: http://webster.openttdcoop.org/?channel=openttd.dev || Voice (talk-right) upon request via #openttd; make sure you are registered to NickServ before asking" 18:45:30 *** DorpsGek changes topic to "OpenTTD Dev Channel || Latest SVN: r26348 || Logs: http://webster.openttdcoop.org/?channel=openttd.dev || Voice (talk-right) upon request via #openttd; make sure you are registered to NickServ before asking" 19:41:36 <frosch123> hmm, we use SendSignal and WaitForSignal with posix behaviour 19:42:03 <frosch123> but thread_os2 and thread_win32 actually do not provide the posix behaviour for WaitForSignal 19:42:26 <frosch123> the signal waiting and critical section entering/leaving is not atomic 19:48:55 <planetmaker> https://bugs.openttd.org/task/5842 <-- hm... I'd have thought it might be fixed by invalidating also order windows... 19:52:58 <frosch123> you have to reinit or close them or something 19:53:26 <frosch123> the order window uses a different window_desc and widget tree depending on owned/competitor 19:53:33 <frosch123> it's the only window which does that :p 19:54:16 <frosch123> you either have to remove that magic from the order window by using nestedwidgetstacks 19:54:32 <frosch123> or you have to close all order windows, which may look weird :p 19:55:59 <planetmaker> yeah... and will quickly give the next bug report :P 19:57:05 <frosch123> oh, the win32 waitforsignal is actually fine 20:02:58 <frosch123> http://paste.openttdcoop.org/show/3107/ <- fixes the issues reported by helgrind with sdl 20:03:31 <frosch123> there were some weird assumptions which VideoDriver_SDL methods were called with which mutexes already acquired 20:03:51 <frosch123> this fixes it by making the mutex allowing recursive locking, and then always acquireing them 20:04:17 <frosch123> the os/2 and windows code is not tested whether it compiles :p 21:12:21 * Rubidium wonders whether OS/2 still compiles at all; guess you'd need to talk to orudge for that 21:14:08 <frosch123> iirc he some 1.4 beta 21:14:14 <frosch123> +compiled 21:14:25 <frosch123> but morphos looks weird 21:14:31 <frosch123> there is a thread class for it, but no mutex class 21:14:51 <planetmaker> who added / maintained morphos anyway? 21:14:55 <frosch123> tokai 21:15:16 <planetmaker> hm, saw him online the other day 21:15:23 <frosch123> but it stopped compiling around 0.6 when more c++ stuff got used 21:15:38 <frosch123> morphos had no newer compiler than gcc 2.95 for years 21:19:08 <Rubidium> frosch123: http://paste.openttdcoop.org/show/3108/ might be of interest for you 21:19:49 <frosch123> how boring :p 21:19:55 <frosch123> but thanks :) 21:20:03 <frosch123> saves a few iterations on the farm 21:20:44 <Rubidium> adding recursive_count removes all but two 21:21:06 <frosch123> yeah, need to add the default param also in the derived classes 21:32:57 *** Alberth has left #openttd.dev 21:37:13 *** DorpsGek changes topic to "OpenTTD Dev Channel || Latest SVN: r26349 || Logs: http://webster.openttdcoop.org/?channel=openttd.dev || Voice (talk-right) upon request via #openttd; make sure you are registered to NickServ before asking" 21:43:15 *** linuxman has joined #openttd.dev 21:47:00 *** DorpsGek changes topic to "OpenTTD Dev Channel || Latest SVN: r26350 || Logs: http://webster.openttdcoop.org/?channel=openttd.dev || Voice (talk-right) upon request via #openttd; make sure you are registered to NickServ before asking" 21:57:23 *** DorpsGek changes topic to "OpenTTD Dev Channel || Latest SVN: r26351 || Logs: http://webster.openttdcoop.org/?channel=openttd.dev || Voice (talk-right) upon request via #openttd; make sure you are registered to NickServ before asking" 21:57:53 <frosch123> now we only need to figure out whether the same applies to allegro, osx, windows, and what not :p 21:58:16 <LordAro> suspect you'll soon find out 21:59:10 <frosch123> planetmaker: btw. fs#5900 also one existed for sdl, because VideoDriver_SDL::ChangeResolution missed that mutex 21:59:23 <frosch123> but i have no idea wheather osx uses multithreaded drawing at all 21:59:36 <frosch123> LordAro: i had a task for you, but i do not remember what it was 21:59:51 <LordAro> :o 22:00:05 <LordAro> i know that i never finished the news item crashlog patch 22:00:18 <LordAro> that's pretty much the last time i did :( 22:00:35 <frosch123> well, there was also the "load heightmap/start scenario does not allow to view readme" 22:00:42 <frosch123> but i had another one a few days ago :p 22:03:43 *** frosch123 has quit IRC 23:03:05 *** linuxman has quit IRC