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

Change: Ability to convert between road types for town roads and bridges now indi… #8457

Merged

Conversation

@UnsuspiciousGooball
Copy link
Contributor

@UnsuspiciousGooball UnsuspiciousGooball commented Dec 28, 2020

…rectly depends on town ratings.

Motivation / Problem

Previously it was possible to convert town roads and bridges without restriction by the local authority.

Description

With this change the ability to convert town roads and bridges depends on the players' companies' standing with the local authority, which is determined using CheckforTownRating() with the ROAD_REMOVE and the TUNNELBRIDGE_REMOVE TownRatingCheckTypes, respectively.

Basically the player is allowed to convert town roads if and only if the player would be allowed to destroy town roads, and the same goes for town bridges.

…rectly depends on town ratings.
@LordAro
Copy link
Member

@LordAro LordAro commented Dec 28, 2020

Not that we particularly mind, but the commit author does not appear to match your GitHub user. Might want to fix that

Copy link
Member

@LordAro LordAro left a comment

This appears to do exactly the same thing at the top of both if/else branches.
Instead of duplicating quite so much, I think a better solution would be to pull it out of the loop, and just use a ternary (tt == MP_TUNNELBRIDGE ? TUNNELBRIDGE_REMOVE : ROAD_REMOVE) for the only bit that actually differs. Would be able to get rid of the is_owner_town variable too.

…idge conversion to a single if statement.

Also, ClosestTownFromTile() is now only called when the tile in question actually belongs to a town.
@UnsuspiciousGooball UnsuspiciousGooball requested a review from LordAro Dec 28, 2020
@LordAro
LordAro approved these changes Jan 2, 2021
Copy link
Member

@LordAro LordAro left a comment

LGTM

@TrueBrain TrueBrain merged commit c017a36 into OpenTTD:master Jan 5, 2021
8 checks passed
8 checks passed
Emscripten
Details
Commit checker
Details
Check preview needs update Check preview needs update
Details
Linux (clang, clang++)
Details
Linux (gcc, g++)
Details
Mac OS (x64, x86_64) Mac OS (x64, x86_64)
Details
Windows (x86) Windows (x86)
Details
Windows (x64) Windows (x64)
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants