Skip to content

Set terrain arg to store true. #374

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

Merged
merged 2 commits into from
Apr 12, 2025
Merged

Conversation

benjamin051000
Copy link
Contributor

This was erroneously set to store false, which means it's impossible to enable the terrain from the cli.

@benjamin051000
Copy link
Contributor Author

Hm, this is failing with the following:

cargo r --no-default-features --release -- --bbox "-71.035194,42.327427,-71.008286,42.342210" --path "/home/benjamin/.var/app/org.prismlauncher.PrismLauncher/data/PrismLauncher/instances/Fabulously Optimized(3)/.minecraft/saves/testing" --terrain

(with a dbg! around the first line of ground::fetch_elevation_data) Prints:

[src/ground.rs:130:32] bbox_str.split_whitespace().map(|s: &str|
s.parse::<f64>()).collect::<Result<Vec<f64>, _>>() = Err(
    ParseFloatError {
        kind: Invalid,
    },
)
Warning: Failed to fetch elevation data: invalid float literal

@benjamin051000
Copy link
Contributor Author

I'll add a unit test that verifies this before merging.

@benjamin051000 benjamin051000 force-pushed the fix-terrain-flag branch 11 times, most recently from 5bdea24 to afdfc0a Compare February 22, 2025 04:09
@benjamin051000
Copy link
Contributor Author

benjamin051000 commented Feb 22, 2025

Blocked on #378

This was erroneously set to `SetFalse`, which means it's impossible to
enable the terrain from the cli.

According to clap docs, the default action for a bool arg is SetTrue,
and the default value is false. (https://docs.rs/clap/latest/clap/_derive/index.html#arg-types)

So we don't need to provide any additional args here.
@benjamin051000
Copy link
Contributor Author

@louis-e once CI passes, this is ready for signoff

@louis-e
Copy link
Owner

louis-e commented Apr 12, 2025

So I guess we're starting with unit tests now ;)
Thanks a lot :)

@louis-e louis-e merged commit 2a1233c into louis-e:main Apr 12, 2025
3 checks passed
@benjamin051000 benjamin051000 deleted the fix-terrain-flag branch April 12, 2025 22:51
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