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

Improved logic of sharing industry production between stations #7922

Merged
merged 1 commit into from Jan 12, 2020

Conversation

@ldpl
Copy link
Contributor

ldpl commented Jan 9, 2020

Since the dawn of time Industries in TTD only share cargo between two stations max. This behavior doesn't seem to be based on any logic and leads to some issues in mp as it can be easily abused.

So this patch:

  1. Removes the limit of 2 stations.
  2. Shares cargo between companies first and then between stations of each company separately to prevent exploiting it by spaming station.

Shares are still based on station rating like it was before and, in fact, this patch doesn't change behaviour at all if there are less than 3 stations around (except for negligible rounding losses).

I didn't add a new setting for this change as IMO it only fixes exploits/limitations and doesn't change the gameplay logic. So I can't imagine why would anyone ever want to opt-out of it.

@ldpl
Copy link
Contributor Author

ldpl commented Jan 9, 2020

Hm, apparently there was PR #7184 that got completely derailed and rejected. So, just to emphasize some points.

  1. This PR doesn't change the way cargo is delivered from stations to industries which is indeed questionable.
  2. This implementation has much better performance (though I haven't seen any evidence of this function being particularly time critical).
  3. It doesn't try to invent a new way to determine share sizes and just replicates original logic of using station ratings even if it somewhat sucks.

It only solves one specific quirk of game mechanics that so far is only known to confuse people and/or being abused. For example, currently there is no such thing as competition between three companies on one industry. Third station won't even get any cargo unless it uses another exploit to start acceptance.

src/station_cmd.cpp Show resolved Hide resolved
src/station_cmd.cpp Outdated Show resolved Hide resolved
src/station_cmd.cpp Outdated Show resolved Hide resolved
@ldpl ldpl force-pushed the ldpl:improved-goods-to-station branch 4 times, most recently from eddf593 to 59989d5 Jan 10, 2020
@ldpl
Copy link
Contributor Author

ldpl commented Jan 10, 2020

After a discussion in IRC I changed implementation so that it takes cargo bits that would've been lost previously to rounding issues and distributes them among the best rated stations, similarly to what original implementation did.

Also I left CanMoveGoodsToStation as a separate function for better readability even though I don't need to call it twice anymore.

Notably it works about as fast as original implementation in the most common cases of 0 or 1 stations. For the other cases it's about 6 times slower but that seems to be ok as they are quite rare when playing normally and it's probably going to waste even more time in StationFinder anyway.

P.S. Can also be similarly optimized for 2 stations I just didn't see any particular reason to complicate it even more.

Copy link
Member

LordAro left a comment

Nothing too major

src/station_cmd.cpp Show resolved Hide resolved
src/station_cmd.cpp Outdated Show resolved Hide resolved
src/station_cmd.cpp Outdated Show resolved Hide resolved
src/station_cmd.cpp Outdated Show resolved Hide resolved
src/station_cmd.cpp Outdated Show resolved Hide resolved
@ldpl ldpl force-pushed the ldpl:improved-goods-to-station branch from 59989d5 to 1e7307e Jan 10, 2020
@ldpl
Copy link
Contributor Author

ldpl commented Jan 10, 2020

FWIW here is the simple code I'm using to roughly check performance.
move-goods-to-station.zip

@LordAro LordAro merged commit 1225693 into OpenTTD:master Jan 12, 2020
8 checks passed
8 checks passed
OpenTTD CI Build #20200110.6 succeeded
Details
OpenTTD CI (Linux commit-checker) Linux commit-checker succeeded
Details
OpenTTD CI (Linux linux-amd64-clang-3.9) Linux linux-amd64-clang-3.9 succeeded
Details
OpenTTD CI (Linux linux-amd64-gcc-6) Linux linux-amd64-gcc-6 succeeded
Details
OpenTTD CI (Linux linux-i386-gcc-6) Linux linux-i386-gcc-6 succeeded
Details
OpenTTD CI (MacOS) MacOS succeeded
Details
OpenTTD CI (Windows Win32) Windows Win32 succeeded
Details
OpenTTD CI (Windows Win64) Windows Win64 succeeded
Details
@ldpl ldpl deleted the ldpl:improved-goods-to-station branch Jan 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.