allegroflare / allegro_flare

Application toolkit for Allegro 5
https://clubcatt.com/allegroflare
MIT License
35 stars 6 forks source link

In Random, calling member functions out of order will break predictable outputs #212

Closed MarkOates closed 2 years ago

MarkOates commented 2 years ago

Scenario

Functions in Random are guaranteed to be predictably random given the same seed across platforms. This design makes Random a reliable source for seeding procedurally generated content - content that will be the same on each run, and will be the same on all platforms.

Problem (problem 1)

However, if you call different get_random_* methods or change the order of the get_random_* methods then the predictability could break. As an example, if you call get_random_int and get_random_int again. Each time this will be consistent after you set the seed. However, if between those calls you make another call to, say, get_random_letter, then that second call to get_random_int will return a different result. This is because get_random_letter internally calls another get_random_int to generate its random letter, resulting in 3 total calls to get_random_int.

From a software design perspective, the design flaw is that there is some side-effect to the internal internal state when calling the methods.

Another Interesting Problem (problem 2)

Because methods are used internally to drive random numbers, there can potentially be some awkward aliasing between methods if used along side each other. Specifically, get_random_bool() and get_random_sign() use the exact same technique to randomly choose between either true/false or 1/-1. That is, if two Random instances were created with the same seed, and one was used make calls to get_random_bool() while the other was used to make calls to get_random_sign(), the outputs would be symmetrical, something like:

//random_generator_a outputs get_random_bool():
{ true, false, true, true, false, false, false, true, true }
//random_generator_b outputs get_random_sign():
{ 1,    -1,    1,    1,    -1,    -1,    -1,    1,    1    }

The two outputs, while different value types, would still have the same underlying pattern. This is an issue if you used the generators to generate random content (say for random flower graphics on a game), and the procedurally content oddly aligned with attributes (all "false" flowers also became "-1" for their other random attribute 100% of the time.) This would be odd and unintuitive behaviour for a Random number generator.

Solution

The easiest solution I can think of is to give each method its own instance of the random_number_generator (an instance of std::mt19937). This would sill allow each method to be able to generate repeatability on a given seed without having it potentially be disrupted by code changes.

To solve problem 2, each method's instance of random_number_generator would need to have a different seed. So, when setting the seed (either via the constructor or via set_seed), the individual seeds would need to be offset by some amount so they remain consistently reproducible regardless of "primary_seed". I'm sure just a simple +1 for each sequential seed would be fine.

MarkOates commented 2 years ago

K, this will be fixed with https://github.com/allegroflare/allegro_flare/pull/213.

the individual seeds would need to be offset by some amount so they remain consistently reproducible regardless of "primary_seed". I'm sure just a simple +1 for each sequential seed would be fine.

Rather than using +1 for seeds, I opted for a simple hashing value on a string to produce a number to add to the primary_seed for each.

That way if the cascaded seeding were to change order, it wouldn't change the value of the outputs.

MarkOates commented 2 years ago

finished in https://github.com/allegroflare/allegro_flare/pull/213