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

[WIP] Refactor noise function construction #199

Closed
wants to merge 1 commit into from

Conversation

Ralith
Copy link
Contributor

@Ralith Ralith commented Oct 7, 2018

This is a draft of the API refactoring proposed in #197, with usability tweaks as discussed in IRC. I'd like to get some feedback before proceeding further. Changes were applied to BasicMulti and Perlin as a hopefully-representative sample, with some redundant code left in place to keep the crate as a whole building until the proposed APIs are more widely implemented.

The most significant impact of this change is that fractal noise functions are constructed with all parameters supplied up front. This is necessary because no meaningful seed value can be saved for RNG-produced noise functions in the general case, and has the incidental benefits of reduced boilerplate and that redundant work is no longer performed when constructing a noise function with a non-default seed or octaves.

@Ralith
Copy link
Contributor Author

Ralith commented Oct 11, 2018

It looks like not all fractal noise functions take exactly the same parameters, so maybe having a common trait for them doesn't make sense. As far as I can tell, the only reason to define the constructors via traits at all is to avoid a small amount of boilerplate, so perhaps the way forwards is to remove Random and RandomFractal and just give everything the corresponding constructors directly.

@Ralith
Copy link
Contributor Author

Ralith commented Jul 8, 2020

Closing as, in the absence of any feedback, I cannot move this forward.

@Ralith Ralith closed this Jul 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants