Add noise map generation utilities

[tomrijnbeek]

✨ What's this?

This PR introduces a new namespace that can be used to automatically generate noise. Specifically, we expose an interface that can return a value for a rectangular area based on an underlying noise function. This PR includes basic uniform noise and Perlin noise (the latter implementation pretty much straight up copied from TD).

🔍 Why do we want this?

Any algorithm generating 2D randomness (whether graphically or mechanically) will need functionality similar to the functionality in this PR. While uniform randomness is fairly simple to generate, Perlin randomness is a bit fiddly. Having reusable methods to "just" give you a Perlin noise map sounds hugely beneficial to anybody bootstrapping some of this generation in their game.

🏗 How is it done?

This PR describes an extendable set of interfaces for generating noise. The result of generating noise is an interface that allows the caller to look up a noise value for any coordinate within a rectangular area (upper bounds exclusive). This has already proven beneficial since Perlin noise and uniform noise have different methods of getting to a value based on their underlying noise array. It also allows for extendibility such as for example transformation methods, combining multiple noise maps in interesting ways, etc. without having to have to deal with a different interface. Of course performance concerns creep in at that point, but the current interface doesn't preclude you to "pre-calculate" a noise texture by sampling the underlying noise functions, though it is unlikely to give a huge performance boost overall unless you sample the same points a lot.

As explained above, the current PR introduces two noise types:

• Uniform noise, which creates a "noise texture" with pseudo-randomly generated values for each "pixel". It also allows you to specify an interpolation method that is used to handle values between pixels. Nearest and bilinear are the most logical here (bilinear wraps around).
• Perlin noise. This implementation is copied from TD, and is generated by using a lattice of vectors. The noise value at a given point is generated from some smart interpolation of dot products to create an overall smoother noise.

In addition, since the noise functionality had a need to pass in an interpolation method, I created a structure where interpolation methods have a very enum-like behaviour, but are in fact static objects under the hood. Introducing further interpolation methods should not be too hard, though further design will have to be done if we want to do something like (bi)cubic or Bezier interpolation, since they need additional values besides "from" and "to".

🔬 Why not another way?

There are a few decisions I made along the way. These are the options I remember rejecting:

• Make noise generation generic. Wouldn't it be great if you could generate a noise of floats, doubles, even vectors? While yes, it would be great, I don't think one noise generation algorithm maps to another domain trivially, so sticking with a simple value seems like it made the most sense. You can also transform the double into something else if really needed. All in all, I believe the simplicity in this interface outweighs hypothetical use cases.
• Make interpolation generic. Same as above: applying interpolation to colours and vectors can be useful, though it isn't immediately clear you would use exactly the same techniques. On top of that, it just sounds like a pain if you have to apply for each interpolation type how to add, multiply, etc. given that there just isn't a way in C# to add type restrictions based on the present operators. All in all, I followed the same argument as above: let's stick with something simple that works, before trying to come up with hypothetical use cases. If we need a generic interpolation framework, we can revisit when we have a solid use case.
• More tests. One would be right to argue that the current PR has very few tests. I could definitely extensively test both the interpolation and noise generation, but instead I opted to only test some basic properties of the noise generation. The reason is only partly laziness: I think property tests are super helpful for this domain, but the documentation of FsCheck is incredibly hard to delve through given that the majority of examples is in F# and doesn't seem to easily map to C# + XUnit. I'd much rather find a property testing framework that is not F# based, or otherwise find a better method of setting up these tests.

🦋 Side effects

We may want to consider moving the existing Interpolate methods over, but since they are floats, and the new interpolation methods work with doubles, this would potentially be a bigger change.

I can think of many additional improvements. I have created issues for all of them (I might remember more later): #222, #223, #224

💡 Review hints

I am aware this PR probably will not be without controversial choices, even if the actual added functionality is not ground-breaking. I considered splitting it already, but given GitHub's awkward support on stacking PRs and the fact that all of this goes together somewhat, I decided to keep it.

Feel free to do an initial pass over the proposed interfaces and APIs first before delving in implementations.

I would ask to focus review comments on the overall structure and decisions made here, rather than on the choice of what to include or not. As seen above, it was easy to come up with many future improvements to this framework, including potential performance optimisations. My hope with this PR is to have an initial foundation submitted.

[paulcscharf]

All the tests here use 5x5 size, and there are no tests to validate whether these parameters do anything at all. Is that alright?

[tomrijnbeek]

As explained in the PR description: the tests om this PR are by no means meant to be exhaustive.

In addition, it is hard to test that these parameters really do something. The randomness involved makes it hard to make hard assertions about the output. There is always a chance that the random output of a 10x10 grid just happens to line up really well with the properties of a 5x5 grid.

We can always generate two maps with the exact same seed with different dimensions and assume they are different in at least one point when converted to a discrete array, but there is no guarantee this is going to be the case for all seeds, and I think even that level of flakiness is not something we want to avoid in our tests.

[paulcscharf]

This and the test below can't tell if both coordinates are being checked for range correctly, and the -1/+1 makes it look a lot like discrete indices, even though it isn't.

[tomrijnbeek]

Addressed by splitting the tests across dimensions and changing the 1 to 1.0. The choice for 1 is on purpose as it will exercise edge cases.

[paulcscharf]

Do we intend others to implement this? If so, should it be outside this class perhaps? If not, it should be an abstract class with internal protected constructor.

[tomrijnbeek]

It already is an outer class. I just didn't really want to have a new file for just this one line method. That being said, given the confusion this seems to be leading to, I have extracted these interfaces to separate files.

[paulcscharf]

There is nothing about this interface related to actual noise. Should we rename it more general?

[tomrijnbeek]

Renamed to ProceduralTexture since according to Wikipedia, that is the right name for something like this. Perlin noise specifically is mentioned as an example. I'm open to renaming the entire namespace, or move the non-noise specific procedural texture types to Core or their own namespace.

[paulcscharf]

So this actually wraps around? Is that really intended? I definitely didn't expect it. I would have thought it would map 0 to the first pixel and 1 to the last, instead both map to the average of first and last?

I'm not sure if wrapping is good default behaviour. If anything maybe we have to inject it, if we want to support it?

[tomrijnbeek]

Imagine a simple noise texture as actual pixels. The array here represents the pixels of that texture. The value you pass in is actually the continuous coordinate on that texture, and there's a good chance that coordinate doesn't exactly align with the centre of a pixel. This is where the interpolation method that gets passed in comes in. If you do bilinear interpolation, you need something to interpolate with. We could interpolate with 0, but this causes artefacts along the outside, which I think is undesirable.

Note that if you use nearest neighbour as interpolation method, this will all still add up to returning the value of pixel [0, 0] in coordinates (0, 0).

I'm not sure if wrapping is good default behaviour. If anything maybe we have to inject it, if we want to support it?

I agree the wrapping behaviour should be injected, but given the already existent complexity of this PR, has been deferred to future PRs. I think it makes sense to provide a default value though, and I chose repeat in this case (which is in line with the OpenGL texture default)

[paulcscharf]

Instead of injecting a seed, I think it would be better to inject a Random instance. That would allow better control over where numbers come from and can prevent unnecessary allocation.

[tomrijnbeek]

I am happy to be convinced otherwise, but let me explain my reasoning:

One of the properties is that this method returns the exact same result if you give it the same input, i.e. it's deterministic (unless you choose not to pass in a seed). When you replace the seed with a Random instance, this because much more nuanced. Sure, if you pass in the "same" Random, it is still deterministic. However, there isn't an Equals method on Random, so the whole concept of having the "same" Random is a bit weird. Using a seed makes this much more obviously correct.

Like I said, happy to be convinced otherwise.

[paulcscharf]

I see what you mean. In many other places we do inject Random though, like in the enumerable shuffle extensions, and we do the same in TD level generation. Would you argue we shouldn't do so there?

The main arguments I have is that injecting a seed is very low level (yuck, primitives), and that it causes potentially unnecessary allocation. Also, of course you could easily new Random(seed) at the call site.

I think using Random is nicer for public interfaces at least.

[paulcscharf]

It is not clear to me what the numCells parameters do/mean.

Next to that I think this should take a Random, not a seed.

[tomrijnbeek]

I think you need to know something about how Perlin noise works to understand these parameters. I have added a <summary> fragment since I think that is the only way to explain how these parameters map to the underlying algorithm.

[paulcscharf]

Similar to above, why do we wannt it to wrap? Should that be the default? Should that be configurable?

[tomrijnbeek]

Similar to above, why do we wannt it to wrap?

By default, yes.

Should that be the default?

Yes, this sounds the most expected and is in line with OpenGL defaults for traditional textures, and this doesn't immediately break cases that don't expect it.

Should that be configurable?

Yes, to be added in a future PR.

[tomrijnbeek]

Still awaiting further review I believe.

[paulcscharf]

It's been a while since I last took a look, but I think I like it better now. :)

[tomrijnbeek]

Ready for another round of review.

[paulcscharf]

I think you addressed my comments pretty well. Apart from what looks like a typo, go ahead.