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

Wide rivers #76

Merged
merged 7 commits into from
Oct 19, 2024
Merged

Wide rivers #76

merged 7 commits into from
Oct 19, 2024

Conversation

scd31
Copy link

@scd31 scd31 commented Oct 19, 2024

Working on adding rivers. Code works as-is, but it's very slow. Takes about 40 minutes to run on my small city. Have some improvements locally but I haven't pushed them up yet.

This required a bit of refactoring so that we could handle relations. I cleaned up some related code at the same time. And this is also based on my other PRs, so I recommend looking at them first (as some of their code is duplicated in this PR)

Before:

freddy_before

After:

freddy_after

This makes a big difference to how my city looks!

@louis-e louis-e marked this pull request as ready for review October 19, 2024 17:52
@louis-e
Copy link
Owner

louis-e commented Oct 19, 2024

I love your inversed floodfill approach.
Awesome work, thanks for your time and effort! :)

@louis-e louis-e merged commit 14c6499 into louis-e:main Oct 19, 2024
3 checks passed
@scd31
Copy link
Author

scd31 commented Oct 20, 2024

@louis-e Sorry, I was out for a wedding late last night and only just got up. I think we should to revert this PR and #75 as they were both draft PRs. This one I'm working on optimizing more (and tbh, it should probably be tested a bit more as I refactored a bunch of code) and the other one I left some questions on in the PR.

@scd31
Copy link
Author

scd31 commented Oct 20, 2024

Actually, we'd probably okay to not revert (I can put up new PRs to finish things) but #75 has a few questions I was wondering about

@scd31
Copy link
Author

scd31 commented Oct 21, 2024

Sorry for all the spam, but my open PRs should fix all the WIP stuff! Can probably ignore my above comments

@scd31 scd31 deleted the wide_rivers branch October 21, 2024 02:34
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.

2 participants