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

Fix #7626: Allow building of drive-through stops over one-way/blocked roads owned by towns (instead of crashing). #7627

Merged
merged 1 commit into from Sep 6, 2019

Conversation

ddm999
Copy link
Contributor

@ddm999 ddm999 commented Jun 26, 2019

Originally I thought about showing an error and preventing the stop from being built to prevent people from destroying a town's one-way system, but you can do that regardless by just removing the roads, so I don't think it really matters.
Makes more sense to just allow the stop to be placed.

@stormcone
Copy link
Contributor

Personally I better like @JGRennison's fix (JGRennison/OpenTTD-patches@2164eb4) in #7626. That is shorter and using the already available part of the CheckOwnership function.

@nielsmh
Copy link
Contributor

nielsmh commented Jul 13, 2019

Agree with @stormcone, this fix looks weird.

@ddm999
Copy link
Contributor Author

ddm999 commented Jul 15, 2019

That fix prevents the building of drive-through station on one-way road, mine instead allows it.

With that fix, you can still just destroy the one-way road, then build a drive-through station in it's place.
This is why I decided on allowing the station to be built instead: by making the check for one-way road also check if it is town-owned.

Also with that fix, the error shown when attempting to place a station is the generic CheckOwnership one:

Can't build x ... owned by y (closest town name, in this instance)

which I don't think really makes sense to the player (doesn't specify the reason being that it's a one-way road).

@nielsmh
Copy link
Contributor

nielsmh commented Sep 6, 2019

Okay this does look correct on a second check.

At first it looked like this fix would prevent you from building stops over your own one-way roads, but all it really does is circumvent the assert(owner != OWNER_TOWN) check in CheckOwnership(). The entire purpose CheckOwnership() is to confirm whether the current company is the owner and otherwise generate a relevant error, with the error generation really being its raison d'être.

@nielsmh nielsmh merged commit 2d9eb1c into OpenTTD:master Sep 6, 2019
@nielsmh nielsmh added the backport requested This PR should be backport to current release (RC / stable) label Sep 6, 2019
@nielsmh
Copy link
Contributor

nielsmh commented Sep 6, 2019

Also, the command has previously established that if the road was town owned, the player is allowed to build a road stop over it, so this is definitely the correct behaviour. It would be strange if you're allowed to build road stops over town roads, unless it's a one-way town road then you're not.

@LordAro LordAro added backported This PR is backported to a current release (RC / stable) and removed backport requested This PR should be backport to current release (RC / stable) labels Sep 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backported This PR is backported to a current release (RC / stable)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants