tresinformal / drakkar

The tresinformal video game called 'Drakkar'
GNU General Public License v3.0
11 stars 4 forks source link

Create a different seed every time #645

Closed ollyturner closed 1 year ago

ollyturner commented 1 year ago

Just setting the seed to 1 on line 240 of color.cpp only reflects 1 possibility. If we set it to the current second of the year, it will be different almost every time.


  {
    color c;
    int n_red = 0;
    int n_green = 0;
    int n_blue = 0;

    const color r = create_red_color();
    const color g = create_green_color();
    const color b = create_blue_color();

    game g;
    std::mt19937 rng(g;
    for (int i = 0; i < 100; ++i)
      {
        c = get_random_rgb(rng);
        if (c == r) { ++n_red; }
        else if (c == g) { ++n_green; }
        else if (c == b) { ++n_blue; }
        else { throw("get_random_rgb should not return any other color than r, g, b"); }
      }

    assert(n_red > 0 && n_red < 100);
    assert(n_green > 0 && n_green < 100);
    assert(n_blue > 0 && n_blue < 100);
  }```
richelbilderbeek commented 1 year ago

I enjoy the enthusiasm to get two random colors. However, a fixed or unknown seed is A Good Thing in testing.

I think this a great test without using a RNG engine (i.e. it uses std::rand):

// Check if the color is actually random
{
   const auto c = get_random_color();
   const auto d = get_random_color();
   assert(c != d); // Chance 1 in 256^3 this fails
}

I think this a great test with using a RNG engine (i.e. it uses std::mt19937):

// Check if the color is actually random
{
  const int seed{42}
   std::mt19937 rng(seed);
   const auto c = get_random_color(rng); //Will modify RNG
   const auto d = get_random_color(rng); //Will modify RNG
   assert(c != d); // Chance 1 in 256^3 this fails, pick a different seed if needed
}
// Check if the color is actually random yet repeatable
{
   const int seed{42}
   std::mt19937 rng1(seed);
   std::mt19937 rng2(seed);
   const auto c = get_random_color(rng1); //Will modify RNG
   const auto d = get_random_color(rng2); //Will modify RNG
   assert(c == d);
}
TheoPannetier commented 1 year ago

Hey @ollyturner, I think the updated test I pushed on develop for #628 now tests for two seeds. Please have a look and, if you're happy with it I think this issue can be closed as a duplicate?

ollyturner commented 1 year ago

👍 Looks good. I agree, this issue should be closed!