Config
Log for #openttd.dev on 20th October 2013:
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

Powered by YARRSTE version: svn-trunk