Diagonal maintenance costs

[ras52]

Simutrans Experimental 7.2 introduced a change so that way maintenance costs for diagonal ways were based on Euclidean rather than Manhattan distance (i.e. a diagonal road cost less than one going round two sides of a rectangle to get there).  This is definitely a good thing in my opinion, however there are a few bugs with current implementation that means that a road layout can have differing maintenance costs depending on the order in which the tiles were created.  When the tiles are bulldozed away, the player's maintenance costs always reduce by the full tile amount, rather than at cheaper diagonal rate.  This means that if you create a road system with lots of diagonals and later delete it, your maintenance costs reduce to less than where started and possibly go negative.  Aside from the possibility of cheating this raises, it makes it very difficult to see exactly where your maintenance costs are coming from which can make it hard to analyse or balance the game.

An unrelated bug is that when creating and deleting a complex road layout, it is possible to end up with the diagonal road graphic used in places where the corner graphic should be used, and would have been used had you created the roads in a different sequence.

The attached patch fixes both of these bugs.  It also changes the diagonal conversion factor from 1.4 to 1.414, not because the greater accuracy is important but to make it more obvious where the number comes from.  (It's no more efficient to work with 1.4.)

[AP]

Out of interest, what happens if one draws the diagonals manually - first lots of north-sough sections of two tiles of track, then joining them up east-west - so as to make sure the diagonal is laid precisely where it's wanted. Laying a diagonal track as one piece often results in it not quite ending up in the best place, so I've often done it the 'slow way'...

[ras52]

Out of interest, what happens if one draws the diagonals manually - first lots of north-sough sections of two tiles of track, then joining them up east-west - so as to make sure the diagonal is laid precisely where it's wanted. Laying a diagonal track as one piece often results in it not quite ending up in the best place, so I've often done it the 'slow way'...

If you draw the diagonals manually as you describe in the unpatched code, the maintenance is the full cost -- i.e. without the 1.414 diagonal reduction.  My code fixes this for roads but not railways.  That's partly because creating roads uses separate code from other line types, partly because only roads had the bug that meant that creating and deleting a diagonal decreased your total maintenance costs, and partly because I'm trying to do get the balancing of roads right and this was stopping me from doing it. However since you mention it, the attached patch here extends this to apply to railways, narrow gauge railways, trams, monorail and maglevs too.

(Incidentally, have you tried holding down control when creating the diagonals?  Once I found out about that, I stopped needing to create them by hand.)

[jamespetts]

Richard,

thank you very much indeed for this - it is very much appreciated. I am preoccupied with the server desync bug at present, but will get around to testing and hopefully implementing your patches soon, I hope!

Incidentally, the graphics bug may well be present in Standard, so you might want to consider reporting it there, too: I have hardly changed the graphics code in Experimental.

[ras52]

Incidentally, the graphics bug may well be present in Standard, so you might want to consider reporting it there, too: I have hardly changed the graphics code in Experimental.

The graphics fix and the diagonal maintenance fix are heavily tied together as both require extra passes over the route during creation and looking up to two tiles away from the bulldozed tile during destruction, and these extra passes can be shared.  So I'm not sure its worth unpicking the fix for graphics bug (which is really a very trivial bug) from the rest of the patch.  But if anyone does want to, simply delete the calls to weg_t::subtract_maintenance(), replace the calls to weg_t::add_maintenance() with weg_t::check_diagonal(), and leave the calls to grund_t::calc_bild() alone.

[jamespetts]

Interesting! Perhaps the Standard developers might produce a different fix, but it should probably be reported in Standard, at least.

[ras52]

I've posted it to the Standard development area.

Edit: Dwachs has patched Standard in a rather different way.  His patch makes much more sense for Standard than mine and is no worse than mine for Experimental (other than being a bit more verbose).  It makes sense to apply his patch to Experimental too on the basis that we don't want to introduce gratuitous differences between the two.  The attached patch incorporates Dwachs' code from r5246 on Std, my original road diagonals maintenance cost patch (minus the graphics fix), and my subsequent railways patch.

[jamespetts]

Thank you very much indeed! I was wondering how the Standard and Experimental fixes would be reconciled. As noted elsewhere, I can't do anything about this until next week, but I do very much appreciate your work on it.

One small thing in relation to patches - it would help me if, instead of using .patch files, you could set up Github branches for each different set of fixes/improvements that you produce, as that makes it easier for me to merge the code and switch between different branches. Don't worry if you don't have the time to do this, but, if you do, it would be extremely helpful. Thank you again!

[ras52]

One small thing in relation to patches - it would help me if, instead of using .patch files, you could set up Github branches for each different set of fixes/improvements that you produce, as that makes it easier for me to merge the code and switch between different branches.

Hopefully this should be a suitable github branch.  I've never used github before, so do please tell me if I've done anything in a silly way.  (In particular, what is the "proper" way of making the merge of r5246 on std?  I've simply used svn to get a patch out, and then used git-apply to apply it.)

[jamespetts]

Ahh, thank you - that looks helpful! I see that you have based it on the -devel branch, rather than the 10.x branch, which means that it will be incorporated, not in the next bugfix release, but the next major release  - is that intended?

As to merging the latest code from Standard, I have been doing that on the 111.1-merge branch, then merging that, in turn, back into -devel. I generally merge all changes (and correct any merge conflicts manually) periodically. You are more than welcome to do the merge up to the latest Standard revision into your copy of the 111.1-merge branch, then merge that onwards into -devel if you are so inclined.

Thank you very much for your work on this!

[ras52]

Ahh, thank you - that looks helpful! I see that you have based it on the -devel branch, rather than the 10.x branch, which means that it will be incorporated, not in the next bugfix release, but the next major release  - is that intended?

I hadn't given it much thought as I'd inferred from this topic that the next major release was not too long away.  What, in general, is your policy for new code in bugfix releases?

[jamespetts]

It is difficult to predict how far away that the next major release is, as that will depend entirely on how many bugs are found during testing; but it will probably be a few weeks, at least, since I have to implement brake forces and rolling resistance parameters in a pakset in order to be able to test it properly.

As to the policy for bugfix releases: new code that fixes a bug or makes a small but important gameplay change that can be compassed in one or two lines of code (as with the prevention of bridges with no length limits over deep seas) goes into bugfix releases, whereas significant new features or fixes/changes that depend on upstream features go into major versions.