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: Allow aqueduct to use foundation #11012

Closed
wants to merge 1 commit into from

Conversation

marcossouza1232
Copy link

@marcossouza1232 marcossouza1232 commented Jun 15, 2023

Motivation / Problem

fix #10995

Description

Removes aqueduct restriction, allowing automatic slope correction while constructing aqueducts.

Limitations

I don't know the side effects, because I don't know why they disabled this functionality for aqueducts.

@rubidium42 rubidium42 added the preview This PR is receiving preview builds label Jun 15, 2023
@DorpsGek DorpsGek temporarily deployed to preview-pr-11012 June 15, 2023 05:24 Inactive
Copy link
Member

@2TallTyler 2TallTyler left a comment

Choose a reason for hiding this comment

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

The commit checker is failing because you didn't follow the message format specified in the coding style.

It should be Change: Allow aqueduct to use foundation or similar and then note in the PR description that it closes #10995. It's not a fix because the original choice seems to be intentional.

Also, this does not terraform the end tile, it builds the aqueduct on a foundation. That is a change, since we don't allow flat canals to be built on foundations. A proper "fix" might be to terraform the end slope to allow building the aqueduct without a foundation.

@@ -408,7 +408,7 @@ CommandCost CmdBuildBridge(DoCommandFlag flags, TileIndex tile_end, TileIndex ti
} else {
/* Build a new bridge. */

bool allow_on_slopes = (_settings_game.construction.build_on_slopes && transport_type != TRANSPORT_WATER);
bool allow_on_slopes = (_settings_game.construction.build_on_slopes );
Copy link
Member

Choose a reason for hiding this comment

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

Extra space after conditional

@2TallTyler 2TallTyler added the work: needs more work This Pull Request needs more work before it can be accepted label Jun 27, 2023
@TrueBrain TrueBrain added preview This PR is receiving preview builds and removed preview This PR is receiving preview builds labels Jul 8, 2023
@TrueBrain TrueBrain added preview This PR is receiving preview builds and removed preview This PR is receiving preview builds labels Jul 8, 2023
@@ -408,7 +408,7 @@ CommandCost CmdBuildBridge(DoCommandFlag flags, TileIndex tile_end, TileIndex ti
} else {
/* Build a new bridge. */

bool allow_on_slopes = (_settings_game.construction.build_on_slopes && transport_type != TRANSPORT_WATER);
bool allow_on_slopes = (_settings_game.construction.build_on_slopes );
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
bool allow_on_slopes = (_settings_game.construction.build_on_slopes );
bool allow_on_slopes = (_settings_game.construction.build_on_slopes);

@TrueBrain TrueBrain changed the title fix #10995 Change: Allow aqueduct to use foundation Aug 18, 2023
@2TallTyler
Copy link
Member

This doesn't seem like the right approach, but if anyone feels differently feel free to comment, re-open, etc. I am closing this PR because it appears to be abandoned.

@2TallTyler 2TallTyler closed this Nov 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
preview This PR is receiving preview builds work: needs more work This Pull Request needs more work before it can be accepted
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Aqueduct construction on slopes inconsistent with other bridges and tunnels
6 participants