Config
Log for #openttd.dev on 25th October 2012:
Times are UTC Toggle Colours
01:13:38  *** Knogle has quit IRC
01:14:26  *** Knogle has joined #openttd.dev
04:22:49  *** Supercheese has joined #openttd.dev
06:10:39  *** Zuu has joined #openttd.dev
06:10:39  *** ChanServ sets mode: +v Zuu
06:50:52  *** Supercheese has quit IRC
07:22:08  *** Zuu has quit IRC
13:10:43  *** Knogle has quit IRC
13:11:24  *** Knogle has joined #openttd.dev
13:25:57  <Belugas> hello
13:26:11  <planetmaker> hello Belugas
13:26:38  <Belugas> sir maker :)
13:42:06  <Terkhen> hi Belugas
15:27:15  *** frosch123 has joined #openttd.dev
15:27:15  *** ChanServ sets mode: +v frosch123
16:27:42  *** Alberth has joined #openttd.dev
16:27:42  *** ChanServ sets mode: +v Alberth
17:29:07  <Terkhen> https://secure.openttd.org/bugs/task/5342 <-- warning_road_cmd.patch is the same solution I implemented a few days ago (sadly, that biohazard guy never tested it for me) so I'm going to commit it... is the other patch correct WRT the unsigned/signed issue too?
17:29:41  <Terkhen> not adding parentheses to surround this->group_sb->GetPosition() + this->group_sb->GetCapacity() looks somewhat wrong to me
17:31:12  <frosch123> both diffs look fine to me
17:31:17  <frosch123> i don't think it needs parens
17:32:10  <Terkhen> thanks, I'm usually overzealous with them :P
17:32:15  <Terkhen> I'll commit both then
17:38:12  *** DorpsGek changes topic to "OpenTTD Dev Channel || Latest SVN: r24626 || 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:15  *** DorpsGek changes topic to "OpenTTD Dev Channel || Latest SVN: r24627 || 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:46:54  *** Zuu has joined #openttd.dev
17:46:54  *** ChanServ sets mode: +v Zuu
18:30:06  <Zuu> I'm currently looking at reproducing FS#5344 (RemoveRoadFull failing)
18:34:51  <Zuu> It appears to be a visible error in the code, but currently GSSign.GetLocation give me problems :-p
18:37:07  <Alberth> frosch123: FR#5316 is done, isn't it?
18:38:19  <frosch123> yeah :)
18:47:10  <Zuu> Hmm, RemoveRoadFull works for all intended cases.
18:47:21  <Zuu> Ah.. the reporter tries to pass the same 'from' and 'to' tile to the API
18:47:59  <frosch123> there might also be differences between 1.2 and trunk
18:48:27  <Zuu> In the docs it is said that 'from' and 'to' must be in a straight line. Eg. that they have the same X or Y coordinate. But it is not strictly said to be a precondition that the tiles are not the same.
18:48:51  <Zuu> frosch123: Also true. I have so far only tested on trunk to see if the error is in trunk.
18:49:07  <frosch123> yeah, the road api was meant to only build connection between tiles
18:49:17  <frosch123> not to build individual bits on a single tile
18:49:30  <Terkhen> there were changes to build road in trunk since 1.2.x, but I don't remember any remove road changes
18:49:34  <frosch123> no idea how removal should work though
18:50:17  <Zuu> I think that method should stay as is, but could get a pre condition that 'from' != 'to'. A new API could be added to remove individual road bits if that is needed.
18:50:48  <Zuu> Terkhen: None of those should have affected AIs anyway?
18:52:15  <Alberth> Zuu: why not extend the current api work for the case 'from' != 'to' (instead of adding another api) ?
18:52:29  <Zuu> IIRC, API methods can have named optional parameters, so it is probably better to not a direction parameter to the current API
18:52:33  <frosch123> i guess build from x to x should build nothing
18:52:40  <frosch123> remove from x to x should clear the tile
18:53:18  <frosch123> Zuu: missing a negation?
18:53:25  <Alberth> build is exclusive the mentioned tiles?
18:53:39  <frosch123> Alberth: it builds the connection
18:53:48  <frosch123> so, only a half tile on the start and end tile
18:53:53  <Alberth> ah, it's 'between'
18:54:01  <Yexo> <frosch123> remove from x to x should clear the tile <- in which direction?
18:54:10  <Yexo> remove all bits in y direction, all bits in x direction or all roadbits?
18:54:10  <frosch123> complete
18:54:16  <Yexo> we have cleartile for that
18:54:17  <frosch123> like ClearTile
18:54:27  <Zuu> frosch123: Yea, APIs can't have named parameters if I remember correctly.
18:54:32  <Yexo> I'm not in favor of that change, imo using it like that is a programming error
18:54:37  <frosch123> ok, we can also just say "use cleartile" instead :)
18:54:43  <Yexo> Zuu: squirrel supports it but our c++ <> squirrel layer doesn't
18:54:46  <Terkhen> @commit 24503
18:54:46  <DorpsGek> Terkhen: Commit by terkhen :: r24503 trunk/src/road_cmd.cpp (2012-09-01 14:58:36 UTC)
18:54:47  <DorpsGek> Terkhen: -Change [FS#5228]: When building long roads or tramways, only build the roadbits at the beginning and the end if they can connect to something.
18:55:00  <Zuu> Yexo: Yeah that was what I was remembering too.
18:55:39  <Terkhen> Zuu: ^ in theory AI behavior was unchanged
18:55:43  <Yexo> I think AIs should almost always use BuildRoad (not BuildRoadFull), same for removal
18:56:03  <Zuu> Indeed.
18:56:51  <Yexo> so change RemoveRoadFull() by adding a precondition start!=end?
18:56:58  <Zuu> That is my idea
18:57:24  <Yexo> good :)
18:57:33  <Zuu> And if it's really needed, a RemoveRoadBit API can be added.
18:58:17  <Yexo> that's require a lot more API functions, like BuildRoadBit, QueryRoadBits etc.
18:58:51  <Yexo> which would all work on a very low level which is not really required for AIs
18:59:18  <Zuu> The bug reporter was a GS author, but I doubt GS need that detail too.
19:00:42  <Zuu> The only reason I can think if you want to clean up roads in some situations.
19:01:42  <Yexo> You can use RemoveRoad() for that
19:02:01  <Yexo> give two neighbouring tiles, if only one roadbit is on the map it'll only remove that one
19:02:19  <Yexo> I don't see why you'd ever want to remove one bit without removing the neighbouring roadbit too
19:02:28  <Alberth> hmm, can you not remove road (x, y) -- (x+1, y) to remove a half tile at (x, y) ?
19:03:17  <Yexo> Alberth: yes, it'll also remove half a tile at (x+1, y) though
19:03:17  <Alberth> oh, Yexo just had the same idea :)
19:03:41  <Zuu> The only reason I see that it might fail is that AI build actions are ment to fail if some part fail instead of doing something half-way as the human interface does.
19:04:11  <Zuu> But it might be that removal differ from build on this point.
19:04:47  <Yexo> if not then the removeal function should be changed to remove as much as possible, since it makes sense
19:06:54  <Terkhen> IIRC the original fix that introduced a specific AI behavior for build long road did the same for removal
19:07:10  <Zuu> I think it already works to use the removal tool with two tiles to remove bits on only one tile. But I only say that it contradicts that design of the API.
19:07:35  <Zuu> s/removal tool/removal API/
19:08:21  <Terkhen> bbl
19:08:31  <Zuu> http://devs.openttd.org/~zuu/remove-road-full.patch
19:08:44  <Zuu> Is it OK to have different order of the pre condition checks in cpp and hpp?
19:08:59  <Zuu> I think the order in hpp makes more sense there, but in cpp this order should be quicker.
19:09:13  <frosch123> the result is 'false' in both cases
19:09:19  <frosch123> what about the build functions?
19:09:25  <frosch123> should they also get this precondition?
19:09:32  <frosch123> or do they have it already? :)
19:10:11  <Zuu> They don't have it already as explicit as it gets now.
19:10:19  <Zuu> Maybe add it explicit there too.
19:13:12  <Zuu> Hmm, my bad, Build already have it.
19:13:37  <Zuu> I'll update to use the same wording as there.
19:18:07  <Zuu> Btw, I wonder why RemoveRoad use (start < end ? 1 : 2) and RemoveRoadFull use (start < end ? 2 : 1) in their DoCommands. BuildRoad doesn't show this difference.
19:18:09  <Zuu> http://paste.openttdcoop.org/show/1820/
19:18:13  <Zuu> (relevant code)
19:19:44  <Yexo> except for that both look the same to me
19:20:18  <Zuu> Hmm true.
19:21:05  <Zuu> So perhaps the 'full' bit is included with some other bits in some interesting way. :-)
19:22:38  <Zuu> Should we note this enforced pre condition in the change logs?
19:23:07  <frosch123> i think it's a bugfix
19:23:16  <frosch123> no need to document it, it never worked
19:23:16  * Zuu agrees
19:25:31  *** DorpsGek changes topic to "OpenTTD Dev Channel || Latest SVN: r24628 || Logs: http://webster.openttdcoop.org/?channel=openttd.dev || Voice (talk-right) upon request via #openttd; make sure you are registered to NickServ before asking"
20:05:05  *** DorpsGek changes topic to "OpenTTD Dev Channel || Latest SVN: r24629 || Logs: http://webster.openttdcoop.org/?channel=openttd.dev || Voice (talk-right) upon request via #openttd; make sure you are registered to NickServ before asking"
20:35:37  *** Webster has joined #openttd.dev
20:35:38  *** ChanServ sets mode: +v Webster
20:36:46  *** Guest3156 has quit IRC
20:37:54  *** Alberth has left #openttd.dev
21:14:38  *** frosch123 has quit IRC
21:44:41  *** FLHerne has joined #openttd.dev
23:36:54  *** Zuu has quit IRC
23:50:49  *** Supercheese has joined #openttd.dev
23:59:24  *** FLHerne has quit IRC

Powered by YARRSTE version: svn-trunk