[CICLO 1] Abbiamo il primo bug!

[cisc]

ecco il test in gemTestCollisionSound:

    public void testCollisionSoundNearWall() 

    {

        Input input=Input.createInputForTesting();

        gem.setCollisionSound(sound);

        gem.move(-67.99f,0);

        assertEquals(gem.getWidth()/2+0.01f, gem.getX(), 0.001f);

        assertTrue (gem.isPulsing());

        assertFalse (sound.wasPlayed());

        gem.draw(null);

        gem.reactToInput(input);

        assertFalse (sound.wasPlayed());

    }

questo codice testa il fatto che senza dare un move (ma solo a causa del "pulsing") si verifica una collisione che genera un sound, testare i suoni a ripetizioni che in realtà si verifichiano è impossibile!!!

[BlueDragon]

Originariamente inviato da cisc

cionci a quanto ho capito il problema è che quando ti avvicini in un certo modo, il diamante sta pulsando e quindi, dato che il codice delle collisioni controlla valori che in realtà stanno variando, si crea questo bug, penso che la soluzione più semplice sia nel mantenere altre due variabili che memorizzino le dimensioni "statiche" del diamante, l'unico problema che sto avendo è il fare un test che mi rilevi il bug..

Concordo con cisc...tra l'altro io suggerirei anche qualche refactoring...

Per esempio a me CheckForCollision risulta molto più chiaro così:

   private boolean checkForCollision(float dx, float dy)
    {

        boolean collided = false;
        
        if(testLeftEdgeCollision())
        {
            x = (width / 2);
            collided = true;
        }
        if(testRightEdgeCollision())
        {
            x = (displayWidth - (width / 2));
            collided = true;
        }
        if(testTopEdgeCollision())
        {
            y = (height / 2);
            collided = true;
        }
        if(testBottomEdgeCollision())
        {
            y = (displayHeight - (height / 2));
            collided = true;
        }

      
        return collided;
    }

Rispetto alla versione attuale in cui ci sono 3 variabili boolean. una per il return finale, una per l'angolo altosinistra ed una per l'angolo in basso a destra...che poi confondono ancora di più se si considera che la variabile collisionBottomRight viene settata dopo aver controllato Top e Bottom collision anziché dopo aver controllato Bottom e Right come suggerirebbe il nome...

Inoltre nei metodi testBottomEdgeCollision (e Left,Right,Top) non serve passare alcun valore visto che usano direttamente le variabili interne, ed infatti nella mia versione locale gli ho tolto i parametri come potete vedere dalle chiamate del metodo checkForCollision suggerito sopra.

Inoltre, visto che nel codice il movimento è true quando non ci sono collisioni, io direi che le collisioni devono essere true quando la texture tenta di superare i limiti, non quando ci arriva pari pari.
Cioè, tradotto in codice:

    private Boolean testLeftEdgeCollision(float dx)
    {
        return ((x - (width / 2)) < 0); // < 0 anziché il <= 0
    }

Infatti quando un oggetto muove scende preciso lungo un bordo verticale (tralasciando la questione delle pulsazioni) non sta collidendo con il bordo, ma ci si sta muovendo tranquillamente accanto...quindi il movimendo dovrebbe true e la collisione false (e quindi niente suono mentre scendi o sali).

[fek]

Originariamente inviato da BlueDragon

Concordo con cisc...tra l'altro io suggerirei anche qualche refactoring...

Procedi pure al refactoring. Cerca di scrivere anche un test che verifichi la condizione che hai esposto.

Bravi tutti e due.

[BlueDragon]

Originariamente inviato da fek

Procedi pure al refactoring. Cerca di scrivere anche un test che verifichi la condizione che hai esposto.

Bravi tutti e due.

Adesso penso di andare a nanna ma domani magari tiro fuori qualche test se non l'ha già fatto qualcun'altro.
Per quanto riguarda il "procedere al refactoring", la mia copia locale è già modificata, i test non sembrano dare segnali rossi (spero di aver impostato bene ) però non so come si fa il commit verso il repository (serviva anche una pass no?).

[BlueDragon]

Originariamente inviato da BlueDragon

Adesso penso di andare a nanna ma domani magari tiro fuori qualche test se non l'ha già fatto qualcun'altro.
Per quanto riguarda il "procedere al refactoring", la mia copia locale è già modificata, i test non sembrano dare segnali rossi (spero di aver impostato bene ) però non so come si fa il commit verso il repository (serviva anche una pass no?).

Ok, anziché andare a nanna ho fatto il commit del refactoring di
checkForCollision(float dx, float dy)
e dei 4 metodi di check collisioni sui bordi (ma senza modificare il <=0 in <0 per adesso).
Grazie fek per la disponibilità su Msn

[cionci]

Grandi

[cover]

Originariamente inviato da fek

E invece no!

Ti sei accorto che premendo prima la freccia a destra e poi la freccia a sinistra contemporaneamente il diamante si sposta a destra, mentre vuoi che stia fermo. Bug!

Da correggere, ma non prima di un bel test che lo esponga

come? Mi dici che non te n'eri accorto? Dettagli implementatitivi...

Della serie a volte ritornano...
Appena fatto l'update, provato a far passare i test e passano tutti....ok, è tutto a posto fin'ora....lanciamo il tutto e....nooo, il background non viene caricato completamente (256*512) ma solo una piccola parte (256*40), mah..sarà normale? si passa avanti a "giocare" con il movimento,
destra --> destra
wow
sinistra --> sinistra
figata!
su --> come può andare su qualcosa che deve scendere per forza?
giu --> niente, bè, si vede che non è possibile mandare giu più velocemente la gemma...
diagonali --> ovviamente nulla
destra+sinistra (e contrario) --> nooooooooo, cos'è successo? il test fatto ai tempi non ci ha segnalato questo? come è possibile, qua urge qualche modifica...noi (o meglio jocchan ) lo vogliamo fermo, non questo:

Exception in thread "main" java.lang.IllegalArgumentException
at it.diamonds.Grid.insertGem(Grid.java:72)
at it.diamonds.Grid.updateGemAfterInput(Grid.java:320)
at it.diamonds.Grid.reactToInput(Grid.java:158)
at it.diamonds.Game.processInput(Game.java:156)
at it.diamonds.Game.main(Game.java:52)

Lo metterei a posto io, però c'è un piccolo problema... sono ancora in fase di studio di java e pian piano mi sto guardando tutto il codice di diamond... a voi esperti quindi...

[71104]

ahio... sto iniziando a pensare che il refactoring di fek del codice di cionci non sia stata una gran bella idea dopotutto...
ok: è colpa di chet!! chi sistema il codice?
(io non mi smentisco!!! )

[fek]

Originariamente inviato da 71104

ahio... sto iniziando a pensare che il refactoring di fek del codice di cionci non sia stata una gran bella idea dopotutto...
ok: è colpa di chet!! chi sistema il codice?
(io non mi smentisco!!! )

Tu
E prima scrivi il test relativo.

[71104]

Originariamente inviato da fek

Tu
E prima scrivi il test relativo.

ok, mi prudevano giusto le mani per l'inattività

[71104]

ragazzi, la sequenza di input da generare per mettere in risalto l'errore è piuttosto complessa, ho dovuto farmi aiutare dal debugger, comunque ecco qua il test fallimentare che ho aggiunto in TestGridReactionToInput:

    public void testRapidInputs()
    {
        grid.insertGemIntoColumn(1, gem);
        input.generateKey(KeyCode.vk_Right);
        input.generateKey(KeyCode.vk_Left);
        grid.reactToInput(input);
        input.generateKey(KeyCode.vk_Left, false);
        grid.reactToInput(input);
        grid.reactToInput(input);
    }

cercherò di risolverlo dopo pranzo, ma penso che sarà un lavoro molto lungo O_o'

[71104]

errata coccige: il problema si verifica a causa dell'inserzione della stessa gemma in posizioni diverse; per correggere il problema degli "input rapidi" basta correggere quest'altro; ora provo a scrivere un altro test per quest'altro problema (ma quando pranzo oggi? )

[71104]

eccolo qua, si trova in TestGrid:

    public void testSameInsertion()
    {
        grid.insertGem(4, 3, gem);
        try
        {
            grid.insertGem(4, 4, gem);
        }
        catch(IllegalArgumentException e)
        {
            return;
        }
        fail("same gem inserted at different positions");
    }

l'altro test comunque non lo elimino, può sempre servire; ancora non ho committato nulla, cercherò di risolvere i test dopo pranzo.

[fek]

Ottimo lavoro.

[71104]

Originariamente inviato da fek

Ottimo lavoro.

della serie, "ottimo lavoro, e mo' ad implementarlo ti ci voglio!!"
questo problema non è di semplice soluzione; a me vengono in mente 3 possibili maniere di risolverlo:
1) doppio for in insertGem che cicla su tutta la matrice per vedere se la gemma è già inserita da qualche altra parte
2) aggiunta di un campo row e column in Gem che identificano la posizione nella rispettiva Grid (da impostarsi a (-1, -1) nel caso la Gem non sia inserita in nessun oggetto Grid)
3) implementazione di una hash map in Grid per ritrovare facilmente tutte le Gem inserite.
mi riesce difficile giudicare quale sia la soluzione più semplice: la prima è troppo complessa (troppo codice in una volta sola), la seconda comporta una duplicazione dell'informazione che potrebbe rivelarsi difficile da gestire e potrebbe portare errori, la terza ha lo stesso problema della prima ed inoltre non so se sia fattibile perché non conosco le hash map in Java e non so se il puntatore ad un oggetto può essere usato come hash dell'oggetto stesso.
beh, io per adesso opterei per la prima ma credo che alla fine della storia sia opportuno un bel refactoring in cui eventualmente si applicherà la terza.

[fek]

Per ora vai con 1).

[71104]

Originariamente inviato da fek

Per ora vai con 1).

non mi ero accorto che è stato aggiunto un metodo findGemCell: grazie a questo la soluzione 1 diventa molto più semplice; ora cerco di risolvere l'altro test.
PS: ho anche aggiunto un piccolo test a findGemCell che serve per il modo in cui uso quel metodo in insertGem.

[71104]

ed anche questa è fatta ^^
ho committato tutto e la build dovrebbe essere verde.
resta solo da vedere perché l'abbassamento veloce non funziona più; lo faccio io?

[fek]

Originariamente inviato da 71104

ed anche questa è fatta ^^
ho committato tutto e la build dovrebbe essere verde.
resta solo da vedere perché l'abbassamento veloce non funziona più; lo faccio io?

C'e' un task che richiede di implementare l'abbassamento veloce?

[71104]

Originariamente inviato da fek

C'e' un task che richiede di implementare l'abbassamento veloce?

lo chiede il customer ^^
comunque hai ragione, aspetterò il task