Long-term request for help: fixing meters_per_tile

[neroden]

Currently meters_per_tile behaves in a way which... doesn't work right.

Rather than actually adjusting distances, it screws around with time.

I would like to rewrite the code, eventually, so that it actually adjusts distances.  This means that when a train goes at 10 km/h, if you divide meters_per_tile by 2, it will cross twice as many tiles in the same amount of time.

This will clean up a LOT of experimental and make a VAST quantity of stuff work better.

But there's a lot of work to do to do this, and if anyone knows where to start it would help.

[Carl]

I was playing around with a 20 metres per tile map a while ago. On your proposal, wouldn't vehicles on such a map zip around the screen at untrackable speeds?

Which is to say: I think the "screwing with time" option is designed to make things look visually similar no matter what meters per tile value you select.

[Erik]

I was also playing with the meters_per_tile. I had set it on 100 meters_per_tile. (And station_coverage set on 4.)
I was wondering if it would break the graphics by untraceable speeds.
I didn't notice much difference and now I know why.

I like to see what happens when this is corrected.

[jamespetts]

Carl is correct - it is intentional that the meters per tile setting adjusts the time rather than the space, so that the actual speed at which vehicles move is fixed at a constant, pleasant speed. I don't think that this needs "fixing".

[Erik]

Couldn't it corrected by making the time pass slower instead day(/month) longer?

[jamespetts]

What do you mean?

[Erik]

Far I understand the meter_per_tile meshes with the timescale, while it (like the name said) has to scale the length of the tiles.

About the issue that vehicles will go too fast to catch up on the screen.
This can been adjusted by running the game slower.

I want to see that the tiles can represent less meters.
With the game play I didn't like for instance  that for a car it takes pretty long time to pass a railway.
This ended with that a train has to stop because the car wasn't passed soon enough or there wasn't time for the car to starting to pass at all before the next train.
By defining the tiles less long this could be solved.

[jamespetts]

I am still having trouble understanding you, I am afraid. What do you mean by referring to cars passing a railway here?

In the current system, the actual movement speed of vehicles on the screen in real time is constant, but the relationship between real time and game time varies. This has been implemented with great care, and the relationships have been set up precisely to ensure that physics, timings, reported distances, revenues, costs, and the generation of passengers and freight are constant in their relationship with in-game time and distance. The only effect of altering the meters per tile would be to make the speed at which vehicles appear to move around in the game window variable rather than fixed, which I do not think would be a happy side-effect.

[Carl]

I think he means that, despite the adjustment, vehicles do still appear to move slower at lower metres_per_tile values. I've never found this to be a problem, but it's certainly noticeable at very low values.

[jamespetts]

I am not aware of any reports of this before: this would be very odd indeed, as the meters per tile value does not affect the actual relationship between speeds and movement on the screen. It is factored into the physics code for the purposes of acceleration and braking, however; is that what is meant? That, of course, is necessary.

[Carl]

Yes, I assume it's the acceleration and braking that give the appearance of slower movement.

[neroden]

This needs fixing.

Why does it need fixing?   Because the current system does not work right, and is extremely fragile.Changing the time is not symmetrical with changing the distance.  Someone who has taken physics should understand that.  Attempting to change the time rather than changing the distance affects everything.  It affects acceleration in a complex and nonlinear manner, and it affects the derivative and integrals of acceleration more, which means it screws up power and tractive effort and breaks the new physics code. It affects the computation of journey times (which have to be corrected to "unbreak" the mess made by adjusting time), which affects the effect of waiting times, which affects the factory production rates, and on and on and on and on and on.

This makes for ugly, unmaintainable code which is always broken somewhere.  As long as this is the case, paksets will be restricted to a specific meters_per_tile setting, will not balance properly at any other setting -- and this code will never be merged into standard.

Now, you want the train travel to "look pretty"?  Well, you can adjust both the distance and the time, but you should adjust them separately. 

Remember, the player can set the game at any speed he likes, anyway, so he can make the trains "look pretty" no matter how fast they're going in internal game terms.

So if everyone is completely obsessed with the visual speed, how about we add an option to the pakset called, oh, I don't know, "default_milliseconds_per_tick", to "cancel out" the visual effect of the meters_per_tile behavior change. This would be clean and maintainable whereas the current situation is not. Think about this:
- the vehicles would go a constant number of kilometers per tick
- this would be a variable number of tiles per tick, depending on meters_per_tile
- with a variable number of ticks per millisecond (at "speed 1.00") this could be, visually, the same number of tiles per millisecond regardless of what you set meters_per_tile to, just by changing ticks_per_millisecond.  The only difference is that the game would actually model the world correctly (rather than wrongly), and it would be possible to maintain the code without running into more and more subtle bugs.

---
I could implement default_milliseconds_per_tick in a day and it would solve your "visual issues".  Then can I clean up the code so that it works right?!?

---
Sorry to get so irritated.  It annoys me when people say "oh, we must break the world modelling and break the game logic in order to make things look pretty".  Especially when you don't actually have to.

Mod note: Giant letters removed. Please avoid exaggerated use of text formatting, that sounded like you were shouting. This is just a remind; sorries accepted. :>
~IgorEliezer

[neroden]

In the current system, the actual movement speed of vehicles on the screen in real time is constant, but the relationship between real time and game time varies. This has been implemented with great care,

It breaks routinely.  Please go back and count the number of times when you've had to patch yet another location in the code.
Perhaps great care, but I cannot in all honesty say that this has been implemented with sufficient care to make it work right.  It has been implemented with a great deal of effort, but that is not the same thing -- that is the result of a defective design.  It is not worth spending the effort on continually stamping out the infinite bugs which crop up due to this.

If we do it the right way we will have a far smaller patchset against standard, and it will be maintainable.  The current system is not maintainable, and it has never worked right.  Changing meters_per_tile routinely breaks pakset balance because of this.

and the relationships have been set up precisely to ensure that physics, timings, reported distances, revenues, costs, and the generation of passengers and freight are constant in their relationship with in-game time and distance.

No, they're not.  I can't count the number of bugs in this system.  And they're all due to doing this the wrong way.  If you do it the right way, you don't have to edit 500 places, you only need to edit a few places.

The only effect of altering the meters per tile would be to make the speed at which vehicles appear to move around in the game window variable rather than fixed, which I do not think would be a happy side-effect.

On the contrary, this would fix an enormous number of bugs simultaneously, including many you haven't found yet.

Here's the thing: I can already make the speed the vehicles appear to move into whatever I like by hitting the "faster" and "slower" keys.  Personally I like to run the game at 2.00, usually.  1.00 is too slow. 
If you want the default speed to "appear" the same as the old default, change a "game_speed_factor" setting in the pakset at the same time as you change meters_per_tile.  Do it right, don't do it wrong.

[neroden]

(Last reply got stuck on bold for some reason....)

Looks like step one, I should just implement that game_speed_factor option for paksets, to eliminate these silly objections.  I'll get back to you after I've done that.

(Then comes the hard part: figuring out how to implement meters_per_tile correctly while the incorrect implementation is still floating around.  I may have to have a separate real_meters_per_tile option for a while.)

---
Thanks to everyone for making it clear what feature I need to implement before fixing meters_per_tile.

[jamespetts]

It's always always good when people get passionate about Simutrans - it's such a lovely hobby - but a change as significant as is suggested here needs perhaps some more considered reflection.

Perhaps we could start with a list of the known bugs that are due to this feature and would not occur if things were done differently? I am not aware of any such bugs at present (Bernd integrated the meters per tile setting into his physics code some time ago, which works fine so far as my tests show). All of the things to which you refer in your first post are indeed adjusted for meters per tile.

Can you elaborate a little on how the default milliseconds per tick thing would work, especially in a network game? How would this be different to the faster and slower keys (which are intentionally disabled in network mode)? Would this potentially affect the computational intensity of the game run at a higher meters per tile setting to take it beyond what can work effectively on a large map in a network game?

The meters per tile setting is as it is in large part because when I first introduced the concept of explicit journey times for the original implementation of the journey time based routing system, there was no standardised idea of how long that a tile was: it was said that it loosely correlated to 1km, but that seemed too large for many purposes, so I took a figure of 250m/tile, but made it adjustable. Since the code that I was adding was the journey time code, and there was nothing particularly lacking about the vehicle movement code, it was the journey time calculation that I made dependent on the meters per tile setting, rather than try to mess with the vehicle movement code. The principal aim was just to be able to customise how the length of a tile is interpreted, as this started from a rather arbitrary base. With that in mind, it was never intended that this setting be changeable on the fly or that it be possible to change it without having to recalibrate some settings of a pakset: the purpose was simply to allow flexibility in how the assumptions made in the calculations of the journey times. As more features have come to interlink more precisely with the journey times, the setting has gained increasing importance, and all of the things that you mention have come to be calibrated in accordance with it.

[neroden]

It's always always good when people get passionate about Simutrans - it's such a lovely hobby

Thanks.

Perhaps we could start with a list of the known bugs that are due to this feature and would not occur if things were done differently?

It's very hard to track down subtle bugs in journey speed calculations, waiting time calculations, revenue calculations, maintenance calculations, and so on.  They don't show up as "one big bug", they just show up as little tiny things being a little bit off.  Please note the sheer number of different times you've had to patch something for this.

I am not aware of any such bugs at present (Bernd integrated the meters per tile setting into his physics code some time ago, which works fine so far as my tests show). All of the things to which you refer in your first post are indeed adjusted for meters per tile.

You *think*.  You also thought you'd implemented freight coverage distance.

Can you elaborate a little on how the default milliseconds per tick thing would work, especially in a network game?

Well, OK.  So suppose that our default was 1000 meters / tile and one real millisecond per tick.
Now, for any given pakset/game, we have a specified ratio of game hours/minutes to ticks (I hope!) and a set ratio of game months to ticks (although the number of game hours per game month depends on pak settings, and there are some other issues to clean up there).

Now suppose you shift to 500 meters / tile.  The vehicles -- because they retain the same kilometers/game hour speed -- will all move *twice as many tiles each tick*.  This will mean fewer computations per tick, but computations with larger numbers.  (If you shift to something like 20 meters/tile, we may have to fix some overflow errors.  I believe these overflow errors are showing up already at high game speeds, which cause the same effect: fewer computations but using larger numbers.)

Now, suppose further that you shift to two real milliseconds per tick.  The vehicles will now, visually, move the speed which they did before, because it takes twice as much time to do one tick.  Furthermore, the computation work will be spread out over... twice as much real time.

The most noticeable systematic effect will be that the game months will change less often in real time, meaning that the whole game will run for longer if you're trying to run from 1900-2000 or whatever. 

For a long network game, you might then want to have fewer game hours per game month.  (Having fewer game hours per game month would have some gameplay effects in *timeline games only* by causing vehicles to be introduced and become obsolete more quickly, relative to numbers of hours run.)

How would this be different to the faster and slower keys (which are intentionally disabled in network mode)?

Not very different.  Digging through the code, it would really just be a change in how many ticks (is "simloops" the right term? or is "steps" the right term?) the main server attempts to get through on each real millisecond.  My idea is that it would redefine what "speed 1.00" means, while not changing anything else.  In a network game, it would be set immediately at game startup and would do its thing thereafter; it would have to be passed to the clients, of course.

Would this potentially affect the computational intensity of the game run at a higher meters per tile setting to take it beyond what can work effectively on a large map in a network game?

It should work the other way around.  There should be less computational intensity for most of it. 

In general, if we're going with smaller values for meters_per_tile and trying to make the visuals look the same, it would force the clients to do fewer steps per millisecond (less computational intensity).

The potential problems, indeed, will be quite the opposite: the computational inaccuracy problems generated by the approximations used in high fast-forwarding, which could in a worst-case situation lead to "jumpy" trains.  And also the problems of potentially doing too many "hops" in a single "step", which could have complex repercussions.

I don't actually think these problems will show up, because 125 meters/tile is only equivalent to "8x", and I've never had overflow problems or "double hop" problems even at 16x.   At 20 meters/tile, equivalent to "50x", you might start seeing trouble.

----

But hey -- let's go ahead and worry about this.  If problems DO show up, the next step would be to alter the number of ticks/steps per in-game hour!   Now, this is OK if it's what you're trying to *do*, mind you.  (It's not what you're trying to do with meters_per_tile.)  If done correctly, this should merely be a matter of "precision in computation", and should be something you actually could change without altering any paksets at all.  Although it would undoubtedly cause a network desync if you used different values on different clients, so this would also have to be preset in advance.  Implementing this is also on my long term TODO list.

Now, if you're planning to use much *larger* meters_per_tile -- say 100,000 -- then if you want the visual speed to be maintained, you would end up with computational intensity and problems (but you would end up with arithmetic underflow problems first).  This could again be altered by adjusting steps_per_hour *if necessary*.

Now, I know I'm suggesting something which sounds like it has a lot of settings to adjust.  But this is because if it's done right, the code ends up modular; most of it ends up encapsulated cleanly in one or two files (perhaps simunits.h or maybe simworld.h / simworld.cc). 

Have you heard of the software writing principle known as "once and only once", the rule that any computation which is even slightly complex should only have one copy in the code, with all others calling the same subroutine -- which should have a natural and obvious name?  (You will note that this rule is what I used when fixing the halt code in simfab.h, as soon as I noticed that the list of halts was computed in two places.)  The confusion between distance and time makes it impossible to implement "once and only once" here.  And indeed it results in multiple copies of the game logic all over the code; it's very hard to make sure that you've gotten ALL of the places where the logic needs to be, which is why bugs keep cropping up.

A second software writing principle is the "principle of least surprise", under which the code should be comprehensible to the average reader -- this means that future people maintaning your code are less likely to break it -- and the distance/time confusion violates that even more severely.

If we detangle the code, it will satisfy "once and only once", and it will satisfy the principle of least surprise.  And it will be possible to maintain it, and to spot bugs quickly rather than over the course of years.

[Carl]

I'm having a hard time seeing what's supposed to be "broken" here, beyond flat-out assertions that the feature has to change.


Acceleration, calculation of journey times, etc, are not broken in recent versions, despite your assertions. In fact, they're working impressively well. If there are errors in calculation, they are insignificant enough not to matter. So why "fix" them?

[neroden]

I'm having a hard time seeing what's supposed to be "broken" here, beyond flat-out assertions that the feature has to change.


Acceleration, calculation of journey times, etc, are not broken in recent versions, despite your assertions. In fact, they're working impressively well.

I'm still finding lots of breakage in recorded journey/waiting times, but it's very hard to pin down because it's subtle breakage.  The "fastest route" algorithm is not routinely sending passengers on the fastest route, but tracking down exactly why is very hard.  It's made much harder by the unreadability of the journey time calculations, which is because they're all doctored with the meters_per_tile nonsense.

If there are errors in calculation, they are insignificant enough not to matter.

They matter.  Given that the pak is not balanced yet, they don't seem to matter so much yet, but they will matter.

So why "fix" them?

Every single time you change anything in the code, you are going to introduce more places where this stuff has to be adjusted, and *you're going to miss some of them*.  James had to patch yet another one just a week or two ago, and he's been trying to implement this for what, 5 years ?

There are good ways to implement things and bad ways to implement them.  Historically, for various reasons, simutrans programmers have sometimes chosen the bad ways.  It is best to fix these mistakes.  (Another dramatic example is the rotation code; while this works, I don't know why anyone would ever implement it by rotating the world.  It makes it impossible to rotate network games.  It would be quite feasible to implement it correctly, by having "viewports" onto the world, and it's another thing on my to-do list.) 

Having a correct system architecture saves an awful lot of trouble in the future and makes it possible to make further improvements.  Right now, the code is a hairball where further improvements are practically impossible because the code is so hairy that touching it breaks things.

---
I should be clear that I'm viewing this code from the point of view of a programmer who maintains old code, and when appropriate, I do big refactors for readability, debugging, and cleanliness.  It's something I'm very good at.  If you're not good at it, you may not see the value.

[Erik]

I'm not a real programmer and (jet) not really good at it.
But still I understand the value of good coding and happy that there are people who are willing to do something about this.
I'm also convinced that it will fix some bugs and make further bug fixing easier.

@neroden: I have the same frustrations as you sometimes.

[kierongreen]

I don't know why anyone would ever implement it by rotating the world.  It makes it impossible to rotate network games.

Rotation was introduced before network support was added. Rotating the world means that there is no performance hit other than during rotation, and that working code remained untouched with changes limited to very specific parts of code. That's not to say that it's still the best solution, but at the time it was the most suitable.

[isidoro]

In Programming, you sometimes (or frequently) have to trade correctness for efficiency.  But the way rotation is implemented in ST is hard to buy.  It completely breaks the model-view-controller pattern.  The consequences of doing that have been evident with time.

I seriously doubt that it was the most suitable solution even at that time.  Not few bugs have come from the rotation code.  Bad design decisions have to be carried on and on and give a lot of problems.  Sometimes a redesign, even if it means a lot of effort, is well worth in the long run.

[jamespetts]

I'm still finding lots of breakage in recorded journey/waiting times, but it's very hard to pin down because it's subtle breakage.  The "fastest route" algorithm is not routinely sending passengers on the fastest route, but tracking down exactly why is very hard.  It's made much harder by the unreadability of the journey time calculations, which is because they're all doctored with the meters_per_tile nonsense.

Lacking time to respond fully at present, but a brief response is better than nothing: this seems rather the crux of things - if there really are bugs, how best to deal with the matter would be a considerably higher priority. What is the nature of the behaviour that you have observed that gives you to think that there might be bugs? I have not noticed anything, and neither, it seems, has Carl, who has been spending the last few months painstakingly building the whole UK railway network - complete with timetables, to which the trains mostly stick when given realistic values (with only one exception so far as I am aware) - using Simutrans-Experimental.

One other brief point: however we implement the code, we will still need to define somewhere in the code a relationship between any given vehicle movement speed (including the current speed, at present the only option) and a specific distance for timing purposes. Adding variability to the vehicle movement speed does not seem in principle to make this any simpler: we will still need an equivalent of the meters per tile setting somewhere. If the suggestion is that this is implemented in a different place in the code, does it not follow that it is possible to use that different implementation to retain the current fixed speed of vehicle movement by automatically pegging that movement speed against the distance per tile setting? In other words, is a conceptual change of the relationship between meters per tile and timing really needed here, or will an implementation change suffice? Is the current system different in principle to your proposed system with the single modification that there is a fixed relationship between meters per tile and the movement speed?

[neroden]

Lacking time to respond fully at present, but a brief response is better than nothing: this seems rather the crux of things - if there really are bugs, how best to deal with the matter would be a considerably higher priority.

Fix the underlying code.

Adding variability to the vehicle movement speed does not seem in principle to make this any simpler:

Actually, it does.  It does make it simpler in principle and in practice.  At this point, since I've attempted to explain the technical reasons, perhaps the only way to say it is to argue from authority: I'm a better programmer, I have experience with this.  It does make it simpler.

If the suggestion is that this is implemented in a different place in the code, does it not follow that it is possible to use that different implementation to retain the current fixed speed of vehicle movement by automatically pegging that movement speed against the distance per tile setting?

Well, technically you could do that, but why would you want to?

There are several very good reasons why you might want to make vehicles visually move faster.  As I may have noted, I generally run my games at 2.00 at all times.  It reduces computational load and therefore makes for better network games.  Et cetera.

In other words, is a conceptual change of the relationship between meters per tile and timing really needed here,

Yes.

or will an implementation change suffice?

The implementation change is the conceptual change.  We can hide it so that casual players don't notice, if you like.

Is the current system different in principle to your proposed system with the single modification that there is a fixed relationship between meters per tile and the movement speed?

Yes.  It is different in principle.  I have explained this.  My system would actually follow physics as people understand it.  The current system does something completely baroque.  If I may use an analogy, my system is like Kepler's ellipses, and the existing system is like epicycles.

---
My system is ALSO less computationally intensive.

---
The epicycles comparison is apt.  You'll get a correct computation in the end (if you keep spending time debugging it for years and years on end), but you're doing it the hard, unmaintainable, unreadable way.  Why not do it the easy, maintainable, readable way?  I don't understand your resistance to doing it right.

If they are both done correctly, they will give the same results, but mine will be *faster* and will contain *smaller code* and will be *more readable* and easier to maintain.  Yours took five years to get correct (at least).  Mine won't.

----

What is the nature of the behaviour that you have observed that gives you to think that there might be bugs?

Try running some long 18th century shipping lines -- ones where the travel time for each ship is measured in months.  The journey time recording, and the routing selection, is not right at all; it's hard to explain what's wrong, the passenger choice between multiple routes just isn't right, the average waiting times are wrong, and the journey times are wrong.

Now, I suspect the problem in this code is not really in the meters_per_tile computations, but in another part of the journey time code.   Normally, I'd just track it down and debug it.  But the meters_per_tile nonsense makes it impossible for me to debug the code.  It's unreadable.  Because of the meters_per_tile nonsense, variable values don't contain meaningful or readable time numbers so I can't debug it with a debugger, either.  I have to get rid of this gunk in order to figure out what's going on in the code. 

How can I get it through your head that the current implementation is not maintainable?

[isidoro]

neroden, I know how you feel.  It reminds me an old thread about some ST-Standard code that would prevent deadlocks just by looking a couple of tiles ahead of an intersection.  I now and again work with deadlock theory and it has little/nothing to do with that but it was hard to make me understood.  In fact, I wasn't able to do it.  Be patient...

You're asking a big effort and to throw away something that (at least) seems to work and that have cost a lot of time to develop.  It's natural that people involved in it want to be very sure that it is worth the change.

Usually the best teacher isn't the one that knows more.  Much on the contrary, best teachers know less but know how to make others understand the few things that they know.

[jamespetts]

Thank you everyone for your contributions to this thread: it is most worthwhile to have an in-depth discussion about issues such as these. Thank you, Nathaneal, for your patience; as indicated on other threads, I am currently away from home, and had been busy in the few days before that. I now have a little more time in which to respond, but I do not currently have access to my development environment.

Isidoro is correct when he writes,

You're asking a big effort and to throw away something that (at least) seems to work and that have cost a lot of time to develop.  It's natural that people involved in it want to be very sure that it is worth the change.

It takes a considerable amount of effort to think through the various implications of what is proposed, how that will affect the code, maintenance of the code, future proposed projects, and, perhaps most critically, pakset balance (see, for example, this guide to the relationships between speed, time and money in the current version of Experimental, this lengthy discussion of the calibration of mail production, this even lengthier discussion of calibrating passenger generation, and this discussion relating to calibrating industry passenger demand, all of which have some detailed calculations about how to calibrate things based on the current system). I know that you have a greater understanding of and skill at coding than I do, and can process these things more expeditiously than can I, but since it is I that will have to be doing the maintaining of the code and pakset for years into the future, I have to be able to understand it and all its implications myself before I can be content to commit any changes as fundamental as those proposed (not least because I will have to understand how the new system will work in practice and interact with things such as passenger/goods/mail generation, costs, physics, timings and computational intensity in order to work on my own projects with Experimental). With that in mind, I should be very grateful if you could assist me in carefully extrapolating the consequences of the proposed change so that I can fully understand all of its various impacts.

First of all, starting with the threads to which I refer above about calibrating the generation of passengers, mail and goods - what sort of impact would your proposed system have on these calculations? What sort of rebalancing of a pakset that has so far been balanced to use the existing system would be needed were your suggested change implemented? How, if at all, would this affect the convoy spacing feature? How, for example, would it affect Carl's giant saved game, which accurately recreates a growing proportion of the UK rail network, with realistic timetables? If there is to be less computational intensity by using larger numbers rather than computing more often, does this mean that vehicle movement will become less smooth at higher (lower?) meters per tile settings?

Also, one issue that has troubled me somewhat: we will always have to define a tile length in meters for any given relationship between real time speed and game speed. There is currently a single, fixed relationship between real time speed and game speed. I understand that you are proposing a variable such relationship. But, nonetheless, the current relationship would still be an option. There would therefore still be a question as to what the relationship is between that relationship and the relationship between a tile and its length in meters. That relationship is what is currently customisable by the meters per tile setting, which automatically changes the timing to match. Would you propose to have this fixed? If so, at what value? Is it better that things are fixed than flexible as they are at present?

In terms of the benefits, am I correct in understanding that the reason that this is proposed is to improve the maintainability and readability of the code, not because there are any known bugs associated with the feature itself? Can you elaborate a little on how the current system has hampered you tracking down the bugs in very long distance transport? If the numbers that you are getting out of variables are not meaningful in themselves, could you not just set up some temporary code, as I do in that situation, to translate the values into something readable so that they make sense in a debugger (such as "const uint16 TEST_distance = tile_distance * welt->get_settings().get_meters_per_tile()")? It would help me to have a better insight into what about the current system makes it hard to debug/maintain. (I understand that it might appear counter-intuitive at first for a setting ostensibly about distance to affect time, but I do not think that that is a reason enough by itself to change things, as a person who merely plays the game and does not do any development need never know about it, and for developers, once one gets past the basic concept, which is not hard once it is explained, the way in which the relationships work are not particularly hard to grasp on a conceptual level). One of the things that I am having some difficulty in grasping, and perhaps with which you can help me is this: once one gets over the initial hurdle of the (relatively slight) conceptual complexity of the relationship between time and the meters per tile value, how does this affect the actual complexity of the implementation as compared to a system such as you propose? Apologies for being dim if you have already explained this and I have not fully understood.

Incidentally, as to the idea that the fact that there have had to be a great many fixes to the feature over the years, perhaps this does not reflect well on my coding ability (I am, after all, somebody with no professional coding background, and learned C++ specifically for Experimental back in 2009), but I have not noticed that the meters per tile feature is unusual in this respect: I have found that many of the features of Experimental have thrown up multiple bugs sometimes years after they were first implemented. The interaction between me being a relatively inexperienced coder and the code itself being somewhat unstructured (its design not taking as full an advantage of the principles of object-orientation as possible, sometimes for valid performance reasons, but sometimes for no readily discernible reason; the absence of an "asset" base class which handles all purchasing, sale and maintenance costs is a good example of the latter) is probably to blame for this.

Would you mind also helping me to understand this element of things rather better:

The confusion between distance and time makes it impossible to implement "once and only once" here.

Why does the fact of meters per tile affecting times given the fixedness of speeds make it harder to implement this principle? Can you perhaps give a simple example (hypothetical if that makes it easier) of where this is so?

Incidentally, as to epicycles, that is an interesting piece of history: I was not aware of this (outdated) theory until I Googled it just now.

Thank you again for all your work on this; I hope that you understand why this has to be considered very carefully to make sure that I fully understand all the implications of this before committing to it, even if those implications are obvious to you. I hope that you do not feel too frustrated by such a necessary process.

Incidentally, if this is to be done, then it had better be done after the release of 11.0, as it would not, I do not think, be sensible to have such a major change implemented so soon before a release: I hope to release 11.0 after I have fixed the Linux server bug, the performance issue and the remaining reported bugs with freight station coverage.

[neroden]

Incidentally, if this is to be done, then it had better be done after the release of 11.0, as it would not, I do not think, be sensible to have such a major change implemented so soon before a release: I hope to release 11.0 after I have fixed the Linux server bug, the performance issue and the remaining reported bugs with freight station coverage.

Absolutely, definitely.  This is a structural change.  You should always do those after a release, or on a branch, not on a release candidate.

For one example of why I want to fix this (sigh), I was finishing off the accounting merger.  (By the way, I like the new accounting system.  It's very much cleaner than the old one.) 

I recently fought a mysterious bug introduced during the accounting merger... it turned out to be because revenue calculations are multiplied by (roughly) 3000 and then need to be divided by 3000 later, and I'd lost one of the factors.  This is a case of extra units.

The revenue code turns out to be several times more complicated than I expected, for several reasons.  One of them is the magic factor of 3000.  One of them is because meters_per_tile shows up in an unexpected location.  I would have *expected* to start with a revenue value which was per kilometer, and work from there, but nooooo. 

In terms of the benefits, am I correct in understanding that the reason that this is proposed is to improve the maintainability and readability of the code, not because there are any known bugs associated with the feature itself?

Yes.  I hope to eliminate unknown and future bugs.  :-)

It is best to minimize the number of different units floating around.  Unit conversion errors destroyed the Mars rover, remember.  :-)  Several unit conversions are necessary for simutrans, but this is too much.

Can you elaborate a little on how the current system has hampered you tracking down the bugs in very long distance transport? If the numbers that you are getting out of variables are not meaningful in themselves, could you not just set up some temporary code, as I do in that situation, to translate the values into something readable so that they make sense in a debugger

Well, frankly, no, because I'm spending my time figuring out what transformations have to be performed!  The problem is, first and foremost, the problem of figuring out what the heck sort of manipulations have been performed to the data in a given variable.  I've been chasing them halfway across the code-base just to figure out what they mean.

Remember that on top of the meters_per_tile business, there's a time-factor adjustment (bits_per_month) floating around too. 

If meters_per_tile adjusted only distance, and bits_per_month adjusted only time, then we could write clean *finance* code.  The money subroutines would always think in terms of time (in hours/minutes/seconds) and distance (in kilometers), and could completely ignore both meters_per_tile and bits_per_month.

Instead, nearly every piece of finance code has a set of adjustments for meters_per_tile and for bits_per_month, and *THAT* is what is violating "once and only once".

perhaps this does not reflect well on my coding ability (I am, after all, somebody with no professional coding background, and learned C++ specifically for Experimental back in 2009), but I have not noticed that the meters per tile feature is unusual in this respect:

Well, perhaps it is just your level of coding experience.  You'll probably get better with practice and research.  Although I also have no *professional* coding background, I do have about 25 more years amateur practice in amateur coding than you; I do know most of the CS undergrad curriculum; I've debugged machine language and written expert systems in PROLOG and written toy compilers.

You may notice that occasionally I walk in and sweep up six or eight bugs at once.  Or fix bugs nobody's noticed yet, just by reading the code and spotting a logic error.  Or implement a feature with maybe 1-2 bugs total.

Now, I (and quite a lot of other skilled programmers) can do this with "clean" code.  With "clean" code, each function has a clear "contract" (it promises to do something very specific) and each variable has a clearly defined data format.  You can then spot -- just by reading the function -- when the function isn't obeying its contract, and so fix it.

I find it much, much harder to do that with certain parts of the code, including the meters_per_tile code and the rotation code. 

The current situation with meters_per_tile means that there are, at least, the following different time codes in use:
(1) milliseconds
(2) ticks (there is a clean separation between milliseconds and ticks, thankfully)
(3) "game minutes" (/hours/seconds)
(3.5) the tenth-of-minute variant, too
(4) "game months" (/hours/seconds)
(5) "pakset months" (which are in a fixed ratio to "game minutes", not to "game months")
(6) "factory pakset months" (which are 1/4 of pakset months, if I remember correctly, or is it 4x?)
(7) "meters_per_tile adjusted game minutes"
( "meters_per_tile adjusted game months"
(9) "meters_per_tile adjusted pakset months"
(10) "meters_per_tile adjusted factory pakset months"
You have to keep track of whether things are "meters per tile adjusted" because you have to fake up the speeds for all of them: if the numbers are "meters per tile adjusted" then you can't get speeds by dividing distance by time...

...This is too much to keep track of.

It's now very hard to figure out, for a given function, what its contract is, because it's unclear which of the 10 or more formats it's using.  Even if it seems clear that it takes a "time" argument, you have to trace through the entire codebase to figure out what sort of "time" argument it is.

If this can be reduced to approximately 5 formats, with three of them isolated to specific subsystems, it becomes a lot easier to know what's going on, just by reading the code.  And that change shouldn't be too hard to do.  Some weirdness is going to be needed in the most frequently called code, but the code which is called less often should be able to be clean.

I haven't even started listing the different speed formats used internally.

Anyway, a short catalog of classic programming tips, starting with the earliest rules developed by assembly language programmers:
(1) Never use raw numbers in code, except for "-1", "0", "1", and "maxint" and "minint" (the maximum and minimum values for a given integer type).  Always use named constants.  (Or find a way to not write the numbers explicitly at all.)  I often violate this rule in quick-and-dirty test code, but I try to clean it up.  Sometimes it's worth ignoring this rule, in stuff like user interface code where the spacing is defined somewhat arbitrarily according to how "pretty" it looks, but even there it's usually better to have named constants.
(2) Break out any computation or logic, which appears more than twice, into a common subroutine.  Exceptions may include extremely simple pieces of logic, but even those often should have subroutines.  For instance, 'if (sp)' is in far too many places in the Simutrans code; more recent code contains static function wrappers which contain the if (sp).   Even if a computation only appears twice, it may deserve a subroutine.  If it appears only once, it probably doesn't deserve a subroutine, but it might deserve a subroutine anyway.  (If you find that you have to pass a huge amount of data to the subroutine and back out again, then you probably don't have an appropriate subroutine.)  Another exception here is backwards-compatibility code, where it's usually OK if it's not well-maintained.
(3) Use as few different units as you possibly can.  You'll obviously need one for time and one for distance, and in a game like simutrans, you'll need at least three for time (real time, "year" time, and "hour" time) and at least two for distance ("km" distance and "tile" distance).  Avoid having any more units than that if at all possible.  (There may be computational issues which will require doing something else, but those are much rarer than you might think.)  Data format conversions are  a pain, and a source of constant bugs, and you want to avoid them.
(4) Make the "contract" of every function as clear as possible: what it takes as inputs, what it puts out as outputs.  In C++ a function can have multiple outputs (by passing in reference variables, the ones with the & in front) and this is useful.
(5) Make a function complete in itself: this is the rule of "structured programming".  A function should not have to refer to any data except what was passed to it as arguments.  In "object oriented programming", we make an exception for the data of the object which the function is part of -- but that data is special, as it should relate to the "permanent attributes" of the object.  Apart from that data, the rule still holds.

You'll probably look back over your last 5 years of code and spot where you didn't follow these rules.  I know 20 years ago I looked back at my old code and did exactly that.  :-)

What sort of rebalancing of a pakset that has so far been balanced to use the existing system would be needed were your suggested change implemented?

It *should* have no change needed.

If there is to be less computational intensity by using larger numbers rather than computing more often, does this mean that vehicle movement will become less smooth at higher (lower?) meters per tile settings?

This is a possibility.  I will have to watch for this.  I do not believe it's going to be noticeable at meters_per_tile values from 62.5 to 16,000.

If you like, I can implement this in the *completely* functionally-identical manner, so that it appears exactly the same to the casual player, and by default keeps the computational intensity the same.   The current meters_per_tile could do pretty nearly the same thing it does now -- but with fewer lines of code, because it should only show up in distance computations, not in time computations, and only indirectly in speed computations.

In order to actually have the right internal "knobs", I would need a "real_meters_per_tile" and a "ticks_per_game_second" -- but I can have the existing meters_per_tile set both of them, if the new parameters aren't provided in the pakset.  I think I would still want to implement target_ticks_per_real_ms just so that network games could be run at different speeds.

[jamespetts]

Thank you very much for your detailed reply - that is most helpful. One specific question arising out of this just to clarify one point (and forgive me if I am being a little dim): the would eliminating all instances of times that are not adjusted by meters per tile not have the same effect on the code implementation, or would this somehow be more complicated, less consistent or violate any of the good programming rules that you outline above? If time conversions were handled by subroutines that always adjusted by meters per tile, would it not be possible to achieve this cleanly with fewer conceptual changes; or is there a reason that this is harder than the suggested reconceptualisation?

[jamespetts]

Now that we have the release of 11.0, it is worthwhile bringing this forward for consideration again. The option discussed earlier of implementing this in such a way as to be functionally identical to the existing system, but altering the underlying mechanics to be more robust as suggested, appears on the face of it to be appealing. Is this still a viable option?

[neroden]

Hey.  This is still a very, very good idea, and it's still viable.  It also looks like it hasn't been fixed yet.  Unfortunately health problems struck and meant I was unable to work on Simutrans for 10 years, and am still unable to work on it this year.  Maybe next year, if you're willing to have it fixed.

(To answer your other question, no, eliminating all instances of times not adjusted by meters_per_tile would be much, much worse, and would involve the same amount of work.)

[jamespetts]

Hey.  This is still a very, very good idea, and it's still viable.  It also looks like it hasn't been fixed yet.  Unfortunately health problems struck and meant I was unable to work on Simutrans for 10 years, and am still unable to work on it this year.  Maybe next year, if you're willing to have it fixed.

(To answer your other question, no, eliminating all instances of times not adjusted by meters_per_tile would be much, much worse, and would involve the same amount of work.)

Hello and welcome back! I am very sorry that you have had such health problems for such a long time. It is good to see you back.

In principle, yes, it would be good to have a more robust way of doing the scaling. This would definitely be a *lot* of work (including complex work with the movement code, which I have really not looked into at all). If you would like to do this, you would be most welcome.

One thing to bear in mind, however, is that we are currently in a long transition process from 14.x to 15.x. The 15.x features are quite complex and have proven difficult to implement, which is why they have been delayed. I had made quite a bit of progress last year and in the early part of this year, but have been somewhat delayed by the fact that our UI expert is taking a break, and the UI for these features is not implemented enough to be able to test them easily, making further progress very hard (for the avoidance of doubt, this is not a criticism of our UI expert - he has worked tirelessly on massive UI improvements since 2019 and this is just a voluntary project). That is something of a tangent - but the important thing to note is that any major change has to take into account the fact that there is a major set of changes in the works. The branch for this major set of changes is the ex-15 branch on our Github repository.

I think that we would need to discuss in some detail how the new system would work before implementation to ensure that the new system works well with paksets and balancing.

[neroden]

All right.  It's going to take me a long time to get to this anyway, since I have *massive* real-life obligations right now which will continue for more than a year.  I'm only working on little features in the corners right now in my not-extensive free time.

Mathematically, the new system should be almost completely transparent to the pakset writers and for balancing purposes.  There would be a couple of top-level flag switches to handle the transition (swapping "real_meters_per_tile" into the simuconf.tab instead of "meters_per_tile") and then the idea is that it all works the same from the point of view of the pakset creator and player.

There will be internal computational differences, which means you'd want to stop a server running "meters_per_tile" and restart it running "real_meters_per_tile" to do the update -- but they should amount to variations in roundoff error.  They shouldn't have *any* impact on pak balancing or gameplay.

The process might, however, uncover latent pre-existing bugs which would be fixed, which *would* have an impact on pak balancing and gameplay.

[jamespetts]

All right.  It's going to take me a long time to get to this anyway, since I have *massive* real-life obligations right now which will continue for more than a year.  I'm only working on little features in the corners right now in my not-extensive free time.

Mathematically, the new system should be almost completely transparent to the pakset writers and for balancing purposes.  There would be a couple of top-level flag switches to handle the transition (swapping "real_meters_per_tile" into the simuconf.tab instead of "meters_per_tile") and then the idea is that it all works the same from the point of view of the pakset creator and player.

There will be internal computational differences, which means you'd want to stop a server running "meters_per_tile" and restart it running "real_meters_per_tile" to do the update -- but they should amount to variations in roundoff error.  They shouldn't have *any* impact on pak balancing or gameplay.

The process might, however, uncover latent pre-existing bugs which would be fixed, which *would* have an impact on pak balancing and gameplay.

Excellent, thank you for clarifying.