[patch] Cargo Payment Fix (r27460)

Forum for technical discussions regarding development. If you have a general suggestion, problem or comment, please use one of the other forums.

Moderator: OpenTTD Developers

Vetteman06
Engineer
Engineer
Posts: 59
Joined: 03 Aug 2013 16:39

[patch] Cargo Payment Fix (r27460)

Post by Vetteman06 »

Cargo Payments
Cargo_Payment_R27460.patch
This is a small patch to fix cargo payment.
This is probably a bug in the game. So if people think so, I will create a bug report.

So, I was trying figure out why some airports payed more for flights than others.
And what i found was basically a calculation that favored diagonal stations.

If you have one station and directly (S,E,N,W) of this station you have another station.
Let's use 10 tiles as an example.
You would get paid 10 tiles distance for any cargo.

Now for diagonal...
Same situation above except it is 10 tiles down and 10 tiles over. (Diagonal)
It would use the calculation of Manhattan Distance and calc that as 20 tiles distance. (10 + 10)
So your plane would fly approx. 15 tiles to deliver the cargo (planes, trains, ships all move diagonal).
This is called Euclidean Distance.
But would get paid for 20 tiles distance.

So for any cargo type, you could amplify your earnings by trying to place stations as diagonal as possible from each other.

So I made a small patch to fix that issue...
But it should probably be in trunk.

And it should probably be enhanced to take vehicle type into consideration.
Planes, ships, trains use Euclidean
Road Vehicles use Manhattan.
Or make roads go diagonal also... Probably much harder to do than to enhance the payment calc.

The other thing that was a surprise was the function in map.cpp that claimed to calc Euclidean...
It doesn't...
So I made a correct one that does.

Anyway, I would be interested in any comments on this...
User avatar
JGR
Tycoon
Tycoon
Posts: 2558
Joined: 08 Aug 2005 13:46
Location: Ipswich

Re: [patch] Cargo Payment Fix (r27460)

Post by JGR »

Using doubles and floating point square root is generally a bad idea for game variables as the rounding issues can lead to desyncs in multiplayer.
The IntSqrt() function is probably a better idea.

The function DistanceSquare doesn't claim to calculate the Euclidean, it's Euclidean squared. The square aspect is in the name of the function and multiple times in the documentation...

It doesn't really make sense to the change the distance calculation per vehicle type, as with cargo dest payments are done per complete trip, only the feeder payments are done per vehicle hop.

I would not personally consider using the Manhattan distance instead of the Euclidean a bug, it's a design choice carried over from the original game.
Ex TTDPatch Coder
Patch Pack, Github
Aphid
Traffic Manager
Traffic Manager
Posts: 168
Joined: 16 Dec 2011 17:08

Re: [patch] Cargo Payment Fix (r27460)

Post by Aphid »

Actually, I already made a patch for this, see also FS#6121.

There's a couple things you missed with that implementation, which I consider a bit naive.

First off, you can't really use diffrent calculations for diffrent vehicle types, as it allows for exploits using transfers, e.g. using a transfer to get a free switch to a more beneficial calculation for either the full part of the payment or 75% of it. E.g. I could use a train to get 75% of an euclidian calculation, and then transfer to a road vehicle for the last 3 tiles and get the last 25% using manhattan distance, which still leaves me with 25% of the current bonus gained 'unfairly' from a diagonal route over a straight route. Since ships, trains have the shortest possible routes, road vehicles should use their calculations. It would be a pretty hard task to make your algorithm completely safe against transfer exploits to get effective manhattan payment back. You're welcome to try though using the same calculation for all types of vehicles makes things more simple.

The proper way of thinking about it is to say we want the direction of travel to have no effect on the income, only the distance and time taken when using the most direct possible route. Using euclidian gets you close, but not quite exactly to the point.

So second: Euclidian distance is still the wrong type to use. Imagine going 200 tiles left, then 100 tiles up. Euclidian distance of this is sqrt(5)*100 or about 224. Manhattan distance is 300. However, a train travelling this route can't actually use the euclidian distance; vehicles can only travel in 8 directions in openTTD. So the most direct route is to go 100 tiles diagona, and 100 tiles straight. This has a length of 141 + 100 = 241 tiles. The point is that 'the shortest distance route' a vehicle can take between these two points is 241 tiles long. Hence any cargo payment should be done assuming the distance is 241.

The way I've calculated this is to use an approximation. First, take the distance between two tiles as [dx, dy]. Then calculate U + max(dx, dy) - min(dx, dy), where U = min(dx, dy) * sqrt(2). This is done using integer math by just multiplying everything by say 65536 (shift left by 16 places), e.g. do (pseudocode):

Code: Select all

note: 2<<16 * sqrt(2) ~ 92682. 
0) function planeDistance(int32 dx, int32 dy)
1) int32 dd, ds
2) ds = 2<<16 * max(dx, dy) - min(dx, dy)
3) dd =  92682 * min(dx, dy)
4) return (ds + dd) >> 16
The '92682' magic number here is just the closest integer to sqrt(2) * 2^16. It's really close to muliplying by sqrt(2) -- it's at most 1/65536th off. Which when it comes down to it is a much closer approximation than the current, which can be off by as much as 41%.

Another small thing; we don't risk integer overflow as we know that dx <= 4096 and dy <= 4096. Thus left-shifting 16 bits is fine, as the largest number possibly handled is 4096 * 92682 = 379,625,472 which is still within the limit for an int32.

I do agree using manhattan is bad. Design choice or not, there should be an option not to have this weird artifact where diagonal lines are more profitable; it's unintuitive.
Vetteman06
Engineer
Engineer
Posts: 59
Joined: 03 Aug 2013 16:39

Re: [patch] Cargo Payment Fix (r27460)

Post by Vetteman06 »

Aphid wrote:Actually, I already made a patch for this, see also FS#6121.
:bow:
So very cool that you are already looking into a fix for it...
I had not thought of the 8 direction issue...

But I think this is probably a bit short sighted...
There are already patches that expand dx and dy much higher.
Even OpenTTD has expanded this over time.
Aphid wrote:Another small thing; we don't risk integer overflow as we know that dx <= 4096 and dy <= 4096. Thus left-shifting 16 bits is fine, as the largest number possibly handled is 4096 * 92682 = 379,625,472 which is still within the limit for an int32.

So this comment is what I would like to try and discuss...
Aphid wrote:First off, you can't really use diffrent calculations for diffrent vehicle types, as it allows for exploits using transfers, e.g. using a transfer to get a free switch to a more beneficial calculation for either the full part of the payment or 75% of it.
Ok, when I look at the cargopacket, it tracks from the source out and it isn't until the packet reaches the destination that the final calc is done..
Wouldn't it be better to include the destination into the cargo packet?

Then you could do something like get the distance from source to destination and then feeder to destination.
If you are moving away from the destination, reduce the feeder payment. Like 20% or something.
if you are moving parallel to destination, then use the median.. Like 40%...
If you are moving closer... Then 60%...

But if the final segment is using road vehicles...
Then you could calc current station to final destination using Manhattan and not have the exploit.

So is it not possible to have the destination in the cargopacket when it is generated?

The main motivation is that there are so many negative income planes when playing on large maps with many transfers...
Out of a 1000 planes, it is not uncommon to have 100+ planes in the negative.
Which is annoying since the company is largely positive.
And your hub to hub planes rake in huge profits...

Trying to balance out every planes income is probably a major undertaking...
I was just trying to find something to bring it closer to a balance.
Vetteman06
Engineer
Engineer
Posts: 59
Joined: 03 Aug 2013 16:39

Re: [patch] Cargo Payment Fix (r27460)

Post by Vetteman06 »

Aphid wrote:So second: Euclidian distance is still the wrong type to use. Imagine going 200 tiles left, then 100 tiles up. Euclidian distance of this is sqrt(5)*100 or about 224. Manhattan distance is 300. However, a train travelling this route can't actually use the euclidian distance; vehicles can only travel in 8 directions in openTTD. So the most direct route is to go 100 tiles diagona, and 100 tiles straight. This has a length of 141 + 100 = 241 tiles.
So I looked at your formula and I thought maybe I would give it a try...
Yours is probably faster, but much harder to understand.
I essentially build a tile that is the exact diagonal and get the Euclidean of that... Then add in the offset...
In your example of over 200 up 100 equaling 241.
DistanceOpenTTD will return 241 for that condition.

This uses the IntSqrt function mentioned by JGR
Since IntSqrt is uint32, should be good for any map size...

uint DistanceSquare(TileIndex t0, TileIndex t1)
{
const int dx = TileX(t0) - TileX(t1);
const int dy = TileY(t0) - TileY(t1);
return dx * dx + dy * dy;
}

uint DistanceEuclidean(TileIndex t0, TileIndex t1)
{
return IntSqrt( DistanceSquare(t0, t1) );
}

uint DistanceOpenTTD(TileIndex t0, TileIndex t1)
{
const uint dx = Delta(TileX(t0), TileX(t1));
const uint dy = Delta(TileY(t0), TileY(t1));
return DistanceEuclidean(t0, TileXY( TileX(t0) + minu(dx, dy), TileY(t0) + minu(dx, dy) )) + Delta(dx, dy);

}
Aphid
Traffic Manager
Traffic Manager
Posts: 168
Joined: 16 Dec 2011 17:08

Re: [patch] Cargo Payment Fix (r27460)

Post by Aphid »

Actually, I've thought a bit about this, but map sizes above 4K*4K shouldn't get you any problems, since the 'bigger maps' patch still has a limit of 16M tiles as far as I know. E.g. you could go 8192*2048. But in that case the 'diagonal part' of any distance is less than 2048 tiles on one side, which means the largest int32 number in the calculation is half as small as before -- we're still fine using int32. Same for any larger size; the shortest side of the map matters. The largest 'supported' map size by my formula is 16K*32K, which has 512M tiles and probably won't fit into 4GB memory anyway, apart from being both impossible to generate and to fill with vehicles and incredibly slow to play on using current hardware -- not something to really worry about. If it is, just use an int64 and we're fine for anything.
Yours is probably faster, but much harder to understand.
The calculation using 'plane distance' is simply faster because it's approximately 10 math operations; the non-linear bit is pre-computed. Doing a square root involves some ~100ish math operations, and we're really only interested in one 'square root'; that of 2. In general though doing one square root every time a vehicle is paid won't really cause problems. After all a computer does on the order of 10^10 operations per second; and you can't really get more than about 65,000 vehicle payments per second. So if your formula uses less than 10^5 operations, it's probably not a problem when it comes to performance.

Indeed, my formula looks a bit opaque and probably could do with some code commenting explaining how it works -- there's some 'bit tricks' involved. Essentially yours does the same thing, with the bonus that it's more accurate with large numbers - though for the purpose it's used for that doesn't matter much; 0.01%-order inaccuracy in cargo payments isn't a big deal -- the distances won't get big enough for there to be more than a 1-tile inaccuracy between the two methods. On the other hand, it's a bit slower, mostly since the IntSqrt function is relatively slow. But again since it's likely within the 10^5 ops target it's fine to do things like this.

Edit: Your formula will also be less accurate for small numbers. E.g. when we're looking at a distance of 3 diagonal tiles; isqrt(18) = 4 (if rounded down -- haven't looked at the exact implementation), while the real number is 4.24264068712. I'm working with what's effectively (92682 * 3) / 65536, or approx. 4.24264526367. Note the bolded part, we're accurate to 6 decimal places instead of 1 here. This diffrence is more significant than the inaccuracies we're seeing with very large distances relatively speaking.
So is it not possible to have the destination in the cargopacket when it is generated?
Generally, trying to figure out where something is going to go beforehand can be tricky because of programmable trains. Someone at openttdcoop might have a formal proof of the following statement: Openttd is Turing-Complete on an infinitely large map using infinitely many trains and tracks. E.g. openttd can be used to create 'train-computers' much like you can in dwarf fortress or minecraft - you can make a NOT gate, an OR gate, and you can make a memory cell, and a timer; those are the four necessary components. This means figuring out where a train 'will eventually go' involves literally running the simulation; as you can create a scenario where two trains race and the one who gets to a junction first gets destination A, the other gets destination B.

The 'pay 75% of a partial route, figure out the rest later' is thus a band-aid fix -- we don't really pay the company yet but want to allocate parts of the profit to vehicles that are just transferring. Because routes taken usually aren't direct (there's obstacles like mountains or buildings in the way) it's to correct for this.

With the default way of handling things, assigning two diffrent formulas => the vehicle type that is used 'last' determines the formula used. That's a problem as transferring diagonally transported stuff to a road vehicle right before delivery then increases profit -- introducing a new exploit.

Fixing road vehicles:
Now if you'd want to have RVs use manhattan distance and the rest use plane distance, you could perhaps write a 'record' of how many tiles a packet travelled by road R, and how many tiles by 'other means' O. Let payment(manhattan) be M, payment(plane) be P (computed for the whole real distance between origin, destination). The true payment for a cargo packet becomes:

Code: Select all

T = (R * M + O * P) / (R + O)
Where:
  • M = cargovalue * distance(manhattan, source, destination)
    R = amt. tiles travelled by road
    P = cargovalue * distance(plane, source, destination)
    O = amt. tiles travelled by other means
If R = 0, use my method. If O = 0, then use the original openTTD formula. (Having these two checks will speed up the computation somewhat, they're common branches).

Such a record could be handled by incrementing one of two counters for each cargo packet in a vehicle every time a vehicle enters a new tile.

This requires some extra bookkeeping; quite a bit of new code. My patch just has the 'easy way out' using very little code. There's an argument to be had for using one formula ony (consistency) and also one for using two ('fairness' towards RVs -- current code treats them the right way if considered in isolation) so I did the simplest thing.

EDIT: Actually, using some number theory, we might improve the approximation of sqrt(2) somewhat; by using its infinite fraction expansion <1,2,2,2...>. Writing down the table:

Code: Select all

N	P	Q	S
-2	0	1	
-1	1	0	
0	1	1	1
1	3	2	2
2	7	5	2
3	17	12	2
4	41	29	2
5	99	70	2
6	239	169	2
7	577	408	2
8	1393	985	2
9	3363	2378	2
10	8119	5741	2
11	19601	13860	2
12	47321	33461	2
13	114243	80782	2
14	275807	195025	2
15	665857	470832	2
16	1607521	1136689	2
17	3880899	2744210	2
18	9369319	6625109	2
19	22619537	15994428	2
20	54608393	38613965	2
-----------------etc----------------------
These are 'convergents' of sqrt(2) with the amazing property that they are the 'closest' fraction of sqrt(2) of that 'size'. Meaning that for example p/q = 577/408 is the best approximation of sqrt(2) for any p < 1393 (or any q < 985, basically the same thing). There's quite a bit of math involved in proving this so I'm not doing that here. For example, using 114243 / 80782 -- we can multiply by 114243, add 114243 * 'the straight part', multiply by the cargo payment per tile, then divide by 80782. Or even use a bigger number and use int64 for increased accuracy.

Edit 2: For a formal proof, refer to chapter 6 (starting pg. 121) of Saylor. The <1,2,2,2,...> expansion of sqrt(2) can be seen by observing that 1 / sqrt(2) = (1/2) * sqrt(2) and apply induction.
Vetteman06
Engineer
Engineer
Posts: 59
Joined: 03 Aug 2013 16:39

Re: [patch] Cargo Payment Fix (r27460)

Post by Vetteman06 »

Aphid-
You make my head hurt... :D

:bow: :bow: :bow:
Aphid wrote:Edit: Your formula will also be less accurate for small numbers. E.g. when we're looking at a distance of 3 diagonal tiles; isqrt(18) = 4 (if rounded down -- haven't looked at the exact implementation), while the real number is 4.24264068712.
In my testing with IntSqrt, it does round down and should arrive at the same answer. Albeit more expensively...

Aphid wrote:With the default way of handling things, assigning two diffrent formulas => the vehicle type that is used 'last' determines the formula used. That's a problem as transferring diagonally transported stuff to a road vehicle right before delivery then increases profit -- introducing a new exploit.
So I have been thinking about this.... And I want to run it by you as a possible fix...

The reason it is an exploit is because the last leg of cargo travel isn't considered a "feeder".
Right now it does a calc of something like Final -= Feeder;
Where "Final" is calculated using the last vehicle's movement system.

If the final destination instead did a Feeder calc using Manhattan for Road Vehicle Call it "LastLeg", then did the final calc using Plane distance to get the correct final value call it "Final"...
Then your final formula would be Final = Final - LastLeg - Feeder;
You could even put in a check to only calc final if not a single hop cargo packet.
That way if the travel is all done by RV, they get paid in Manhattan.
But if mixed, get paid in Plane.

This would sort of cause the effect of player avoiding RV's. Because they would have to travel further to get the same pay.
But RV's are typically used in cities anyway. Where other forms of travel are not practical.
And the player can offset that a bit by adjusting the Feeder %.
And it would only happen with mixed vehicles on same route.

This should remove the exploit, letting your fix go into trunk.
Would improve the system without introducing an exploit anyway.

So, your formula of
Aphid wrote:T = (R * M + O * P) / (R + O)
Is a better idea... But without knowing the final destination, would be an exploit itself.
Players could run stuff all over the map, making the distances huge using fast planes or trains, before reaching final destination.
You would have no idea which tiles of travel were more valuable. (Heading towards the final destination)

I would think you would have to do some kind of % based system...
Like 85% plane and 15% RV.
But again, without Final destination, You can't really calc what percentage a vehicle is performing because you have no idea whether you are heading towards the destination or away.
One vehicle might be heading straight towards the destination, and one might be heading away from the destination.

But the overall effect of either system, is that all vehicles suffer for poor routes. Not the one vehicle that is messing things up...

Really need to know the final destination of the packet, then you could better estimate each legs value and payment.
Vetteman06
Engineer
Engineer
Posts: 59
Joined: 03 Aug 2013 16:39

Re: [patch] Cargo Payment Fix (r27460)

Post by Vetteman06 »

So after thinking more about the performance...
I came up with this formula...
It calcs the same, just no square root needed...
This should be very fast....


uint DistanceOpenTTD(TileIndex t0, TileIndex t1)
{
const uint dx = Delta(TileX(t0), TileX(t1));
const uint dy = Delta(TileY(t0), TileY(t1));
return minu(dx, dy) + (minu(dx, dy) * .5) + Delta(dx, dy);
}
Eddi
Tycoon
Tycoon
Posts: 8267
Joined: 17 Jan 2007 00:14

Re: [patch] Cargo Payment Fix (r27460)

Post by Eddi »

you cannot use .5 in an expression, you must rewrite this to use integers.

also. "x+0.5*x" is the same as "1.5*x"
Vetteman06
Engineer
Engineer
Posts: 59
Joined: 03 Aug 2013 16:39

Re: [patch] Cargo Payment Fix (r27460)

Post by Vetteman06 »

Eddi wrote:you cannot use .5 in an expression, you must rewrite this to use integers.
Why? I don't understand why, since the end result an integer...
Eddi wrote:also. "x+0.5*x" is the same as "1.5*x"


Nice!! I'll update it...
User avatar
Zuu
OpenTTD Developer
OpenTTD Developer
Posts: 4553
Joined: 09 Jun 2003 18:21
Location: /home/sweden

Re: [patch] Cargo Payment Fix (r27460)

Post by Zuu »

Can you guarantee that on every platform that OpenTTD runs on, the end result of each possible input will be the same?

The answer is unfortunately no. And this is why floating point math has to be avoided for game play parts which run on each client in MP and has to yield the same results, or a desync will occur. In MP each each client as well as the server run a copy of the game. After the initial map transfer, only commands are transferred from the server, not the complete game state.

Edit:
Ignoring problems with high dx and dy values, you can in theory do something like this to avoid floating point math. If you scale up or down the multipliers (150 and 100) that affect the size of the error compared to using a perfect floating point number. But the higher you make the multiplier the larger numbers will be produced and with large dx and dy it could overflow. So you need to study the constrains of dx and dy to assert what is possible to do.

Code: Select all

uint DistanceOpenTTD(TileIndex t0, TileIndex t1)
{
    const uint dx = Delta(TileX(t0), TileX(t1));
    const uint dy = Delta(TileY(t0), TileY(t1));
    return (150 * minu(dx, dy) + 100 * Delta(dx, dy)) / 100;
}
My OpenTTD contributions (AIs, Game Scripts, patches, OpenTTD Auto Updater, and some sprites)
Junctioneer (a traffic intersection simulator)
User avatar
JGR
Tycoon
Tycoon
Posts: 2558
Joined: 08 Aug 2005 13:46
Location: Ipswich

Re: [patch] Cargo Payment Fix (r27460)

Post by JGR »

Zuu wrote:Edit:
Ignoring problems with high dx and dy values, you can in theory do something like this to avoid floating point math. If you scale up or down the multipliers (150 and 100) that affect the size of the error compared to using a perfect floating point number. But the higher you make the multiplier the larger numbers will be produced and with large dx and dy it could overflow. So you need to study the constrains of dx and dy to assert what is possible to do.

Code: Select all

uint DistanceOpenTTD(TileIndex t0, TileIndex t1)
{
    const uint dx = Delta(TileX(t0), TileX(t1));
    const uint dy = Delta(TileY(t0), TileY(t1));
    return (150 * minu(dx, dy) + 100 * Delta(dx, dy)) / 100;
}
dx and dy are already integers, there is no increased accuracy by scaling up the multipliers, or by multiplying and then dividing the second term.

Code: Select all

uint DistanceOpenTTD(TileIndex t0, TileIndex t1)
{
    const uint dx = Delta(TileX(t0), TileX(t1));
    const uint dy = Delta(TileY(t0), TileY(t1));
    return ((3 * minu(dx, dy))  / 2) + Delta(dx, dy);
}
would be sufficient.
Ex TTDPatch Coder
Patch Pack, Github
Vetteman06
Engineer
Engineer
Posts: 59
Joined: 03 Aug 2013 16:39

Re: [patch] Cargo Payment Fix (r27460)

Post by Vetteman06 »

Ok....
I am having a difficult time understanding the difference...
So pardon my ignorance...

Let's say for an example that minu(dx, dy) = 3
And Delta(dx, dy) = 0

So Eddi's formula would end up being 3 * 1.5 + 0 = 4.5
Since it returns uint, this would return 4

JGR formula...
(3 * 3) / 2 + 0 = 4.5
Since it returns uint, would be 4

Zuu's formula...
(150 * 3 + 100 * 0) / 100 = 4.5
Since it returns uint, would be 4

So why would JGR formula be safer than Eddie's?
To me, these are all the same...
If anything Eddi's seems safer since no division takes place...
I would be amazed that other platforms would come up with a different answer than 4.5 for 3 * 1.5...
User avatar
JGR
Tycoon
Tycoon
Posts: 2558
Joined: 08 Aug 2005 13:46
Location: Ipswich

Re: [patch] Cargo Payment Fix (r27460)

Post by JGR »

There is no .5 in integer arithmetic.
3 / 2 produces 1, not 1.5.

Floating point arithmetic is not the same as integer arithmetic, have a look here.

You may well be able to get away with using floating point in this particular value range, but even then there's no point as you'd just be making your code slower and bigger for no benefit.

There is nothing inherently unsafe with integer division, and in any case integer division by a constant which is a power of a two is almost free.
Ex TTDPatch Coder
Patch Pack, Github
Aphid
Traffic Manager
Traffic Manager
Posts: 168
Joined: 16 Dec 2011 17:08

Re: [patch] Cargo Payment Fix (r27460)

Post by Aphid »

So our constraint is, at least for diagonal part: d <= 4096.

That means the largest usable convergent must have p ~< 800,000 if we want the exact diagonal distance part available as a variable in an int32.

Using sqrt(2) ~ p/q = 665857 / 470832, we can get:

D = min(dx, dy) * p.
Here D is about 470832 times the diagonal distance. Let cV be the cargo value, cA the cargo amount, then:
P = (cA * D * cV) / q+cA * cV * (max(dx,dy)-min(dx,dy))
where all numbers in this calculation are used as int64 to prevent overflow issues. Even better; just use 'Money' which is overflow-safe.

You could probably use even more accurate convergents than this one; I'll need some more information on what is:

1) The maximum packet size.
2) The maximum 'money' awarded per unit of cargo.

Note that some design space to expand these variables would be nice as well.
E.g. with max. packet size of 256, max money per cargo of 256, and max distance of 4096 (2^12) we have 2^28 so leaves 2^36 worth of space for the convergents. That makes the largest convergent that fits in 64 bits be p=63018038201,q=44560482149 (in the notation of my previous post). I'm also assuming inflation is applied 'afterwards'.

Btw, I strongly advise against using p=150,q=100. Instead, use a set of numbers from the table. E.g. 99/70 is a better approximation.

You shouldn't be working with the distance, but with the 'multiplied' distance.
In pseudocode it looks like this:

Code: Select all

int64 diagDist(int64 x) {
   return x * 63018038201LL;
}
int64 ottdPayment(int64 packetSize, int64 baseMoney, int64 dx, int64 dy) {
    int64 diag = min(dx,dy);
    int64 diagMoney = (diagDist(diag) * packetSize * baseMoney) / 44560482149LL;
    int64 straightMoney = (max(dx,dy) - diag) * packetSize * baseMoney;
    return correctForInflation(diagMoney + straightMoney);
}
It's really accurate :wink: .

Now my original patch doesn't use 44560482149, but instead uses a power-of-two approximator: 92682/65536. That's because dividing by 44560482149 is more expensive than bit-shifting some places to the right.
_dp_
Transport Coordinator
Transport Coordinator
Posts: 278
Joined: 18 Dec 2013 12:32

Re: [patch] Cargo Payment Fix (r27460)

Post by _dp_ »

Guess I'll revive this topic. Somehow everyone missed the point here - make diagonal and straight directions equally profitable. And for that you need to look in vehicle moving code and use the same metrics for payment calculation. And no, it's not Euclidean. It drills down to this function:

Code: Select all

	inline uint GetAdvanceDistance()
	{
		return (this->direction & 1) ? 192 : 256;
	}
Which basically means that tile side is considered to be 192 units and diag - 256 units (actually it's about virtual coords but whatever), that brings us directly to distance formula:

Code: Select all

256 * min(dx, dy) + 192 * |dx-dy|
That can be normalized to yield same payment as manhattan distance either on straight track

Code: Select all

4 * min(dx, dy) / 3 + |dx-dy|
or diagonal one:

Code: Select all

2 * min(dx, dy)  + 3 * |dx-dy| / 2
Straight track normalization decreases overall possible income which is terrible for any kind of competitive scores so diagonal is much more preferable.

Using this formula will make all directions equal for all vehicle types except rvs. For rv it's, on contrary, less "fair" as zig-zags are now less profitable that straight roads, but it's kind of logical imo. Anyway I think it's a much better position to be in that having all other vehicles broken.
User avatar
adf88
Chief Executive
Chief Executive
Posts: 644
Joined: 14 Jan 2008 15:51
Location: PL

Re: [patch] Cargo Payment Fix (r27460)

Post by adf88 »

You are missing one important thing. Not just RV's would be penalized but trains too.

It's because of the fact that building of diagonal rails is limited by lack of diagonal rail slopes, bridges, tunnels, stations, waypoints... You can't just lay a track through a half of the map, you have to build by small pieces. It's hard to avoid dense s-bands that would limit the speed of trains, usually you have to level the land a bit. You can't pass through hills and mountains, you have to bypass them. Junctions take more space and require more turns (imagine a diagonal cloverleaf). Diagonal rail tracks, even straight ones, cost you more (building and maintenance) - you need at least as much rail pieces as if you would place two orthogonal tracks between end points.

All these factors nicely compensate the theoretical extra 33% profit you can get from trains (actually less then 33% because diagonal tracks will never be straight). In the end it's wise to have both kind of tracks (which one depends on actual circumstances) and I like this variety. If we would change the algorithm, diagonal rails would die. Current algorithm is the best what we an have IMO.
:] don't worry, be happy and checkout my patches
xarick
Transport Coordinator
Transport Coordinator
Posts: 341
Joined: 26 Feb 2015 00:52

Re: [patch] Cargo Payment Fix (r27460)

Post by xarick »

Hi!

Have you considered artificially reducing the speed of aircraft, trains and ships by 1.5 when going on diagonal trajectories?

After some tests using openttd 1.6.1 and the travel time from the timetable, I was able to get to that value of 1.5. Say, if a train moves at 100 km/h on a straight axis-y line, on a diagonal line, that speed would be 100/1.5 =~67km/h to achieve the same travel time as the train moving at 100 km/h, given the same A to B distances.

I'd like to give more details about my tests, but I'm currently having issues with my rig unfortunately, and don't have access to it. I've also found that road vehicles need not apply any artificial penalty, but there is some inconsistency, a bug that needs to be fixed. Out of 6 test results, 1 yielded faster travel times than all the other 5, for no apparent reason.
Harninghall Transport, 31st Aug 2069.png
Harninghall Transport, 31st Aug 2069.png (120.87 KiB) Viewed 4052 times
EDIT: This screenshot shows buses traveling at a maximum speed of 10 km/h between cardinal points going A <-> B. The distance is always 222 tiles. Each bus represents 2 results, giving the travel times from A to B, then from B to A. This time I got 8 test results.

Note that I opted for a very low speed to expose inconsistencies in the travel times and to avoid the turn penalty altogther.

NW <-> SE and NE <-> SW show a consistent time of 622 days in either direction. These distances are in the axis-x and axis-y.
W <-> E shows a slight difference of 1 day, 609 and 610 days. If they were to achieve parity with the results above, they should have been 622 days. Regardless, 1 day isn't all that bad when going in opposite direction, and could be within the margin of error or when the bus is arriving at the station.
N <-> S shows a real discrepancy, 597 and 621 days. While I like to believe 621 is a good result within margin of error, coming very close to 622 days for time travel parity with the axis results, the 597 days is bugged. It doesn't make sense.

I ran my tests with Drive on Right and Realistic Acceleration.

Now what does this all mean? If the goal is to achieve the same profit no matter the coordinates, without touching the profit formula, and given that road vehicles do not have diagonal roads, then the travel time should be consistent in all cases if the manhattan distance from any 2 points on the map is the same if the vehicles happen to travel at the same speed. So, goal would be to have NW<->SE, NE<->SW, W<->E and N<->S have the same travel time on both directions. Hopefully my discrepancy findings are bugs that can be patched from whatever value they are to the same 622 days for this set of test result examples.
Formerly known as Samu
User avatar
prissi
Chief Executive
Chief Executive
Posts: 647
Joined: 15 Nov 2004 19:46
Location: Berlin, Germany
Contact:

Re: [patch] Cargo Payment Fix (r27460)

Post by prissi »

A speed reduction for trains unfortunately would mean than the diagonal images for trains needs to be shorter (by the same factor). Simutrans did this (but allows for uncorrected graphics to have the old behaviour), after we corrected the diagonal speed.
I like to look at great maps and see how things flow. A little like a finished model railway, but it is evolving and actually never finished. http://www.simutrans.com
xarick
Transport Coordinator
Transport Coordinator
Posts: 341
Joined: 26 Feb 2015 00:52

Re: [patch] Cargo Payment Fix (r27460)

Post by xarick »

Slow down the entire train, not each wagon
Formerly known as Samu
Post Reply

Return to “OpenTTD Development”

Who is online

Users browsing this forum: No registered users and 28 guests