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

Feature: Generate lock ready rivers upon world generation #7073

Closed

Conversation

@SamuXarick
Copy link
Contributor

SamuXarick commented Jan 19, 2019

This patch is intended to ease water construction, especially locks on rivers. The changes might be too subtle to notice, but I think they're helpful if you're familiar to the constraints of water-based transportation inland.

With this patch applied, and while on map generation, rivers are generated in a way that facilitate construction of locks on river rapids. It assures that the river source can be connected to its mouth, without the need to terraform land for locks. The player only needs to place locks on rapid tiles to connect each lock to the rest of the stream. In this manner, as long as ships can make 90 degree turns, ships can traverse the entire river.

Other features:
- Towns will not build bridges over rivers if a lock could be built under it, while growing during a game and/or during map generation.
Posted as separate: #7195

https://www.tt-forums.net/viewtopic.php?f=33&t=76075

@SamuXarick

This comment has been minimized.

Copy link
Contributor Author

SamuXarick commented Jan 20, 2019

screenshot 87

1 / 2 - A town bridge isn't generated in 1 because 2 is suitable for a lock.
3 / 4 - River tiles 4 are generated to connect to a suitable lock at 3.
5 / 6 - Same as 1 / 2

Copy link
Contributor

andythenorth left a comment

I have tested this. I really want easier lock placement on rivers, and this is one possible solution.

  • I can't comment on the code style
  • the change to town bridge behaviour should be split from this PR, it combines too many concerns into one PR
  • the patch frequently produces isolated rivers that can't flow to the sea, I suspect this is because it bans rivers from using contiguous slopes. I'll attach screenshots. This will need fixed.
  • the approach of adding extra tiles in corners near a slope is 'interesting', but works much better than might be expected, the resulting rivers are more organic looking

Screenshots of isolated rivers with no path to sea. These happen occasionally in master, but are very much more frequent with this patch.
unnamed 01-01-2000 7
unnamed 01-01-2000 6
unnamed 01-01-2000 5
unnamed 01-01-2000 4

src/landscape.cpp Outdated Show resolved Hide resolved
src/landscape.cpp Outdated Show resolved Hide resolved
src/landscape.cpp Outdated Show resolved Hide resolved
src/landscape.cpp Outdated Show resolved Hide resolved
}
}

TileIndex t_2dm = t_dm + m * delta_mid;

This comment has been minimized.

Copy link
@michicc

michicc Jan 20, 2019

Member

Variable name.

This comment has been minimized.

Copy link
@PeterN

PeterN Mar 1, 2019

Member

^^ Still

This comment has been minimized.

Copy link
@SamuXarick

SamuXarick Mar 1, 2019

Author Contributor

I don't know what to do about that.

This comment has been minimized.

Copy link
@SamuXarick

SamuXarick Mar 25, 2019

Author Contributor

image

This comment has been minimized.

Copy link
@SamuXarick

SamuXarick Mar 25, 2019

Author Contributor

tile: main tile
t_dm: tile + delta mid
t_dm_ds: tile + delta mid + delta side
t_dm_2ds: tile + delta mid + two times delta side
t_2dm: tile + two times delta mid
t_2dm_ds: tile + two times delta mid + delta side
t_3dm: tile + three times delta mid
t_3dm_ds: tile + three times delta mid + delta side

This comment has been minimized.

Copy link
@SamuXarick

SamuXarick Mar 26, 2019

Author Contributor

Just removed all those variables, and only kept one which is reused multiple times to compute the offsets. I called it tile_offset.

int delta = TileOffsByDiagDir(dir_rot);

for (int m = -1; m <= 1; m += 2) {
TileIndex t_dm = tile + m * delta_mid;

This comment has been minimized.

Copy link
@michicc

michicc Jan 20, 2019

Member

Variable name.

src/landscape.cpp Outdated Show resolved Hide resolved
src/town_cmd.cpp Outdated Show resolved Hide resolved
@nielsmh

This comment has been minimized.

Copy link
Contributor

nielsmh commented Jan 20, 2019

Agree with @andythenorth it's nice to see some "lakes" being generated as part of rivers.

From what I can tell, right now the code seems to reject certain river flows if there isn't a path that allows it to be lock-ready. I wonder if a better approach would be to let the river flow downgrade regardless, but best-effort place additional river tiles at slopes to permit locks. Or maybe the current approach where it tries to reject rivers that aren't lock-ready, but gives up after a while and just uses the old (simple) approach so the river can flow down.

I hope this makes sense.

@andythenorth

This comment has been minimized.

Copy link
Contributor

andythenorth commented Jan 20, 2019

I am +1 that the river should always ultimately be allowed to flow to the sea.

It is useful however to prefer 'lockable' slopes where possible. If none are available, allowing flow downslope is better than rejecting.

@SamuXarick

This comment has been minimized.

Copy link
Contributor Author

SamuXarick commented Jan 20, 2019

fanley transport 1952-02-29

Variable names inside the now renamed to IsPossibleLockLocationRecursively

@SamuXarick SamuXarick force-pushed the SamuXarick:generate-lock-friendly-rivers branch 2 times, most recently from b60e0d8 to db0d66c Jan 22, 2019
@SamuXarick SamuXarick force-pushed the SamuXarick:generate-lock-friendly-rivers branch 2 times, most recently from 1a040d5 to 3af1f0d Feb 8, 2019
@SamuXarick

This comment has been minimized.

Copy link
Contributor Author

SamuXarick commented Feb 8, 2019

Changes:

  • Don't find a spring near other rivers in a radius of 8 tiles.
  • Only try to generate lakes if the river has already flowed down at least twice.
@SamuXarick SamuXarick force-pushed the SamuXarick:generate-lock-friendly-rivers branch from 3af1f0d to bee74c5 Mar 1, 2019
@SamuXarick SamuXarick force-pushed the SamuXarick:generate-lock-friendly-rivers branch 2 times, most recently from a200bce to 9677ffe Mar 26, 2019
@SamuXarick SamuXarick force-pushed the SamuXarick:generate-lock-friendly-rivers branch from 9677ffe to 8c8035f Apr 13, 2019
@stale

This comment has been minimized.

Copy link

stale bot commented May 13, 2019

This pull request has been automatically marked as stale because it has not had any activity in the last month.
Please feel free to give a status update now, ping for review, or re-open when it's ready.
It will be closed if no further activity occurs within 7 days.
Thank you for your contributions.

@stale stale bot added the stale label May 13, 2019
@andythenorth

This comment has been minimized.

Copy link
Contributor

andythenorth commented Jun 29, 2019

Stale. Closing, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.