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 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 improved-goods-to-station branch 4 times, most recently from eddf593 to 59989d5 Compare January 10, 2020 20:02
@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 LordAro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
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
@ldpl ldpl deleted the improved-goods-to-station branch January 12, 2020 20:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants