search

Auburn / FastNoiseLite

Fast Portable Noise Library - C# C++ C Java HLSL GLSL JavaScript Rust Go
http://auburn.github.io/FastNoiseLite/
MIT License
2.79k stars 328 forks source link

[Rust] Added clone derive for `FastNoiseLite` #138

Closed DanielDK05 closed 6 months ago

DanielDK05 commented 6 months ago

"It ain't much, but it's honest work"

Added a clone derive, as cloning is necessary in some cases.

N.B: the other changes in the file came from cargo fmt

Auburn commented 6 months ago

@Keavon does this look good to you?

Keavon commented 6 months ago

Yep, this looks sensible. While we're at it, @DanielDK05 are there any other traits this ought to implement? I'm thinking probably not to Copy since it's a somewhat large struct (48 bytes) and the default move semantics would be preferable for performance reasons.

DanielDK05 commented 6 months ago

@Keavon Yeah I agree with not adding Copy. Debug might be useful at some point though. Maybe we could also do Serialize/Deserialize from serde, in case anyone would want to have noise configured in a file.

I also thought of Send & Sync for multithreading, but I haven't done much multithreading in rust, so I'm not totally sure how to safely implement that. I know it's often done simple like this:

unsafe impl Send for FastNoiseLite {}
unsafe impl Sync for FastNoiseLite {}

But the unsafe makes me nervous to implement it myself :D

Keavon commented 6 months ago

Maybe let's just go with Debug for now. If someone wants serde for this, they could bother adding it as an optional dependency in a future PR. Likewise, I'm unfamiliar with multithreading so it's probably best to leave that hornet nest for someone who needs it in a future PR.

DanielDK05 commented 6 months ago

Yeah okay, I'll look at Debug.

Keavon commented 6 months ago

Looks good. @Auburn should this also bump the Rust crate's minor version or will you do that in a commit to master after this PR is merged?

Auburn commented 6 months ago

This was also mentioned as a potential change, don't know if we want that but should wait on it before the version if so https://github.com/Auburn/FastNoiseLite/issues/137