2013-02-26 10 views
6

lavorando ad un altro progetto e siamo tenuti a utilizzare il mt19937 per la generazione di numeri casuali. Dovremmo scegliere casualmente una coordinata xey basata sulla sezione di una griglia. Per esempio, la mia funzione passa minX, maxX, minY, maxY a una funzione. La mia coordinata x funziona bene. Continuavo a ricevere errori casualmente durante i test. A volte viene eseguito 10 volte senza problemi, quindi viene visualizzato un errore. Inserisco alcune linee di auto-debug per visualizzare ciò che il generatore mt sta effettivamente generando. Come ho detto, x funziona bene e a volte lo fa. Sarà casuale dammi un -3.437.892 o 9743903.Perché il mio generatore casuale mt19937 mi dà risultati ridicoli? C++

Ecco il mio codice:

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] = '.'; 
        } 

      } 
    } 

} 
+0

Sono confuso sul motivo per cui accade solo con iStartY. X funziona bene. – ModdedLife

+2

'mt19937' è destinato a essere il [motore di numeri casuali C++ 11] (http://en.cppreference.com/w/cpp/numeric/random/mersenne_twister_engine) o un tipo definito dall'utente? –

+0

Quali valori avete per i parametri (minX, maxX ecc.) In questi casi? – Slava

risposta

0

capito l'errore amatoriale che ho fatto con l'aiuto di @haatschii.

Si rende così molto senso ora. iStartY e iStartX non avevano limiti per essere impostati su un numero inferiore o uguale a zero. Mi sento così stupido per non aver capito quel lol. Ho aggiunto un altro ciclo per fare in modo che il valore era maggiore di 0. Ho fatto anche i valori iStartX e iStartY iniziano come maxX + 1 e maxy + 1 in modo che essi sarebbero entrati automaticamente il ciclo di generare una soluzione maggiore di 0.

Heres il codice soluzione:

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] = '.'; 
        } 

      } 
    } 

} 

Grazie per i suggerimenti ragazzi!

16

Credo che si dovrebbe usare una distribuzione casuale per il mt19937. Così utilizzarlo con

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

int iRandomWidth = dist(mt); 
int iRandomHeight = dist(mt); 

In questo modo si sono garantiti per ottenere numeri casuali tra 4 e 13.

Aggiornamento Mentre la mia risposta risolve il problema originale e è a mio parere un miglioramento della leggibilità del codice , in realtà non affronta il problema nel codice originale. Si prega di fare riferimento anche alla risposta di jogojapan per questo.

+0

Esattamente questo. Non riesco a vedere dove si trova l'errore nel codice OP, ma ci sono molti problemi con '% n' per generare numeri casuali fino a' n', e inoltre usando le distribuzioni il codice diventa ** molto ** più pulito. – us2012

+0

Sono d'accordo con la raccomandazione, ma perché usare una distribuzione 'uint32_t' per assegnare valori alle variabili' (signed) int'? – jogojapan

+0

@jogojapan: Ho scelto 'uint32_t' perché tutti i valori creati dalla distribuzione sono'> = 0'. Comunque 'int32_t' o' int' dovrebbero essere altrettanto buoni. Avresti persino salvato un cast implicito su int firmato, ma non penso che questo sia importante, vero? – Haatschii

5

La causa ultima del problema è che si mescolano firmato e interi senza segno nel codice senza prendere le necessarie precauzioni (e senza bisogno).

In particolare, se minY è a volte inferiore a 13, succederà ogni tanto che lo iRandomHeight diventa negativo. Quello che si ottiene è quindi simile all'effetto dimostrato di seguito:

#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; 
} 

Questo primo genera un numero intero senza segno leggermente più grande si adatterebbe in un firmato uno. mt19937 restituisce unsigned e talvolta ti darà valori come i nel codice sopra.

L'output del codice precedente (vedi on liveworkspace) è la seguente:

2 
2147484647 
-2147482649 

La seconda riga mostra il risultato di modulo con un numero negativo (ad esempio iRandomHeight sarà talvolta essere), applicato ad un unsigned numero intero più grande di quello corrispondente a un segno firmato corrispondente. La terza riga mostra cosa succede quando converti quello nuovo in intero con segno (che fai implicitamente quando lo assegni a una delle tue variabili integer con segno).

Anche se sono d'accordo con Haatschii sul fatto che dovresti usare std::uniform_int_distribution per semplificarti la vita, è anche importante utilizzare i messaggi firmati e non firmati in modo appropriato anche in quel momento.

+0

Questa è la vera risposta IMO. 'std :: uniform_int_distribution' non è sempre la scelta giusta, ha alcune proprietà indesiderate. –

Problemi correlati