Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: Distribute cargo to multiple stations or industries #7184

Conversation

@SamuXarick
Copy link
Contributor

SamuXarick commented Feb 5, 2019

Changes the way cargo amounts appear on multiple stations, and the way cargo is received by multiple industries accepting that same cargo type.

For stations, instead of spliting the load between the two highest rating stations for that cargo, it splits to all nearby stations, while still considering station rating from highest to lowest. The higher the rating, the more it receives.
uyzrvzz

For industries, instead of the first nearby industry accepting the cargo, the amount is distributed piece by piece, at random, to any accepting industries within the station catchment area.
7wjqrjg

https://www.tt-forums.net/viewtopic.php?p=1193758

@SamuXarick SamuXarick force-pushed the SamuXarick:distribute-cargo-to-multiple-stations-or-industries branch from 11a1fe4 to 962cb05 Feb 5, 2019
@andythenorth

This comment has been minimized.

Copy link
Contributor

andythenorth commented Feb 6, 2019

This would be a very helpful feature IMO.

For distribution, round-robin would be preferable. Random distribution should come out even statistically over large datasets, but might cause unexpected localised behaviour, which will cause bug reports and be hard to debug. Round-robin is easier to understand.
https://en.wikipedia.org/wiki/Round-robin_scheduling

Also are you aware that the newgrf spec supports industries stopping and starting cargo acceptance, both for the industry, and for industry tiles? This creates some tricky cases. This can be tested with Pikka's Basic Industries grf, which limits acceptance at secondary industries.

@PeterN

This comment has been minimized.

Copy link
Member

PeterN commented Feb 6, 2019

This seems like a reasonable idea. This seems like a fundamental economy change, so I wonder if it warrants a configuration option? Don't add this now, I just wonder how others feel.

num_pieces -= amount;
accepted += amount;
}
} while (num_pieces != 0 && rejected.Length() != st->industries_near.Length());

This comment has been minimized.

Copy link
@PeterN

PeterN Feb 6, 2019

Member

This could still be a for () loop, in which case the massive inner if () {} block wouldn't be necessary.

If you keep it as a do {} while () loop, then if (!st->industries_near.Length()) return 0; before the loop would be better.

@SamuXarick SamuXarick force-pushed the SamuXarick:distribute-cargo-to-multiple-stations-or-industries branch from 962cb05 to f492e4f Feb 6, 2019
@andythenorth

This comment has been minimized.

Copy link
Contributor

andythenorth commented Feb 6, 2019

This seems like a fundamental economy change, so I wonder if it warrants a configuration option?

Presumably it changes the behaviour of existing savegames significantly, so maybe we have to preserve the old behaviour at minimum?

src/economy.cpp Outdated Show resolved Hide resolved
@SamuXarick SamuXarick force-pushed the SamuXarick:distribute-cargo-to-multiple-stations-or-industries branch from f492e4f to 58b9330 Feb 6, 2019
src/economy.cpp Show resolved Hide resolved
@SamuXarick SamuXarick force-pushed the SamuXarick:distribute-cargo-to-multiple-stations-or-industries branch from 58b9330 to 3dbd99d Feb 7, 2019
@SamuXarick SamuXarick force-pushed the SamuXarick:distribute-cargo-to-multiple-stations-or-industries branch from 3dbd99d to ad07b98 Mar 1, 2019
@TrueBrain

This comment has been minimized.

Copy link
Member

TrueBrain commented Mar 3, 2019

Pretty sure this is not a "Change". Possibly even a "Feature? :)

@SamuXarick SamuXarick force-pushed the SamuXarick:distribute-cargo-to-multiple-stations-or-industries branch from ad07b98 to 0003027 Mar 4, 2019
@SamuXarick SamuXarick changed the title Change: Distribute cargo to multiple stations or industries Feature: Distribute cargo to multiple stations or industries Mar 4, 2019
@nielsmh

This comment has been minimized.

Copy link
Contributor

nielsmh commented Mar 8, 2019

I think this risks making competition in multiplayer games more bland.

Consider for example the situation where three companies each have a station by the same industry. The stations have ratings of 80%, 78%, and 76% respectively. With this patch, all three companies will get almost equal amounts, giving much very little incitement to the 76% company to try to do better.
By the original rules, the 76% company would be forced to make a much better service to even get any cargo at all, since only the top two stations get any cargo.

Introduce a fourth station with rating 38% (half of 76%) and it would receive half the cargo of the 76% station, despite being much, much worse.

One solution to this may be to use the ratio of the square of the ratings (each rating multiplied by itself), instead of the ratings straight.

Rating 38% 76% 78% 80%
Ratio of ratings 14% 28% 29% 29%
Ratio of squared ratings 7% 29% 31% 32%

You can make the distribution difference even more extreme by using higher powers than 2.

Another option to possibly add in the mix is to take e.g. the bottom third of all stations out of the running. Or stations whose rating is less than two thirds of the best rating.

@SamuXarick

This comment has been minimized.

Copy link
Contributor Author

SamuXarick commented Mar 9, 2019

@nielsmh Your approach also changes the amounts for when there's only 2 stations. My approach still does the same as master if there's only 2 stations.

Tests:

      squared rating   |  rating sum
      -----------------+------------
      29%, 13 ( 9.77%) | 17 (12.88%)
      29%, 13 ( 9.77%) | 17 (12.88%)
      30%, 14 (10.53%) | 18 (13.64%)
      33%, 17 (12.78%) | 20 (15.16%)
      48%, 35 (26.32%) | 29 (21.97%)
      52%, 41 (30.83%) | 31 (23.48%)
@SamuXarick SamuXarick force-pushed the SamuXarick:distribute-cargo-to-multiple-stations-or-industries branch from faca5c5 to adb7b36 Mar 9, 2019
@SamuXarick

This comment has been minimized.

Copy link
Contributor Author

SamuXarick commented Mar 9, 2019

More tests:

3 stations: https://paste.openttdcoop.org/po46thby7

         47% rating / 72% rating / 83% rating
Samu:    642 (23%)  /  991 (36%) / 1136 (41%)
nielsm:  426 (15%)  / 1012 (37%) / 1331 (48%)
Master:    0 ( 0%)  / 1290 (47%) / 1479 (53%)

2 stations: https://paste.openttdcoop.org/pnp04lwfn

         33% rating / 83% rating
Samu:    664 (28%)  / 1679 (72%)
nielsm:  317 (14%)  / 2026 (86%)
Master:  664 (28%)  / 1679 (72%)
@SamuXarick SamuXarick force-pushed the SamuXarick:distribute-cargo-to-multiple-stations-or-industries branch from adb7b36 to 6e4b599 Mar 10, 2019
Changes the way cargo amounts appear on multiple stations, and the way cargo is received by multiple industries accepting that same cargo type.

For stations, instead of spliting the load between the two highest rating stations for that cargo, it splits to all nearby stations, while still considering station rating from highest to lowest. The higher the rating, the more it receives.

For industries, instead of the first nearby industry accepting the cargo, the amount is distributed piece by piece, at random, to any accepting industries within the station catchment area.
@SamuXarick SamuXarick force-pushed the SamuXarick:distribute-cargo-to-multiple-stations-or-industries branch from 6e4b599 to ddb5827 Mar 10, 2019
@SamuXarick SamuXarick force-pushed the SamuXarick:distribute-cargo-to-multiple-stations-or-industries branch from ddb5827 to e463a94 Mar 10, 2019
@SamuXarick

This comment has been minimized.

Copy link
Contributor Author

SamuXarick commented Mar 10, 2019

I have a dilema:

<Samu> why include an industry to the list if it receives 0?
<planetmaker> it might get something later?
<peter1138> Samu, prove to me that it can receive 0 at that point.
<Samu> incoming_cargo_waiting could be a stockpile
<nielsm> peter1138, I think it can reach that line even if ind->incoming_cargo_waiting[cargo_index] == 0xFFFF
<nielsm> so the second arg to min would become zero
<peter1138> Okay.
<nielsm> question is, should be industry count as having received a delivery (of zero) if its stockpile is full?
<peter1138> And the problem is it will trigger production when it should not have done.
<peter1138> nielsm, probably not.
<peter1138> Samu, i guess this is just a case of explaining the problem clearly 

This PR changed it to not count as having received a delivery of zero. Otherwise, it would be added to both rejected and _cargo_delivery_destinations lists, which wouldn't look right.

@TrueBrain

This comment has been minimized.

Copy link
Member

TrueBrain commented Apr 1, 2019

Without this being an option, I do not see how this is going to work. And if we do this, we should allow control via either GS or NewGRF, instead of hardcoding yet-another-way-of-doing-this.

So I do not think this is the correct solution for the problem. Just changing the current way of doing because it flavours you only means others won't like it, and we will have PRs ping-ponging solutions all day. We need a more constructive solution to this problem.

Tnx for the contribution though!

@TrueBrain TrueBrain closed this Apr 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.