`rng` parameter to Options constructor is never used

[bentorkington]

The method signature looks like it provides for an optional RNG to be injected, providing for some default if this is not specified. In reality, any injected RNG, or the (currently null and invalid) default goes nowhere. The ultimate RNG is either constructed from the supplied seed, or constructs one in the initializer.

The instance variable DefaultRng seems of no value. DefaultSeed is misleading because if the value is allowed to remain as default, Random will use its default initializer which will use the system clock for a seed instead. Callers may be surprised by this special case if using it to preseed.

The public Seed is misleading as it will be incorrect if Rng uses its default initializer. It probably makes no sense for a caller who didn't inject a seed to need to know what it is.

Because injecting an RNG or a seed are mutually exclusive, the class should have three constructors which cover each possibility, which also include the optional strict parameter.

[bentorkington]

This cascades into one of the a constructor signatures for Grammar also. It only has one caller, which doesn't use either of the parameters, so can be simplified if we know nobody is calling it (but it's currently public)

Overall, I think we should probably not be easy to accidentally instantiate either class with a default/fixed seed. Using named optional parameters does seem to cloud the API in that

• a default Rng will produce different results on each invocation
• a default seed will do the opposite

A library user who knows they need to set a seed will probably understand why they're doing that and take care of seeding and injecting the RNG themselves. I can see value in providing a convenience to do this, but I feel like it shouldn't be easy to confuse the two contrasting behaviours.

The C# grammar kinda guides us here as optional parameters:

• don't work well in cases when two or more of them are mutually exclusive
• result in ambiguous method calls if you try to fan those options out

[maetl]

Because injecting an RNG or a seed are mutually exclusive, the class should have three constructors which cover each possibility, which also include the optional strict parameter.

This seems like a sensible way to fix the issue.

With the different options, there are several different use cases here:

• The user doesn’t care at all, just wants a random grammar result (don’t inject anything or use the seed at all)
• The user wants a new grammar instance with a consistent seed across multiple runs, but doesn’t care about the state of the RNG or using it anywhere else in their program
• The user has an existing RNG with state they want to inject—maybe they are doing procedural generation and want to ensure random choices within their grammar are consistently interleaved in sequence with other random choices elsewhere in their program