Times are UTC Toggle Colours
00:44:16 *** adf88 has joined #openttd.dev 00:44:16 *** ChanServ sets mode: +v adf88 00:44:33 *** adf88 has quit IRC 01:09:33 *** LordAro has quit IRC 06:36:09 *** Supercheese has quit IRC 08:23:08 *** zydeco has joined #openttd.dev 08:26:27 *** Alberth has joined #openttd.dev 08:26:27 *** ChanServ sets mode: +v Alberth 08:34:38 *** Ristovski has joined #openttd.dev 08:53:40 *** LordAro has joined #openttd.dev 08:53:40 *** ChanServ sets mode: +v LordAro 09:03:38 <Alberth> moin 09:03:54 <LordAro> /o 09:05:43 <Alberth> lol, head-2-head freerct? :) 09:06:20 <LordAro> just random ideas from my head :) 09:06:32 <LordAro> (sure you're in the right channel?) 09:07:05 <Alberth> hmm, good point, I am in the right window only :p 09:07:12 <LordAro> :p 09:13:46 <LordAro> https://www.dropbox.com/sh/t1dy6bux74088jg/OZVTffg9nK 09:14:05 *** adf88 has joined #openttd.dev 09:14:05 *** ChanServ sets mode: +v adf88 09:45:58 <fonsinchen> Hi 09:46:15 <fonsinchen> I'd like to do something about FS#5671 today 09:46:31 <fonsinchen> I have two alternative solutions available: 09:47:21 <fonsinchen> https://github.com/ulfhermann/openttd/commits/fixes2 09:47:47 <fonsinchen> Either only up to "use smallstack to allow for multiple next hops" 09:47:51 <fonsinchen> or the whole thing 09:48:00 <fonsinchen> I'm in favour of the simpler solution 09:48:38 <fonsinchen> It uses a pool for storing the elements of the stack and thus is not as nice for locality. 09:49:32 <fonsinchen> However, as the stack is currently only used in a relatively obscure case this shouldn't hurt 09:50:32 <fonsinchen> If someone comes up with more uses for such SmallStacks we still may rethink that decision. 10:04:12 <Alberth> refactor pool to make FindFirstFree and ResizeFor... 585e44e672979a824de9db17700 (not sure how to indicate these patches) 10:04:12 <Alberth> core/pool_type.hpp line 78 +struct Pool : PoolBase { <-- adds a second space before the "{" 10:06:18 <Alberth> I cannot say much about the contents itself; it looks ok to me 10:06:56 *** zooks has joined #openttd.dev 10:07:46 <Alberth> Given that you asked yesterday as well, apparently no dev has anything relevant to say 10:38:55 *** ntoskrnl has joined #openttd.dev 10:48:18 *** frosch123 has joined #openttd.dev 10:48:18 *** ChanServ sets mode: +v frosch123 11:01:23 *** adf88 has quit IRC 12:39:40 *** zooks has quit IRC 12:50:12 *** adf88 has joined #openttd.dev 12:50:12 *** ChanServ sets mode: +v adf88 12:50:25 <fonsinchen> pool_func.hpp:197 says that delete'ing NULL PoolItems is valid 12:50:47 <planetmaker> it is 12:50:54 <planetmaker> afaik according to c specs 12:51:19 <fonsinchen> However, pool_type.hpp:164ff dereferences pointers passed to operator delete() 12:51:50 <fonsinchen> I wonder why I'm the first one to hit this ... 12:52:22 <planetmaker> maybe you found the reason for one or few of the crashes in our bug tracker? 12:52:23 <frosch123> most likely 12:52:23 <fonsinchen> The spec probably assumes the default operator delete() 12:52:39 <frosch123> you usually access a pool item via the Get or GetIfValid methods 12:52:46 <frosch123> the former asserts if the item is invalid 12:52:54 <frosch123> so, you only ever get NULL from GetIfValid 12:53:01 <frosch123> and usually that is checked immediately 12:53:21 <fonsinchen> In my case it crashes in CleanPool 12:53:36 <frosch123> (my "most likely" refers to the "you are the first one to tirgger it") 12:53:37 <fonsinchen> That is, there is a "hole" in the pool, which should be common. 12:54:07 <fonsinchen> I can just add a check for NULL to operator delete 12:54:18 <frosch123> irrc there is some _cleaning_pool variable which skips deleteing of stuff which is deleted by other means 12:55:05 <fonsinchen> It's CleanPool itself which triggers it and the hole has been there before 12:55:35 <fonsinchen> Such a hole is created whenever you delete a PoolItem 12:55:54 <fonsinchen> (except for the last one in the pool) 12:58:06 <frosch123> CleanPool does not use PoolItem::delete 12:58:33 <fonsinchen> It does "delete this->Get(i)" 12:58:35 <frosch123> it uses TItem::delete 12:58:49 <frosch123> hmm, maybe that's the same 12:59:51 <fonsinchen> Titem should always be a PoolItem, shouldn't it? 12:59:54 <frosch123> looks weird to me 13:00:02 <frosch123> that should be triggered all the time, indeed 13:01:31 <adf88> hi there 13:01:58 <adf88> @planetmaker there has one thing left to correct in the orders window 13:02:18 <adf88> just remembering you 13:02:19 <adf88> www.dropbox.com/sh/j0whg8ho4uwm20s/hVzyv4cKOE 13:02:38 <adf88> it was about to unify different goto tools 13:03:29 <adf88> as the "normal" goto works little different then others when selecting from the dropdown 13:08:06 <fonsinchen> Probably CleanPool is usually called when the program terminates and nobody cares if it crashes then. But still, very weird 13:08:28 <frosch123> it's also called when returning to main screen 13:08:30 <frosch123> or when loading a game 13:08:37 <frosch123> in fact when starting ottd, and loading the title game 13:08:45 <planetmaker> indeed, adf88, let's see 13:09:18 <planetmaker> hm, why don't they have a .patch or .diff file extension :D 13:09:32 <adf88> is it important? 13:09:50 <adf88> ah 13:09:53 <planetmaker> no 13:10:01 <adf88> sorry, it is kind of discrimination 13:10:04 <adf88> :) 13:10:23 <adf88> if you know what I mean 13:10:31 <planetmaker> not really :-) 13:10:57 <fonsinchen> Could also be that holes in pools are not as common as we think. I'll do some testing. 13:11:55 <adf88> discrimination of OS'es that are basing file types on their extensions ;) 13:13:51 <Alberth> not to mention web-browsers :) 13:13:52 <planetmaker> oh. dropbox knows the extension and then allows syntax highlighting in the browser :-) 13:14:06 <adf88> newvermind 13:14:19 <adf88> I'm working currently on a house picker 13:14:25 <Alberth> indeed, something to remember for the next time 13:14:26 <adf88> in scenario editor 13:14:45 <adf88> to build custom houses at custom locations 13:15:06 <fonsinchen> No, there are tons of holes 13:15:11 <michi_cc> fonsinchen: It's probably more of a compiler problem/difference, see http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#348 13:15:49 <frosch123> fonsinchen: my debug output says it does not call the delete operator when it is NULL 13:15:51 <michi_cc> Especially the proposed (and probably also adopted) resolution for section 5.3.5: "Otherwise, it is unspecified whether the deallocation function will be called." 13:16:07 <michi_cc> unspecified == bad :) 13:16:37 <michi_cc> Just add a "if (v == NULL) return;", I can' 13:17:03 <michi_cc> t imagine any reason what would do any harm. 13:17:13 <fonsinchen> That will be done anyway. 13:17:43 <fonsinchen> I'd like to get to the core of this, though. Even if it's only my compiler, I should have hit this problem about 1000 times before ... 13:18:34 <adf88> it's everything OK with handling house placement, new CmdBuildHouse command works like a charm 13:18:34 <adf88> but I found another thing problematic - house drawing a house in the GUI 13:18:49 <adf88> I'm curius, do you think that letting to some houses 13:19:01 <adf88> to be drawn with gliches is an acceptable solution 13:19:02 <adf88> ? 13:19:23 <planetmaker> why should they glitch? 13:19:27 <adf88> I mean until they get updated to new changes (if that ever happens) 13:19:32 <fonsinchen> Maybe my compiler decides to do this only for PooledSmallStack, not for other types. The reason could be that PooledSmallStack doesn't have its own destructor. 13:20:14 <adf88> most likely because GRF protocol is not fully prepared 13:20:20 <adf88> to drawinf houses in the gui 13:20:30 <frosch123> http://paste.openttdcoop.org/show/2731/ <- fonsinchen: that's the debug output i tried 13:20:39 <adf88> i think I will be able to fix this on the OTTD internals level 13:20:40 <frosch123> my compile does not call operator delete for NULL 13:20:48 <adf88> keeping full compatibility 13:21:13 <adf88> but who knows what crazy assumptions some could make when creating GRFS 13:21:20 <frosch123> adf88: planetmaker: you will obviously run into trouble if the look of the house depends on the town zone and stuff 13:21:24 <frosch123> you have no "location" the gui 13:21:27 <frosch123> *in 13:21:37 <adf88> yes, I dealt with it 13:21:44 <frosch123> you can only return some default values for those 13:21:52 <adf88> i'm chenking which house zones are available for a certain tile 13:21:57 <adf88> and choosing first available 13:22:16 <adf88> sorry - for a certain house not tile 13:23:04 <frosch123> adf88: i mean HouseScopeResolver::GetVariable needs a tile for most variables 13:23:16 <adf88> basically the main idea is to provide "dummy" vales 13:23:19 <adf88> in these cases 13:23:37 <frosch123> yup, that's also what we do for vehicles in the purchase list 13:23:39 <adf88> it's partially done, but not for variables 13:24:09 <adf88> there is not_yet_constructed bool 13:24:32 <adf88> to tell that the house does not exist 13:24:34 <frosch123> yes, but it still needs a "tile" 13:24:36 <adf88> uey 13:24:38 <adf88> yet 13:24:53 <frosch123> not_yet_constructed only makes sure that it does not try to read random bits from the map array 13:25:04 <frosch123> but tile is still used to get nearest town/distance and stuff 13:25:05 <adf88> i'm trying to add new category 13:25:14 <adf88> similar to not_yet_constructed 13:25:29 <frosch123> i think you can also just add a new Reolver these days 13:25:39 <adf88> but this new category will not require 13:25:45 <adf88> to point a tile or a town 13:25:48 <frosch123> these "bool variables" are the old way to do stuff :p 13:26:57 <planetmaker> adf88, I think I'll commit 40 and 41 later today unless s/o objects. 40 would need some alignment of the doxygen in the last hunk, that's no issue, though. Just got a call. Will be back later. 13:27:49 *** zooks has joined #openttd.dev 13:29:12 <adf88> are you sure about 41? 13:29:15 <fonsinchen> http://paste.openttdcoop.org/show/2732 13:29:16 <adf88> you didn't like it 13:29:20 <fonsinchen> Weird compiler 13:30:17 <adf88> this is a work in progress, you can take a look if you wish to know what I mean 13:30:19 <adf88> https://www.dropbox.com/s/b4tt77xavebbbtu/gui_house_resolver_preview.diff 13:30:54 <adf88> it's to allow resolving houses in the GUI (tile-less, town-less) 13:32:32 <frosch123> adf88: this adds switch-cases to all methods of HouseScopeResolver though. instead there could be multiple derived classes from ScopeResolver 13:32:51 <frosch123> i.e. PreviewHouseScopeResolver, and ConstructionHouseScopeResolver or so 13:33:15 <fonsinchen> In fact it depends on the class having a destructor. I'll just add the check for NULL and be done with it now. 13:35:34 <adf88> hmm 13:35:35 *** DorpsGek changes topic to "OpenTTD Dev Channel || Latest SVN: r25887 || Logs: http://webster.openttdcoop.org/?channel=openttd.dev || Voice (talk-right) upon request via #openttd; make sure you are registered to NickServ before asking" 13:36:03 <adf88> 3 different classes? 13:36:22 <frosch123> could be 13:36:28 <frosch123> i am not sure how much they would share 13:37:04 <frosch123> i.e. whether one of them could be derived from another one 13:37:08 <adf88> i'll think on that 13:37:26 <adf88> but for now i'll stick on switch'es and see what happens 13:37:57 <frosch123> true :) first checking whether it works at all can pay off :) 13:40:51 <fonsinchen> I think we should do CeilDiv in vehicle.cpp:1332 13:41:16 <fonsinchen> Usually for very small amounts of cargo you rather want 1% than 0%, so that conditions based on that work correctly 13:41:57 <frosch123> you can say the same about full load :p 13:42:05 <frosch123> but well, i never used conditional orders 13:42:48 <frosch123> you could use CeilDiv for < 50% :p 13:44:48 <fonsinchen> Probably percentage is not the right measure there 13:45:51 *** DorpsGek changes topic to "OpenTTD Dev Channel || Latest SVN: r25888 || Logs: http://webster.openttdcoop.org/?channel=openttd.dev || Voice (talk-right) upon request via #openttd; make sure you are registered to NickServ before asking" 13:46:15 *** DorpsGek changes topic to "OpenTTD Dev Channel || Latest SVN: r25889 || Logs: http://webster.openttdcoop.org/?channel=openttd.dev || Voice (talk-right) upon request via #openttd; make sure you are registered to NickServ before asking" 13:47:12 *** DorpsGek changes topic to "OpenTTD Dev Channel || Latest SVN: r25890 || Logs: http://webster.openttdcoop.org/?channel=openttd.dev || Voice (talk-right) upon request via #openttd; make sure you are registered to NickServ before asking" 13:47:58 *** DorpsGek changes topic to "OpenTTD Dev Channel || Latest SVN: r25891 || Logs: http://webster.openttdcoop.org/?channel=openttd.dev || Voice (talk-right) upon request via #openttd; make sure you are registered to NickServ before asking" 13:48:24 <fonsinchen> Finally 13:50:25 <LordAro> right, now can you guys take a look at these? https://www.dropbox.com/sh/t1dy6bux74088jg/OZVTffg9nK 13:50:30 <LordAro> please? :3 13:53:29 <fonsinchen> LordAro: that's a lot. Any preference about what to look at first? 13:53:45 <LordAro> the ones with the small numbers :p 13:53:59 <LordAro> 007 and 009 got skipped because of concerns, iirc 13:54:19 * LordAro -> out 13:54:25 <LordAro> bbl 13:54:30 <LordAro> have fun discussing :) 14:07:04 <fonsinchen> There's an off-by-one error in GetVia() which prevents some destinations from getting drawn: https://github.com/ulfhermann/openttd/commit/f1196b9d8152db87d668112260b0332cba918256 14:08:43 <frosch123> hmm, pm fell for the same issue with randomRange some days ago 14:08:52 <frosch123> maybe the @param is misleading in that function 14:11:59 <fonsinchen> That may be 14:12:12 <fonsinchen> I can fix that, too, while I'm at it 14:13:03 <fonsinchen> Actually RandomRange doesn't have any documentation 14:13:31 <frosch123> even worse :) the param says "max" only 14:13:42 <frosch123> maybe "count" would be more appropiate 14:16:48 <fonsinchen> "limit" 14:17:06 <fonsinchen> We're not actually counting anything there 14:17:19 <frosch123> it's the number of cases 14:17:41 <frosch123> "num_cases" :p 14:18:47 <fonsinchen> OK 14:19:23 <fonsinchen> I like limit more. It's the limit of the range to be drawn from 14:19:36 <fonsinchen> We're not usually talking about cases in that context. 14:20:01 <frosch123> limit is also fine 14:23:28 <fonsinchen> Otherwise the patch is OK I assume? 14:24:12 <frosch123> i think so :) 14:27:36 *** DorpsGek changes topic to "OpenTTD Dev Channel || Latest SVN: r25892 || Logs: http://webster.openttdcoop.org/?channel=openttd.dev || Voice (talk-right) upon request via #openttd; make sure you are registered to NickServ before asking" 14:29:34 <fonsinchen> https://github.com/ulfhermann/openttd/commit/6a83872e99afd4aef31f672646c550e37c3066d2 14:29:43 <fonsinchen> My take on documenting RandomRange and friends 14:35:04 <Alberth> you can sprinkle some \a and/or \c in them 14:35:35 <fonsinchen> What do those do? 14:35:38 <Alberth> \a limit makes it clear you mean the argument 'limit' 14:36:32 <Alberth> \c <expression without spaces> switches to a fixed-width font 14:36:36 <Alberth> ie "code" 14:37:21 <fonsinchen> I see 14:39:28 <fonsinchen> https://github.com/ulfhermann/openttd/commit/a224caa5e24d16383e4254095e7e01927962d008 14:39:49 <fonsinchen> Some \a but no \c as I find the argument identification more appealing 14:40:36 <fonsinchen> Should I fix FS#5677 ot FS#5684 next? 14:44:06 <Alberth> patch looks fine to me 14:45:37 <Alberth> http://www.stack.nl/~dimitri/doxygen/manual/commands.html seems to be the most appropriate page. Below the large table, there is a short explanation of use for each primitive, which is probably more useful to read/browse 14:46:46 <fonsinchen> I'm actually not sure how many people read the generated doxygen as opposed to the comments in the code 14:47:13 <fonsinchen> When you read the comments those primitives are not so helpful. 14:47:25 <fonsinchen> They make the text harder to read. 14:47:39 <Alberth> yeah, they are quite subtle 14:48:08 *** DorpsGek changes topic to "OpenTTD Dev Channel || Latest SVN: r25893 || Logs: http://webster.openttdcoop.org/?channel=openttd.dev || Voice (talk-right) upon request via #openttd; make sure you are registered to NickServ before asking" 14:49:21 <Alberth> While I have a local copy of generated docs that I use whenever I need to find some item, I also agree that having the documentation right there in the source code is more important than the ability to generate html 14:50:28 <Alberth> on the other hand, the number of primitives that's used is quite small. 14:51:26 <Alberth> that table also contains the @param/@return etc etc stuff, which not used in-line. 14:51:50 <fonsinchen> Those are totally helpful, even when reading the code 14:52:10 <fonsinchen> We just shouldn't increase usage of inline primitives too much. 14:52:24 <fonsinchen> \me will get some food before deciding on what to do next 14:59:59 *** zydeco has quit IRC 15:45:59 *** Supercheese has joined #openttd.dev 15:52:39 *** adf88 has quit IRC 16:08:57 *** Ristovski has quit IRC 16:09:13 *** Ristovski has joined #openttd.dev 16:10:56 *** Ristovski has quit IRC 16:11:12 *** Ristovski has joined #openttd.dev 16:13:36 *** Ristovski has quit IRC 16:13:54 *** Ristovski has joined #openttd.dev 16:21:14 <fonsinchen> I'd like to revert r25495, close FS#5684 and reopen and subsequently close as "wontfix" FS#5553. 16:21:41 <fonsinchen> Changing the station while the train is arriving is probably a case we don't really have to support. 16:22:01 <fonsinchen> However, a train stopping twice in a row at the same station without moving is clearly a bug. 16:22:58 <fonsinchen> It's hard to fix both cases as there is no easy way to tell if the train has left the station after stopping last time. 16:23:48 <fonsinchen> So if we allow a train to stop at a different position than originally intended we can't easily prevent the "stopping twice" behaviour. 16:25:47 <fonsinchen> wait, we can check for VS_TRAIN_SLOWING ... 16:28:28 <fonsinchen> It could slow because of line end, though 16:29:20 <Rubidium> check last station visited? 16:31:20 *** zooks has quit IRC 16:32:41 <fonsinchen> That may be the same 16:32:45 <fonsinchen> legitimately 16:33:10 <fonsinchen> The vehicle may intentionally stop twice at the same station, but it should not do so without moving. 16:57:40 <fonsinchen> However, you're sort of right: if it's passed the point where it should have stopped and has the same station as last_visited we're not in the realm of FS#5553 anymore, except if it's actually stopping twice at the same station on purpose. 16:57:50 <fonsinchen> The thing is not getting less complicated, I fear. 18:56:06 *** Alberth has left #openttd.dev 18:59:34 * Rubidium wonders what the use case is for stopping twice at the same station without an order in between 19:08:35 *** Lord_Aro has joined #openttd.dev 19:10:21 *** LordAro is now known as Guest2949 19:10:21 *** Lord_Aro is now known as LordAro 19:11:40 *** ChanServ sets mode: +v LordAro 19:14:57 *** Guest2949 has quit IRC 19:18:31 <fonsinchen> Implicit orders 19:18:54 <fonsinchen> You make one order and have the vehicle go a huge tour before returning to the same station 19:19:33 <fonsinchen> As implicit orders are truncated if you reach the station after the current block of implicit orders this becomes a problem. 19:33:42 <fonsinchen> Also constructs like unload->implicitly go to depop->load don't work anymore like that 19:33:52 <fonsinchen> *depot 19:45:28 *** DorpsGek changes topic to "OpenTTD Dev Channel || Latest SVN: r25894 || 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:48:30 <planetmaker> https://dl.dropboxusercontent.com/sh/j0whg8ho4uwm20s/AjGxRq28Qw/41-deselect-goto-tool-while-selecting-same-type-again?token_hash=AAGkSZ3RufS0TXNFcXc91iv0qbkKCLz3uYhqzdBeIl5L2g&dl=1 <-- do we want selecting the same 'goto' order type again having no effect (i.e. keeping the giving order mode active) or giving a 2nd selection of the same tool the meaning to cancel giving orders? 19:50:38 <frosch123> doesn't that only apply to OPOS_GOTO anyway? 19:50:53 <frosch123> i think canceling is fine 19:51:32 <frosch123> but do you tell me, that if i click share orders and select another vehicle, that share order stays active? 19:51:51 <frosch123> what would be the point of that? 19:52:33 <planetmaker> it applies to goto and to shared orders 19:52:57 <frosch123> well, but does "shared orderes" stays lowered after sharing? 19:53:21 <planetmaker> but not staying active after you used it. But after you re-considered your choice of what order type to give 19:53:36 <frosch123> ok, then canceling is the right thing 19:53:50 <planetmaker> shared orders staying active after being used makes little sense to me 19:54:04 <frosch123> exactly :) that's why i was asking 19:54:06 <planetmaker> goto staying active after being used depends on our adv. setting 19:54:29 <planetmaker> this is only about whether the order-giving-mode stays active when re-selecting the same action or not 19:54:47 <planetmaker> I actually prefer to NOT apply the patch I linked :D 19:54:57 <frosch123> he :p 19:55:01 <frosch123> i think it makes sense 19:55:35 <frosch123> but might be a rare case anyway 19:55:38 <planetmaker> the goto-tools become inactive when I clicke the button. But when I specifically select one of the items in the drop-down I don't want it to become inactive. Even when I select the same as I did before 19:55:59 <planetmaker> (and that's what the patch is about only. Yeah, very rare case :-) ) 19:56:18 <frosch123> hmm, yeah, one would probably just click the button to deactivate 19:56:23 <frosch123> instead of using the dropdown 19:56:39 <planetmaker> adf88 likes it. I don't so much, but... I won't bitch, if most prefer and would commit it then :-) 19:56:44 <planetmaker> I could ask forums :D 19:57:10 <frosch123> would need screenshots, else noone will likely get it from just textual description 19:57:27 <planetmaker> yes. much work for... very little :-) 19:58:04 <planetmaker> I guess I'll just skip it then :-) 20:04:11 <planetmaker> LordAro, do more patches from that queue exist? I've the feeling some only really make sense in some larger context. They're a code change one can do, yes. But... need to? 20:06:41 *** tycoondemon has quit IRC 20:06:57 *** tycoondemon has joined #openttd.dev 20:11:49 <frosch123> planetmaker: ofc there are more patches 20:12:08 <planetmaker> I know. But not exactly where 20:12:20 <frosch123> well, they won't help you to judge a single diff 20:12:44 <frosch123> you can only really use those which make sense on their own 20:12:47 <frosch123> and skip the rest 20:13:02 <planetmaker> :-) I planned to do just that 20:13:03 <frosch123> i think code deuplications and comments make most sense to commit 20:13:21 <frosch123> but "optimisations" may not 20:13:28 <planetmaker> there are lots sof ^ 20:13:30 <planetmaker> *of 20:13:36 <planetmaker> which I'm not sure about 20:14:26 <frosch123> originally i thought lordaro wanted to filter them out 20:16:55 <planetmaker> I think the idea originally was to go through all patches. But yes, filtering out the non-obvious optimizations for now might be good. Or putting them separate 20:24:15 <LordAro> i could go through all of them, but since the whole queue is ~ 550 patches, i'd rather not if there was no chance of a merge ;) 20:24:32 <LordAro> i'll add the full patch (single file with patches contained) to the folder 20:25:44 <LordAro> wait, it's already there, under raw.patch 20:25:51 <planetmaker> well... would be nice, if you could do some kind of pre-filtering for the possibly useful stuff :-) 20:25:56 <planetmaker> raw.patch is one big patch, no? 20:26:33 <LordAro> raw-241.patch is the first 241 patches in the queue, i determined that after that the difference between codechanges and new features gets 'muddy' 20:26:41 <LordAro> raw.patch has everything in it 20:26:42 <planetmaker> I mean... https://www.dropbox.com/sh/t1dy6bux74088jg/TBlcez-3Mc/cirdanqueue/029-reduce-indentation-drawsignals.diff - what's the point? :-) 20:27:00 <LordAro> looks nicer? 20:27:10 <LordAro> nothing wrong with refactoring to make it look better :) 20:31:17 <planetmaker> and things like https://www.dropbox.com/sh/t1dy6bux74088jg/ggcgh0YG1O/cirdanqueue/017-functionise-dist-to-tile-edge.diff make somewhat sense. If the function is needed another time 20:32:07 <frosch123> yeah, refinery should have the same, but it's not in that patch 20:32:37 <frosch123> hmm, ... i guess refinery is not direction dependent though 20:32:55 <frosch123> yeah, quite unlikely then to be needed a second time 20:34:23 <planetmaker> https://www.dropbox.com/sh/t1dy6bux74088jg/SLJc16Sh3R/cirdanqueue/013-dont-use-error-var.diff yes, possibly 20:34:39 <planetmaker> though I never encountered a global 'error' variable so far :-) 20:34:50 <LordAro> planetmaker: it gets used again in 019 and umm, 289, 347 and some other later patches 20:34:59 <frosch123> it would make sense if there was an actual assert in that diff 20:35:24 <planetmaker> which? 013 or 017? 20:36:14 <planetmaker> https://www.dropbox.com/sh/t1dy6bux74088jg/Jwj4-jf-JR/cirdanqueue/028-new-checkwaitingposition.diff might make sense. But I'm scared to touch pathfinders 20:36:42 <LordAro> lots of detailed text about what it's doing 20:37:20 <planetmaker> yes, the comment actually is nicely elaborate. I quite appreciate that 20:38:45 <planetmaker> but still it means I first have to understand the PF code to actually judge it :-) 20:47:55 <planetmaker> omg, you also only have this one huge file which is a list of concatenated patches, LordAro ? :-( 20:49:00 <LordAro> that was how cirdan presented his patch queue, yes :/ 20:51:35 <LordAro> i probably could split them with a script though :L 20:51:45 <LordAro> not actually review them though 20:53:42 <planetmaker> well. would be nice, if you could sort them by some categories :-) But yes 20:54:11 <planetmaker> so basically cirdan bitches about 'no-one reviews' but makes it a hell to actually look at his stuff? nice 20:54:30 <LordAro> he did originally post a 'proper' patch queue, iirc 20:54:39 <LordAro> but he removed it when he updated the patch 20:55:05 <LordAro> and to up fair, the 'update airports' patch has been sitting in flyspray for years without a proper review 20:55:14 <LordAro> s/to up/to be/ 21:01:30 <planetmaker> one can always find a reason ;-) 21:02:14 <LordAro> well, if one of my patches had gone unreviewed in ~5 years, i'd be a bit annoyed too :p 21:05:15 <frosch123> it's fine to blame that on us, and to fork. but it's no reason to not give a queue to others, which he has anyway 21:05:33 <frosch123> you do not need to respect the devs, but you should respect the project 21:05:41 <frosch123> that's the point of open source 21:05:51 <frosch123> it's about the software, not about the people doing it 21:06:16 <LordAro> :3 that's very deep :p 21:12:41 *** frosch123 has quit IRC 21:20:44 <LordAro> planetmaker: so, what's your assessment of the things? :L 21:37:07 *** ntoskrnl has quit IRC 21:46:17 *** helpmeplease has joined #openttd.dev 21:48:10 *** helpmeplease has quit IRC 21:48:51 <planetmaker> well, as said, some interesting stuff. Others I'm unsure of. And yet again others I'd not do. Some saveload refactoring might be interesting as seen later 21:49:13 <planetmaker> though I'd also like to know more on the purpose of some of the patches... alas. 21:49:16 <planetmaker> bed time now :-) 21:49:29 <LordAro> g'night 21:49:42 <LordAro> and you could always ask cirdan, he is mostly active ;) 22:05:53 *** Ristovski has quit IRC