Thread dei Problemi

[BlueDragon]

Originariamente inviato da Jocchan

Ho notato che le flashing gem saltano fuori piuttosto spesso, nonostante il misero 2% di probabilità. Quindi, probabilmente dovremo introdurre un qualche artificio per limitarle, e che non le renda al contempo TROPPO rare.

Mi piace il calcolo delle probabilità..e quindi ho approfittato dell'occasione per dedicare un po' di tempo a Diamonds (che continuo a seguire ma era un sacco di tempo che non leggevo il codice).

Flashing Gems:
Innanzitutto le Flashing Gems hanno il 2% di probabilità di essere create singolarmente....e questo significa che si ha circa il 4% di possibilità di trovare almeno una Flashing Gem in una coppia di gemme. Significa che in media ogni 25 coppie di gemme una contiene una Flashing Gem....ecco perché sono molto meno rare del 2%!
Impostando il config ad 1% oppure modificando il codice affinché la percentuale del 2% sia intesa "per coppia di gemme", probabilmente otterremo il risultato voluto...a meno che anche 1 Flash su 50 coppie sia troppo!

Topazi rari:
Inoltre, nel leggere il codice delle probabilità ho notato delle strane condizioni che rendono il topazio meno probabile delle altre gemme, circa 13.6% di probabilità contro il 16.3% delle altre gemme su un totale di 800.000 gemme.
Il codice (in GemFactory) è il seguente:

    public Gem createRandomGem()
    {
        int module = GEM_TYPES.length - 1;

        if(firstGemExtraction)
        {
            module = GEM_TYPES.length;
            firstGemExtraction = false;
        }

        int randomIndex = randomGenerator.extract(module);

        if(randomIndex >= nextToLastGemIndex)
        {
            randomIndex = (randomIndex + 1) % GEM_TYPES.length;
        }

        nextToLastGemIndex = lastGemIndex;
        lastGemIndex = randomIndex;

        resetChestExtraction();
        return create(GEM_TYPES[randomIndex]);
    }

Rimuovendo la parte in grassetto, si ottiene una perfetta distribuzione delle probabilità tra le gemme.
Non ho ancora analizzato se la sua rimozione rompe qualche test....

Memory Leak:
Nel fare i test sulle probabilità ho avuto occasione di creare parecchie gemme, ed ho quindi notato un consumo di circa 1 GB di RAM per creare 10.000 gemme....si tratta di ben 100kb a gemma
Creavo le gemme con un questo codice:

            
for (int x= 0 ; x < 100000;x++)
            {
                Gem gemma = gemFactory.createRandomDroppable();
                gemma.cleanup();
            }

Chiamare .cleanup() non era sufficiente... Avevo provato anche a metterci un'invocazione al Garbage Collector ogni 1000 iterazioni ma non sembrava aver alcun effetto
Una volta raggiunto il limite della memoria RAM + swap su disco, Diamonds crashava con la finestrella di bugreport.. che appare 2 volte

[Jocchan]

Grandissimo, BD
Il problema della frequenza non è tanto nella percentuale, dato che a ridurla ulteriormente sarebbero TROPPO rare, quanto nel fatto che la randomizzazione dovrebbe essere controllata verso il basso (ovvero, ci vorrebbe un limite minimo per l'intervallo tra una flashing gem e l'altra).
Diluire ulteriormente la sua occorrenza significa che in diverse partite potrebbe non saltare mai fuori, ed avremmo il problema opposto.

[Jocchan]

Molto probabilmente ho scovato un altro paio di bug.

Giocando ho preparato una crush da 4-5x ammucchiando diverse gemme e bauli. Dopo averla innescata, però, il contatore è arrivato ad un assurdo 8x (Astounding).

Questo perchè molto probabilmente c'è un errore di fondo nell'implementazione del conteggio delle crush (e probabilmente è anche colpa di come abbiamo impostato storia e task): a rigor di logica, il contatore della lunghezza deve aumentare solo se l'evento scatenante di una cancellazione è stato un'altra cancellazione.

Allo stato attuale, invece, il gioco considera come crush anche quando, posizionando una coppia di bauli, vengono cancellate due gemme di colore diverso già ferme. Questo non ha senso, dato che la seconda cancellazione non è conseguenza della prima.

Quindi, bisogna vedere di sistemare il tutto in modo da considerare, per le crush, solo i pezzi che cadono per riempire i vuoti. Ne parlerò con Vicius, e probabilmente sarà uno dei task per il prossimo ciclo.

L'altro bug fondamentale riguarda il contatore mostrato sotto l'area di gioco dell'avversario: dopo la (lunga) combo di cui sopra, il contatore in questione segnava il valore di 5. Avevo già notato delle incoerenze, e questo evento ne è stato la conferma. Direi che bisogna dare un'occhiata al suo funzionamento

[BlueDragon]

Topazi Rari:
Ho provato ad eliminare il codice in bold evidenziato nel mio post precedente, e l'unico test che fallisce è il seguente:

   public void testRandomGemSequence()
    {
        Gem gem = factory.createRandomGem();
        assertEquals("does not return Gem of type topaz", TOPAZ, gem.getType());

        gem = factory.createRandomGem();
        assertEquals("does not return Gem of type diamond", DIAMOND, gem
            .getType());

        gem = factory.createRandomGem();
        assertEquals("does not return Gem of type diamond", DIAMOND, gem
            .getType());

        gem = factory.createRandomGem();
        assertEquals("does not return Gem of type ruby", RUBY, gem.getType());

        gem = factory.createRandomGem();
        assertEquals("does not return Gem of type ruby", RUBY, gem.getType());

        gem = factory.createRandomGem();
        assertEquals("does not return Gem of type sapphire", SAPPHIRE, gem
            .getType());
    }*/

E' chiaro che fallisca visto che la factory è creata nel setup() con un MockRandomGenerator cui è stato fornito il seguente indice di valori:

private final int randomIntArray[] = { 4, 0, 4, 0, 0, 1 };

Questi valori corrispondono alla creazione in sequenza di un topazio, un diamante, un topazio, un diamante, un diamante, un rubino...basta modificare i tipi di gemme nel test e passa nuovamente.


Direi quindi che lo strano codice (forse rimasto da una precedente gestione in cui nello stesso array vi erano sia tipi di gemme che chest) può essere eliminato

EDIT: Ho eliminato il codice in bold ed anche il metodo resetGemExtraction(), che in pratica non serviva più a nulla eliminato quell'if nel createRandomGem().
Committato.

[VICIUS]

Originariamente inviato da BlueDragon

Memory Leak:
Nel fare i test sulle probabilità ho avuto occasione di creare parecchie gemme, ed ho quindi notato un consumo di circa 1 GB di RAM per creare 10.000 gemme....si tratta di ben 100kb a gemma
Creavo le gemme con un questo codice:

            
for (int x= 0 ; x < 100000;x++)
            {
                Gem gemma = gemFactory.createRandomDroppable();
                gemma.cleanup();
            }

Ho fatto partire una micro test sul profiler con circa 20000 gemme. Ed in effetti il leak è presente e anche mooolto evidente. Questa è la situazione dopo circa 3000 gemme.

Non ho idea di dove sia il problema, anche perché sto profiler mi è totalmente nuovo, ma a naso penso sia dovuto dalle routine di log o qualcosa che ha a che fare con le stringhe. 142000 AbstractStringBuilder, 182000 String.

ciao

[thebol]

Originariamente inviato da Jocchan

Grandissimo, BD
Il problema della frequenza non è tanto nella percentuale, dato che a ridurla ulteriormente sarebbero TROPPO rare, quanto nel fatto che la randomizzazione dovrebbe essere controllata verso il basso (ovvero, ci vorrebbe un limite minimo per l'intervallo tra una flashing gem e l'altra).
Diluire ulteriormente la sua occorrenza significa che in diverse partite potrebbe non saltare mai fuori, ed avremmo il problema opposto.

è fattibile senza troppi problemi(basta mettere un contatore in gemFactory)

[thebol]

Originariamente inviato da VICIUS

Ho fatto partire una micro test sul profiler con circa 20000 gemme. Ed in effetti il leak è presente e anche mooolto evidente. Questa è la situazione dopo circa 3000 gemme.


Non ho idea di dove sia il problema, anche perché sto profiler mi è totalmente nuovo, ma a naso penso sia dovuto dalle routine di log o qualcosa che ha a che fare con le stringhe. 142000 AbstractStringBuilder, 182000 String.

ciao

le stringhe in java vengono passate per copia(l'ho scoperto meno di un mese fa, ed è da un anno che lavoro su java )

[thebol]

Originariamente inviato da Jocchan

Allo stato attuale, invece, il gioco considera come crush anche quando, posizionando una coppia di bauli, vengono cancellate due gemme di colore diverso già ferme. Questo non ha senso, dato che la seconda cancellazione non è conseguenza della prima.

in teoria nn dovrebbe essere cosi, provo a scrivere un test(ma secondo me cè gia)

[thebol]

Originariamente inviato da thebol

in teoria nn dovrebbe essere cosi, provo a scrivere un test(ma secondo me cè gia)

    public void testDoubleCrush()
    {
        //int delayCrush = config.getInteger("DelayBetweenCrushes");
        
        insertAndUpdate(createGem(DIAMOND), 13, 1);
        insertAndUpdate(createGem(EMERALD), 13, 2);
        insertAndUpdate(createGem(DIAMOND_CHEST), 12, 1);
        insertAndUpdate(createGem(EMERALD_CHEST), 12, 2);
        
        
        insertAndStopGemsPair();
        makeAllGemsFall();

        controller.update(timer);
        assertEquals(2, grid.getNumberOfGems());
        assertEquals(2, grid.getCrushedGemsCounter());
        assertEquals(1, grid.getChainCounter());
        
        
    }

fallisce sul getChainCounter, che infatti ritorna 2 :|

[VICIUS]

Trovata la magagna. Texture.cleanup non funziona.

    public void cleanup()
    {
        if(texID != 0)
        {
            final IntBuffer textures = BufferUtils.createIntBuffer(16);
            textures.put(texID);
            glDeleteTextures(textures);
            texID = 0;
            System.out.println("cancellata!");
        }
        else
        {
            System.out.println("mierda!");
        }

        loaded = false;
    }

Stampa sempre e solo l'imprecazione perchè texID è sempre 0.

ciao

[^TiGeRShArK^]

mmm..
e km mai si ritrova texid sempre a zero?

[VICIUS]

Originariamente inviato da ^TiGeRShArK^

mmm..
e km mai si ritrova texid sempre a zero?

Non ho mai scritto niente in opengl in vita mia. Dobbiamo aspettare qualche esperto. Anche perché questa parte del codice è stata scritta assolutamente non seguendo il TDD.

ciao

[fek]

Originariamente inviato da BlueDragon

Topazi Rari:EDIT: Ho eliminato il codice in bold ed anche il metodo resetGemExtraction(), che in pratica non serviva più a nulla eliminato quell'if nel createRandomGem().
Committato.

Ottima analisi e ottimo lavoro

Mi piace che dopo molto tempo che non hai guardato il codice ti e' stato relativamente facile andare a scovare il problema delle probabilita' dei topazi.

[fek]

Originariamente inviato da thebol

fallisce sul getChainCounter, che infatti ritorna 2 :|

Bel test. Hai tempo di farlo passare entro oggi? Voglio prendere una tag del codice prima di partire.

[fek]

Originariamente inviato da VICIUS

Non ho mai scritto niente in opengl in vita mia. Dobbiamo aspettare qualche esperto. Anche perché questa parte del codice è stata scritta assolutamente non seguendo il TDD.

ciao

Hmmm provo a dargli un'occhiata io a meno che Raffaele non si faccia avanti.

Ecco cosa accade a non scrivere i test, si perde piu' tempo dopo

[thebol]

Originariamente inviato da fek

Bel test. Hai tempo di farlo passare entro oggi? Voglio prendere una tag del codice prima di partire.

entro oggi pome forse, entro sera sicuro.

Anche perche in quella zona ci starebbe bene un bel refactoring(fare una classe abstract Crush, e 2 implementazione, una per le crush da chest e una quelle da flash). Poi in grid si fa un metodo che itera sulla griglia, a cui si passa una di queste classi.

non penso sia complicato, il piu saranno i test da modificare e aggiungere.

[VICIUS]

Originariamente inviato da VICIUS

Trovata la magagna. Texture.cleanup non funziona.
[...]
Stampa sempre e solo l'imprecazione perchè texID è sempre 0.

ciao

Ok. La situazione non è cosi drammatica. La cleanup non va solo nei test perché non c'è nessun Display reale istanziato e opengl non funziona senza. Probabilmente la grande occupazione di ram dei test che ha cercato di risolvere cionci era dovuta a questo problema.

ciao

[Bonfo]

Originariamente inviato da VICIUS

Trovata la magagna. Texture.cleanup non funziona.

    public void cleanup()
    {
        if(texID != 0)
        {
            final IntBuffer textures = BufferUtils.createIntBuffer(16);
            textures.put(texID);
            glDeleteTextures(textures);
            texID = 0;
            System.out.println("cancellata!");
        }
        else
        {
            System.out.println("mierda!");
        }

        loaded = false;
    }

Stampa sempre e solo l'imprecazione perchè texID è sempre 0.

ciao

Dov'è la cleanup...non la trovo

[fek]

Originariamente inviato da VICIUS

Ok. La situazione non è cosi drammatica. La cleanup non va solo nei test perché non c'è nessun Display reale istanziato e opengl non funziona senza. Probabilmente la grande occupazione di ram dei test che ha cercato di risolvere cionci era dovuta a questo problema.

ciao

Si puo' fare un mock che non allochi fisicamente la texture nei test (tanto non serve).

[VICIUS]

Originariamente inviato da fek

Si puo' fare un mock che non allochi fisicamente la texture nei test (tanto non serve).

Si può fare. Chi ci pensa ?

ciao