[BUG #1] Freeze della tastiera

[BlueDragon]

Ok, che ne dite di questo codice in GridController? E' incriminabile?

    public void reactToInput(TimerInterface timer)
    {
        if(gemsPair.oneGemIsNotFalling())
        {
            inputReactor.emptyQueue();
            return;
        }

        if(!currentState.isCurrentState("StoneFall") && !currentState.isCurrentState("GemFall"))
        {
            inputReactor.reactToInput(timer);
        }
    }

Se una della gemme non sta cadendo, la coda di eventi della tastiera viene svuotata(!!). Quindi se poggi una delle gemme su una colonna e rilasci il tasto mentre la rimanente sta cadendo, il tuo rilascio del tasto viene "svuotato" dalla coda...quindi è come se rimanesse premuto

[Bonfo]

WOW...sembra lui...test per scoprire se hau trovato l'insetto puzzone

[BlueDragon]

Originariamente inviato da Bonfo

WOW...sembra lui...test per scoprire se hau trovato l'insetto puzzone

Il test manuale mi ha già risposto...commentando inputReactor.emptyQueue(); non riesco più a farlo capitare

[VICIUS]

Modificare emptyQueue per risparmiare gli eventi "Tasto rilasciato" dovrebbe risolvere giusto?

edit:
Peroché invece non gestiamo i tasti nella coda invece di cancellare la lista.

    public void emptyQueue()
    {
        while(!input.isEmpty())
        {
            handleKeyEvent(input.extractKey());
        }
    }

In questo modo non riesco piu a riprodure il bug.

ciao

[fek]

Ottimo! Se adesso uccidiamo anche la gemma solitaria, siamo ancora sul filo di lana per rilasciare l'update della FP.

[BlueDragon]

Visto che non mi era chiaro il motivo di:

 if(gemsPair.oneGemIsNotFalling())
        {
            inputReactor.emptyQueue();
            return;
        }

Ho provato ad eliminarlo, e fallisce questo test:

    public void testSlaveGemFallsFaster()
    {
        final double strongestGravity = environment.getConfig().getInteger("StrongestGravityMultiplier") * 0.5;

        controller.getGemsPair().rotateClockwise();
        controller.getGemsPair().getPivotGem().drop();
        controller.update(environment.getTimer());   
        
        assertEquals(strongestGravity, grid.getActualGravity(), 0.01);
                
        input.notifyKeyEvent(KeyEvent.DOWN, KeyEvent.PRESSED);
        
        controller.reactToInput(environment.getTimer());

        assertEquals(strongestGravity, grid.getActualGravity(), 0.01);

        input.notifyKeyEvent(KeyEvent.DOWN, KeyEvent.RELEASED);
        
        controller.reactToInput(environment.getTimer());

        assertEquals(strongestGravity, grid.getActualGravity(), 0.01);
    }

Questo test (che forse potrebbe essere reso più chiaro?) dice che se la gemma slave sta cadendo da sola dopo che la compagna pivot si è arenata(drop), non deve essere possibile alterarne la velocità di caduta con il tasto KeyDown.
Eliminando l'if(gemsPair.oneGemIsNotFalling()) il tasto keydown viene processato e la gemma cade ad una velocità diversa, facendo fallire il test.

Il test mi sembra sensato, ma eliminare tutti gli eventi dalla coda non va bene perché brucia i key released, con il rischio di freezare i tasti.
Anche la soluzione di Vicius purtroppo non va bene, se gestiamo i tasti, ci ritroviamo con il problema che il tasto keydown viene processato e ci altera la velocità di caduta della gemma.

Ma perché il tasto keydown ci altera la velocità quando invece destra e sinistra non ci spostano la gemma che cade?
Andiamo a vedere il codice delll'handler destro...

    public void executeWhenPressed(InputReactor inputReactor)
    {
        KeyEventHandler leftHandler = inputReactor.getKeyHandler(KeyEvent.LEFT);

        if(leftHandler.isRepeated(inputReactor.getLastInputTimeStamp())
            || !gemsPair.canReactToInput() )
        {
            return;
        }

        gemsPair.move(GO_RIGHT);
        inputReactor.logEvent(this);
    }


    public void executeWhenReleased(InputReactor inputReactor)
    {
        ;
    }

E quello del tasto giù...

    public void executeWhenPressed(InputReactor inputReactor)
    {
        grid.setStrongerGravity();
        inputReactor.logEvent(this);
    }


    public void executeWhenReleased(InputReactor inputReactor)
    {
        grid.setNormalGravity();
        inputReactor.logEvent(this);
    }

Anche le rotazioni hanno un if (GemsPair.canReactToInput()), il drop è l'unico a non averlo.
Bene, allora direi che anziché svuotare la coda, basta aggiungere un if (GemsPair.canReactToInput()), affinché la pressione del tasto giù non alteri la caduta della gemma.

DropCommandHandler non ha un riferimento alla pair, ma solo a grid....effettivamente il comando drop è più un comando eseguito sulla griglia (aumenta o diminuisce la sua gravità) piuttosto che sulla gemma.
Aggiungo comunque la gemsPair al costruttore, così come hanno gli altri handler. Modifico GridController in modo che gli passi assieme alla grid anche la pair quando lo istanzia.

Metto l'if:

    public void executeWhenPressed(InputReactor inputReactor)
    {
        if(gemsPair.canReactToInput())
        {
            grid.setStrongerGravity();
            inputReactor.logEvent(this);
        }
    }

Tento di lanciare la build...ma ci sono errori in Diamonds..si è rotto TestLog.

    public void testLogStringOnTwoEvents()
    {
        input.notifyKeyEvent(KeyEvent.DOWN, KeyEvent.PRESSED);
        playField.getGridController().reactToInput(environment.getTimer());

        input.notifyKeyEvent(KeyEvent.LEFT, KeyEvent.PRESSED);
        input.notifyKeyEvent(KeyEvent.DOWN, KeyEvent.RELEASED);
        playField.getGridController().reactToInput(environment.getTimer());

        String eventList;

        eventList = new MoveLeftCommandHandler(
            playField.getGridController().getGemsPair()).getClass().getName()
            + " 1\n";
        eventList += new DropCommandHandler(
            playField.getGridController().getGrid()).getClass().getName()
            + " 0\n";

        assertEquals("Logged string must contain left and down events",
            eventList, playField.getGridController().getEventLogString());
    }

Come mai viene istanziato un DropCommandHandler solo per chiedergli il nome della sua classe? Non bastava DropCommandHandler.class.getName() a questo scopo? Gli passo anche la gemspair e lancio i test.

Sorpresa, fallisce un test di TestGridReactionToInput:

    public void testGravityWhileDownKeyIsPressed()
    {
        float multiplied = grid.getActualGravity()
            * environment.getConfig().getInteger("GravityMultiplier");

        generateKeyPressed(KeyEvent.DOWN);
        inputReactor.reactToInput(environment.getTimer());

        assertEquals(multiplied, grid.getActualGravity());

        generateKeyReleased(KeyEvent.DOWN);
        inputReactor.reactToInput(environment.getTimer());
    }

Ovviamente questo test si aspetta che a prescindere dalle gemme, la gravità cambi nella griglia premendo il tasto giù.

E qui mi fermo e vado a cena
Se qualcuno vuole continuare...prego!

[BlueDragon]

Sebbene mi venga qualche dubbio, proviamo a proseguire su questa strada.
Seguiamo la logica per cui le gemme devono venire accelerate solo se stai controllando la tua gempair (canReactToInput = true).
Aggiungiamo quindi la connessione necessaria al test, ossia che ci sia una gemspair in gioco, e trasformiamolo così:

    public void testGravityWhileDownKeyIsPressed()
    {
        grid.insertGem(0,4,gem);
        gemsPair.setPivotGem(gem);
        
        float multiplied = grid.getActualGravity()
            * environment.getConfig().getInteger("GravityMultiplier");

        generateKeyPressed(KeyEvent.DOWN);
        inputReactor.reactToInput(environment.getTimer());

        assertEquals(multiplied, grid.getActualGravity());

        generateKeyReleased(KeyEvent.DOWN);
        inputReactor.reactToInput(environment.getTimer());
    }

Come vedete ho approfittato del fatto che come già notato nella discussione sul bug della Gemma Solitaria, è possibile nei test comandare una gemspair "singola" con la sola gemma Pivot. Non è che mi piaccia molto devo dire...forse sarebbe meglio mettere anche la Slave, ma visto che non è assolutamente necessaria..non ce la metto.

Lanciamo i test, ora questo test passa, ma fallisce ancora testSlaveGemFallsFaster(), questa volta nel suo ultimo assert.

        input.notifyKeyEvent(KeyEvent.DOWN, KeyEvent.RELEASED);
        
        controller.reactToInput(environment.getTimer());

        assertEquals(strongestGravity, grid.getActualGravity(), 0.01);
    }

Infatti quando il tasto keyDown viene rilasciato, il DropCommandHandler cosa fa?

    public void executeWhenReleased(InputReactor inputReactor)
    {
        grid.setNormalGravity();
        inputReactor.logEvent(this);
    }

Il che significa che sei noi rilasciamo il tasto mentre una delle nostre gemme si è staccata e sta cadendo a velocità massima, il rilascio del tasto riporterà la gravità della griglia a normale. Questo non va bene perché la gemma deve continuare a cadere dritta a velocità massima indipendentemente da quello che facciamo con i tasti.

Quindi anche questa azione che il DropCommandHandler effettua, va condizionata al fatto di avere una gemsPair con cui agire...gemsPair.canReactToInput(), come per il pressed event.
Modifichiamo il metodo:

    public void executeWhenReleased(InputReactor inputReactor)
    {
        if(gemsPair.canReactToInput())
        {
            grid.setNormalGravity();
        }
        inputReactor.logEvent(this);
    }

Ho lasciato il log fuori dall'If perché credo che sia comunque interessante loggare il rilascio del tasto. Vi è però un effetto collaterale per cui il fatto che sia stato loggato un rilascio del DropCommand non vuole più dire necessariamente che in quell'istante sia stata portata la gravità della griglia a normale.

Lancio la build....funziona tutto
Lancio il gioco....non si riproduce più il bug

[BlueDragon]

Committato e buildato con successo
Ho dovuto committare due volte perché alla prima mi ero scordato un file..comunque sono stato abbastanza rapido da far avvenire un solo build.
Al secondo commit ho messo "Mancava un file all'ultimo commit " ed ora nella build 920 si vede come Last Log Entry: "Mancava un file all'ultimo commit ".... Non è molto descrittivo come nome per la build... ma basta che funzioni

Se qualcuno si è letto questo thread e vuole commentare...fatevi avanti

[VICIUS]

Ottimo lavoro BlueDragon. Direi che ora non abbiamo più bug e lunedì possiamo rilasciare l'aggiornamento della First Playable.

ciao

[Bonfo]

Complimenti....

...ottima non solo la soluzione , ma anche come hai esposto il problema e la soluziona a tutti. Grazie perchè potevi risparmiartelo...ma così ho capito che è stato e come è ora il codice.

Sono felice che un test che ho proposto e che testava una cosa che già funzionava ha portato a tutta questa bella messa a posto del codice.
Io purtroppo non ho avuto la pazienza di scavare così a fondo per capire dove era la magagna

[thebol]

gg

[fek]

BD, hai un chiarezza logica impressionante. Complimenti

[Vifani]

Complimenti anche da parte mia. Una esposizione veramente notevole.

[Ufo13]

Finito di leggere, davvero bella

[Jocchan]

Meraviglioso. Semplicemente meraviglioso.

[VICIUS]

Bug Closed.