Skip to content

Make FNL::new and FNL::with_seed const - #139

Open
Kyllingene wants to merge 1 commit into
Auburn:masterfrom
Kyllingene:master
Open

Make FNL::new and FNL::with_seed const#139
Kyllingene wants to merge 1 commit into
Auburn:masterfrom
Kyllingene:master

Conversation

@Kyllingene

Copy link
Copy Markdown

Makes the two constructors const. Nothing else can be constified because they're setters (which require &mut): the two fields that this is a limitation for are transform_type_3d and fractal_bounding, as they are private. Others require float ops, and are thus also barred (such as get_noise_2d).

If/when constant float ops are stabilized, this might warrant being revisited.

@Kyllingene

Copy link
Copy Markdown
Author

Addresses #137.

@Auburn

Auburn commented Feb 9, 2026

Copy link
Copy Markdown
Owner

I'm looking to take fixes for a version bump, it seems this got forgetten about. If you get a moment could you take a look at this @Keavon
Thanks

@Keavon

Keavon commented Feb 9, 2026

Copy link
Copy Markdown
Contributor

If/when constant float ops are stabilized, this might warrant being revisited.

@Kyllingene could you please check if that's still not stabilized?

Comment thread Rust/src/lib.rs
ping_pong_strength: 2.,

/* private */ fractal_bounding: 1. / 1.75,
/* private */ fractal_bounding: 0.5714285714, // = 1.0 / 1.75

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this change necessary? I think it might have been required in the past but now floating point math is allowed in const. I changed this locally and did a cargo check. Although I wonder if we'd need an MSRV declaration, which would make it a breaking change?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I believe you're correct, certain float ops have since been stabilized in const. I would personally consider it something of a breaking change, so you'd have to decide how to handle that.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I just checked that the float ops do now work on stable. Are you concerned about the bump of the MSRV as a breaking change? If so we could explicitly mention the minimum rust version needed as part of the package description so people who are still on older rust version could just the previous version

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@Kyllingene any updates regarding this?

@Keavon

Keavon commented Feb 11, 2026

Copy link
Copy Markdown
Contributor

@Auburn what kind of version bump (in x.y.z, which letter?) are you planning?

@Auburn

Auburn commented Feb 11, 2026

Copy link
Copy Markdown
Owner

Likely y bump, mainly because I want to fix #172 which will change outputs slightly

@Keavon

Keavon commented Feb 11, 2026

Copy link
Copy Markdown
Contributor

In that case, if it's already a breaking change to the behavior, it should probably be fine to raise the minimum supported Rust version. @Kyllingene could you please push a revert to the manual math calculation?

@Kyllingene

Copy link
Copy Markdown
Author

I can, though my setup has since gone away. It might be a couple days before I get the time to get around to it, sorry.

1 similar comment
@Kyllingene

Copy link
Copy Markdown
Author

I can, though my setup has since gone away. It might be a couple days before I get the time to get around to it, sorry.

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.

4 participants