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

start work on noise for chunk generation #90

Merged
merged 9 commits into from
Sep 14, 2024

Conversation

kralverde
Copy link
Contributor

@kralverde kralverde commented Sep 10, 2024

WIP

This PR aims to implement the noise and related methods used for chunk generation. The noise will generate the same values as the Java implementation given the same inputs.

@kralverde kralverde marked this pull request as ready for review September 13, 2024 17:15
@kralverde
Copy link
Contributor Author

Noises used in surface generation is done. Actual chunk population is more in-depth. I'll need to take a longer look at it all to see whats going on.

@Asurar0
Copy link
Contributor

Asurar0 commented Sep 13, 2024

I will review the rest. It shouldn't take more than an hour.

@Snowiiii
Copy link
Member

About the inline, I recommend to take a look at https://gist.github.com/kvark/f067ba974446f7c5ce5bd544fe370186#dont-use-inlinealways

@Asurar0
Copy link
Contributor

Asurar0 commented Sep 13, 2024

About the inline, I recommend to take a look at https://gist.github.com/kvark/f067ba974446f7c5ce5bd544fe370186#dont-use-inlinealways

Interesting resource. I assumed the inlining behavior was the same between Rust and other C compilers. Thank you for the sharing. I guess the inlining changes can be reverted if deemed necessary.

@Asurar0
Copy link
Contributor

Asurar0 commented Sep 13, 2024

Ok I may have been too optimistic, the review will take more than an hour.

Asurar0

This comment was marked as outdated.

Copy link
Contributor

@Asurar0 Asurar0 left a comment

Choose a reason for hiding this comment

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

Everything looks good. Just found two improvements.

pumpkin-core/src/random/mod.rs Outdated Show resolved Hide resolved
pumpkin-world/src/world_gen/noise/perlin.rs Show resolved Hide resolved
pumpkin-world/src/world_gen/noise/perlin.rs Outdated Show resolved Hide resolved
)
}

#[allow(dead_code)]
Copy link
Contributor

Choose a reason for hiding this comment

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

There is a large amount of #[allow(dead_code)] You would be better setting this at module level instead (#![allow(dead_code)])

Copy link
Contributor Author

Choose a reason for hiding this comment

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

how best do you recommend doing this?

Copy link
Contributor

Choose a reason for hiding this comment

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

I've opened suggestions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

should be good now?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes

@Snowiiii
Copy link
Member

Looks good. Thank you @kralverde ❤️

@Snowiiii Snowiiii merged commit 0da078a into Pumpkin-MC:master Sep 14, 2024
5 checks passed
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.

3 participants