Why is my mt19937 Random generator giving me ridiculous results? C ++

Question

Work on another project, and we must use mt19937 to randomly generate numbers. It is assumed that he will randomly select the x and y coordinates based on the grid section. For example, my function passes the functions minX , maxX , minY , maxY to the function. My x coordinate is working fine. I kept getting random errors on test runs. Sometimes it starts 10 times without problems, and then removes the error. I am inserting some self-taping lines to show what the mt generator generates. As I said, x works fine, and sometimes it does. This will randomly give me -3437892 or 9743903.

Here is my code:

 void DungeonLevel::generateRoom(int minX,int maxX,int minY, int maxY){ mt19937 mt; mt.seed( time(NULL) ); // Calculate random width and height; these both range // from 4-13 int iRandomWidth = 4 + (mt() % 10); int iRandomHeight = 4 + (mt() % 10); // Calculate the start points in both X and Y directions int iStartX; iStartX = mt() % (maxX - iRandomWidth); cout << "xStart: " << iStartX<<endl; //cout flag while ((iStartX > maxX) && (iStartX >= 0)){ cout << "xStart: " << iStartX<<endl;//cout flag iStartX = mt() % (maxX - iRandomWidth); } int iStartY = 0; iStartY = mt() % (maxY - iRandomHeight); cout<<"yStart: " <<iStartY<<endl; //cout flag while ((iStartY > maxY)){ cout<<"yStart: " <<iStartY<<endl;//cout flag iStartY = (mt() % (maxY - iRandomHeight)); } // Iterate through both x and y coordinates, and // set the tiles to room tiles // SINGLE ROOM for( int x = iStartX; x <= iStartX + iRandomWidth; x++ ){ for( int y = iStartY; y <= iStartY + iRandomHeight; y++ ){ if (y == iStartY){ dungeonGrid[y][x] = '-'; } else if (iStartX == x){ dungeonGrid[y][x] = '|'; } else if (y == (iStartY+iRandomHeight)){ dungeonGrid[y][x] = '-'; } else if (x == (iStartX+iRandomWidth)){ dungeonGrid[y][x] = '|'; } else { dungeonGrid[y][x] = '.'; } } } } 

Answer 1

Found out an amateur mistake I made with @haatschii.

Now that makes sense. iStartY and iStartX had no restrictions in order to be set to a number lower or equal to zero. I feel so dumb I didn't catch this lol. I added another loop to make sure the value is greater than 0. I also made the values ​​of iStartX and iStartY starting with maxX + 1 and maxY + 1 so that they automatically go into the loop to create a solution greater than 0.

Here is the solution code:

 void DungeonLevel::generateRoom(int minX,int maxX,int minY, int maxY){ mt19937 mt; mt.seed( time(NULL) ); // Calculate random width and height; these both range // from 4-13 int iRandomWidth = 4 + (mt() % 10); int iRandomHeight = 4 + (mt() % 10); int iStartX = maxX+1; //automatically has to enter the second while loop while ((iStartX > maxX) && (iStartX >= 0)){ while ((maxX - iRandomWidth) <= 0){ iRandomHeight = 4 + (mt() % 10); //makes value > 0 } iStartX = mt() % (maxX - iRandomWidth); } int iStartY = maxY+1; //automatically has to enter the second loop while ((iStartY > maxY)){ while ((maxY - iRandomHeight) <= 0){ iRandomHeight = 4 + (mt() % 10); //sets to valid value } iStartY = mt() % (maxY - iRandomHeight); } // Iterate through both x and y coordinates, and // set the tiles to room tiles // SINGLE ROOM for( int x = iStartX; x <= iStartX + iRandomWidth; x++ ){ for( int y = iStartY; y <= iStartY + iRandomHeight; y++ ){ if (y == iStartY){ dungeonGrid[y][x] = '-'; } else if (iStartX == x){ dungeonGrid[y][x] = '|'; } else if (y == (iStartY+iRandomHeight)){ dungeonGrid[y][x] = '-'; } else if (x == (iStartX+iRandomWidth)){ dungeonGrid[y][x] = '|'; } else { dungeonGrid[y][x] = '.'; } } } } 

---

Thanks for the advice guys!

Answer 2

I think you should use a random distribution for mt19937. Therefore use it with

 mt19937 mt; mt.seed( time(nullptr) ); std::uniform_int_distribution<int> dist(4, 13); int iRandomWidth = dist(mt); int iRandomHeight = dist(mt); 

Thus, you are guaranteed to get random numbers from 4 to 13.

Update Although my answer solves the original problem and, in my opinion, improves the readability of the code, in fact it does not affect the problem in the source code. Please also refer to jogojapan's answer for this.

Answer 3

The ultimate cause of the problem is that you mix unsigned integers in code without taking the necessary precautions (and without the need).

In particular, if minY sometimes less than 13, a negative iRandomHeight will occur from time to time. What you get is similar to the effect shown below:

 #include <limits> #include <iostream> using namespace std; int main() { /* Unsigned integer larger than would fit into a signed one. This is the kind of thing mt199737 returns sometimes. */ unsigned int i = ((unsigned int)std::numeric_limits<int>::max()) + 1000; cout << (i % 3) << endl; cout << (i % -3) << endl; cout << (signed)(i % -3) << endl; return 0; } 

This first generates an unsigned integer, slightly larger than fits into the signed one. mt19937 returns unsigned and sometimes gives values ​​like i in the above code.

The code output above (see liveworkspace ) is as follows:

 2 2147484647 -2147482649 

The second line shows the result modulo with a negative number (for example, iRandomHeight will ever be), applied to an unsigned integer more than it matches the corresponding signed one. The third line shows what happens when you convert this back to a signed integer (which you implicitly execute when you assign it to one of your signed integer variables).

Although I agree with Haatschii that you should use std::uniform_int_distribution to make your life easier, it is also important to use signed and unsigned values ​​even then.