FastNoiseLite: Fix cellular jitter using incorrect default value

[Auburn]

Hi I'm the creator of the FastNoiseLite library, I was looking through the godot usage of it and I noticed this error. It seems to be a relic from the original godot PR for FastNoise which was originally based on FastNoise legacy where the default was 0.45

Default value for cellular jitter should be 1.0, using 0.45 will make the cellular noise have an overly grid shaped appearance

Cellular Jitter = 0.45

Cellular Jitter = 1.0

The other issue I see is that the OpenSimplex noise type is just named Simplex, which is a disservice to @KdotJPG who created the open license OpenSimplex algorithms. This is obviously a harder thing to fix though because it would cause breaking changes to existing code

[YuriSizov]

Thanks for opening a PR. You need to update the documentation as well. Since it's just a change in the default value, you can simply run the doctool with a binary that you've compiled from your changes. See the documentation.

[Auburn]

I haven't even cloned the godot repo I just made the change on github 😅 is that an easy thing for someone else to do?

[YuriSizov]

is that an easy thing for someone else to do?

Yes, totally. Godot is very easy to compile. You can follow the documentation for your platform here: https://docs.godotengine.org/en/stable/contributing/development/compiling/index.html

That said, @RedworkDE graciously made the change on your behalf.

[Geometror]

Thanks for stopping by :)
Changing the default value should be fine.
Regarding OpenSimplex noise: Renaming the enum values of course is problematic due to compatibility breakage (at least for Godot 4.x), but maybe we can honor @KdotJPG's work in the documentation by explaining that it uses the open license simplex noise algorithms.
This would also be shown inside the tooltip in the editor:

[Auburn]

That sounds like a good solution 👍

[KdotJPG]

Oh, I appreciate the looking out!

I'm actually chill with the noises being labeled "Simplex", and definitely don't think I need any special callout in the node documentation 😛. I care more that users have access to good quality noise, whatever the authorship may be, and are provided accurate information on the pros and cons.

The 2D case used for TYPE_SIMPLEX is an implementation of plain Simplex anyway, as that wasn't covered by the at-the-time-active US patent, and because it performed better than the canonical extension of OpenSimplex2 to 2D. The 3D TYPE_SIMPLEX is a custom algorithm, as are both the 2D and 3D cases of TYPE_SIMPLEX_SMOOTH.

If you want to give users of the feature more insight on the internals, your suggestion to clarify the implementation/nature in the documentation sounds good to me.

---

On this topic, though, I do think it would be worth changing the order of the entries in the documentation. One of the philosophies considered during FNL's development was to sort the noises by recommended usage consideration order, rather than implementation simplicity or time of invention. The original ordering can be found in the enum.

[Calinou]

On this topic, though, I do think it would be worth changing the order of the entries in the documentation. One of the philosophies considered during FNL's development was to sort the noises by recommended usage consideration order, rather than implementation simplicity or time of invention. The original ordering can be found in the enum.

We can't change the enum's order without breaking compatibility, and we shouldn't make a special case for enum order in the documentation (it should always follow code ordering for consistency).

[KdotJPG]

We can't change the enum's order without breaking compatibility, and we shouldn't make a special case for enum order in the documentation (it should always follow code ordering for consistency).

Hmm, is the enum order not TYPE_SIMPLEX, TYPE_SIMPLEX_SMOOTH, TYPE_CELLULAR, TYPE_PERLIN, TYPE_VALUE_CUBIC, TYPE_VALUE?

The enum is already in the order I'm suggesting, but the tooltip documentation under property noise_type does not reflect it.

[YuriSizov]

Yes indeed, the order of registration for enum values is wrong.

godot/modules/noise/fastnoise_lite.cpp

Lines 445 to 450 in 8649ab8

	BIND_ENUM_CONSTANT(TYPE_VALUE);
	BIND_ENUM_CONSTANT(TYPE_VALUE_CUBIC);
	BIND_ENUM_CONSTANT(TYPE_PERLIN);
	BIND_ENUM_CONSTANT(TYPE_CELLULAR);
	BIND_ENUM_CONSTANT(TYPE_SIMPLEX);
	BIND_ENUM_CONSTANT(TYPE_SIMPLEX_SMOOTH);

[KdotJPG]

Looking further, I do see more opportunity to clarify the documentation on the actual functionality and effects of the options. For example,

	<constant name="FRACTAL_RIDGED" value="2" enum="FractalType">
		Method of combining octaves into a fractal resulting in a "ridged" look.
	</constant>

could be reworded as

	<constant name="FRACTAL_RIDGED" value="2" enum="FractalType">
		Method similar to [constant FRACTAL_FBM] where each layer is passed into a flipped absolute value function.
		Results in a mountainous "ridged" apppearance.
	</constant>

and

	<constant name="DOMAIN_WARP_SIMPLEX" value="0" enum="DomainWarpType">
		The domain is warped using the simplex noise algorithm.
	</constant>

could be changed to

	<constant name="DOMAIN_WARP_SIMPLEX" value="0" enum="DomainWarpType">
		The domain is warped using a modified version of the simplex noise algorithm which outputs vectors.
	</constant>

I wouldn't mind making my own PR for all of this. That way we don't need any further documentation changes in this one.

[akien-mga]

Thanks! And congrats for your first merged Godot contribution 🎉

I wouldn't mind making my own PR for all of this. That way we don't need any further documentation changes in this one.

Yes, please do :)