Skip to content

Conversation

@akx
Copy link
Contributor

@akx akx commented May 16, 2025

First pass at some improvements to terrain loading:

  • A better constructor for Ground – not one function that does two things, but a flat constructor and a loads-data-from-Mapbox one.
  • Actual handling for Mapbox API errors (just panics for now, but better than trying to load an API error as an image)
  • Refactoring of the Mapbox elevation data out of the Ground impl
  • Removal of the hard-coded Mapbox access token (that just 401'd for me anyway).
    • If the access token was meant to be there (and I just happened to try it while the free quota was done with), it can be defaulted back in.
  • Local caching of Mapbox tiles (faster once you've loaded them once, and no more token limits)
  • For debugging, printing out the terrain heights (both from the raw data and the generated Minecraft blocks).
    • -> I think there's something wrong in the scaling math (that I didn't touch yet):

      Height data range: -0.299 to 64.64976453029676 m
      Minecraft height data range: -62 to -15 blocks

    • The generated map with this data is just flat :(

@akx akx force-pushed the terrain-fixes branch from c3010eb to 0572d8d Compare May 16, 2025 11:10
@louis-e
Copy link
Owner

louis-e commented May 18, 2025

Removal of the hard-coded Mapbox access token (that just 401'd for me anyway).
If the access token was meant to be there (and I just happened to try it while the free quota was done with), it can be defaulted back in.

Yup, my old token seems to have gotten revoked suddenly - I fixed that with a quick hotfix today.
Just wanted to point that out real quick, I'm still feeling a bit sick but I'll do a code review asap! :)

@akx
Copy link
Contributor Author

akx commented May 18, 2025

@louis-e Get well soon! I can rebase this on top of the new stuff tomorrow or so, and make the CLI default to the hard-coded default token 👍

@akx akx force-pushed the terrain-fixes branch from 0572d8d to 6a94435 Compare May 19, 2025 05:26
@github-actions
Copy link

github-actions bot commented May 23, 2025

⏱️ Benchmark run finished in 0m 43s
🧠 Peak memory usage: 1614 MB

📈 Compared against baseline: 42s
🧮 Delta: 1s
🔢 Commit: e95a6ff

🟢 Generation time is unchanged.

You can retrigger the benchmark by commenting retrigger-benchmark.

@louis-e
Copy link
Owner

louis-e commented Jun 3, 2025

Hi there, thanks a lot for the great contribution! Especially the tile caching is amazing. I was already thinking about doing that via a proxy server, but turns out that is not allowed according to the Mapbox guidelines. So client side caching is a good idea.

I have to test it more thoroughly but one thing I saw is that you missed out returning the ground in pub fn generate_ground_data(args: &Args) -> Ground {
image

That might explain this :)

The generated map with this data is just flat :(

@louis-e
Copy link
Owner

louis-e commented Jun 22, 2025

retrigger-benchmark

@louis-e
Copy link
Owner

louis-e commented Jun 22, 2025

Sorry for the delay, I was on a business trip and got sicker afterwards - I pushed a fix for the last error I mentioned and will merge the PR now! :)

@louis-e louis-e merged commit 5cbcd68 into louis-e:main Jun 22, 2025
4 checks passed
@akx
Copy link
Contributor Author

akx commented Jun 23, 2025

Good catch on the return! I, too, was AFK for about 3 weeks (in Japan 🗻). Thanks!

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