Refactoring Post FirstPlayable

[Bonfo]

Allora...
...pronti a riniziare a lavorare

Io ho iniziato lavorando sugli state.
Sto facendo 3 cose:
1) spostando i metodi in modo che le prime tre cose che si incontrano siano:
- costruttore
- isCurrentState()
- update();
2) ogni stato che si occupa di gravità dovrà occuparsene internamente, ovvero se setta la gravità ad un valore diverso da normal, dopo deve esso stesso rimetterlo a normal
3) diminuire l'istanziazioni di classi state

Ho modificato WaitStateBeforeNewGemsPair.
Ora ritorna:

            return new GemsPairOnControlState(config).update(timer,gridController);

mentre prima ritornava, praticamente, solamente

new GemsPairOnControlState(config);

e tutto funziona.

Invece in CrushState c'è questo:

return new GemFallState(config);

e con

return new GemFallState(config).update(timer, gridController);

invece non passano i test ??

Ora ci riguardo ....

[Jocchan]

Aspettate con i refactoring, per favore.

[Bonfo]

Ottimo refactoring!!!
...ha introdotto un bug che i test non mi hanno segnalato
Ora la gemsPair quando si spezza, se il tasto giù è premuto, non scende a gravità Strongest ma a Normal.

[Jocchan]

Avete fatto commit per caso????

P.S.: Fek spezzerà personalmente le manine al primo che proverà a fare un commit da venerdi a lunedi'

[Bonfo]

Originariamente inviato da Jocchan

Avete fatto commit per caso????

Sì...io ...ma fek ha detto che aveva preso il tag e che si poteva ripartire con i refactoring....

Ora che la First Playable e' chiusa ed ho preso la tag dalla quale verranno prodotte le build da distribuire, il trunk e' di nuovo libero e potere iniziare i refactoring dei quali abbiamo parlato.

http://www.hwupgrade.it/forum/showpo...17&postcount=1


Ho perso le dita???

[Jocchan]

Originariamente inviato da Bonfo

Sì...io ...ma fek ha detto che aveva preso il tag e che si poteva ripartire con i refactoring....



http://www.hwupgrade.it/forum/showpo...17&postcount=1


Ho perso le dita???

Ha preso il tag, ma sono sorti altri problemi che ci conviene provare a risolvere.
Questo weekend doveva essere destinato al playtesting, credo sia sbagliatissimo cercare di accelerare i tempi e crearci da soli altri problemi.

[Bonfo]

Ok...reverto tutto!!!
O almeno ci provo

...fino a lunedì non tocco più nulla

[Jocchan]

Originariamente inviato da Bonfo

Ok...reverto tutto!!!
O almeno ci provo

Aspetta, vediamo prima che dicono anche Fek e Vicius.
E' la mia opinione, ma se hanno detto così un motivo ci sarà.

[Bonfo]

REVERT ESEGUITO.
Fino a lunedì fermo i lavori "PUBBLICI"...

[Bonfo]

Oooops...non aqvevo letto.

in ogni caso mi sono salvato le modifiche che ho fatto...quindi quando vogliamo le rifaccio in 5 sec

Ora vedo di andare avanti con il refactoring sulla mia copia locale ... poi per i play test uso quella della build.

[Jocchan]

Grazie Bonfo (e meno male che hai salvato le modifiche ), vedrò di parlarne con Fek e Vic per cosa ci conviene fare.
Intanto, vediamo di aprire un altro topic per vedere se riusciamo a risolvere questi problemi di compatibilità sorti all'ultimo minuto?
Sicuramente sono più urgenti.

[Bonfo]

Certo...concentriamoci su quelli.

Sugli state però devo dire una cosa.
Ho notato come in crushState si faccia un return new FallGemState senza update. Lo capisco perchè rende difficili moltissimi test.

A questo punto mi viene da dire....o tutto o niente.

Ovvero o tutti fanno un return new STATE.Update() oppure non lo fa nessuno!!!
è una semplice scelta stilistica....tutto funziona uguale...solo bisogna mettere a posto i test di conseguenza.

Capire cosa succede quando faccio update è una delle cose che mi ha sempre fatto spendere un sacco di tempo nei test. Renderlo chiaro sarebbe fantastico....

A questo punto mi verrebbe da dire di togliere gli update dai return....forse diventa tutto un po' più deterministico

[fek]

Colpa mia che ho fatto un po' di confusione, scusate. Ho preso il tag per lasciare il trunk libero per il refactoring. Potete tranquillamente continuare.

Bonfo, scrivi prima il test che copre la condizione che si e' verificata a seguito del tuo refactoring, cosi' da andare sul tranquillo.

[Bonfo]

Ora non posso....sto uscendo.
Appena torno a casa scrivo il test.

Sarebbe utile anche avere un'opinione da parte di tutti per sapere che fare con questi update.

[Bonfo]

Fatto il test...ma anche con la versione corretta non passa.
Ecco il test:

    public void testSlaveGemFallFaster()
    {
        controller.getGemsPair().rotateClockwise();
        controller.getGemsPair().getPivotGem().drop();
        controller.update(timer);
        
        assertEquals(Config.createForTesting().getInteger("StrongestGravityMultiplier") * 0.5, grid.getActualGravity(), 0.01);
                
        input.notifyKeyEvent(KeyEvent.DOWN, KeyEvent.PRESSED);
        
        controller.reactToInput(timer);

        assertEquals(Config.createForTesting().getInteger("StrongestGravityMultiplier") * 0.5, grid.getActualGravity(), 0.01);

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

        assertEquals(Config.createForTesting().getInteger("StrongestGravityMultiplier") * 0.5, grid.getActualGravity(), 0.01);
    }

Il test non passa perchè in realtà sia grid che la slaveGem lo sentono benissimo che la gravità ritorna normale, solo che GemsPairOnControlState prima di far cadere la slaveGem risetta sempre e comunque la gravità a Strongest.

A questo punto o il test è sbagliato "concettualmente" oppure il modo in cui viene fatta cadere la salveGem non è pulitissimo.

Ora ci sono 2 possibiltà nel caso il test sia corretto:
o si spezza lo stato in GemsPairOnControlState in 2 aggiungendo lo stato SlaveGemFall in modo da cambiare questo codice

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

oppure si fanno 2 metodi pubblici di gridController in modo da abilitare e disabilitare gli input e il relativo getter...così gli stati abilitano meno gli input e il metodo suddetto usa il getter per capire che fare.

A me paice molto più la soluzione 1

P.S.: ma dove vengono disabilitati gli input per la salveGem dopo che la gemsPair si è separata??? Dentro gemsPair??

[Bonfo]

Sarà anche
..ma riuppo il thread perchè il test che ho postato è ancora lì che deve capire che fine deve fare.
Ripeto:il comportamento è strano.

In sti giorni ci sarò a sprazzi...ma dopo questo gli state sono abbastanza puliti.
Dopo passerò all'attaco degli action .... e lì si che ci sarà da fare a botte

[fek]

Ufo, puoi dare una mano a Bonfo che tu conosci benone questa parte?

[Ufo13]

Originariamente inviato da fek

Ufo, puoi dare una mano a Bonfo che tu conosci benone questa parte?

Certo, Bonfo contattami su MSN, non ci sono oggi pomeriggio quindi dovremmo fare sul tardi o subito :P

[cisc]

vorrei proporre una forma di organizzazione del refactoring, stile assegnazione del refactoring delle varie parti del codice stile task, però a rotazione, tipo un giorno 1 guarda l'audio, un'altro guarda l'engine, ecc, ecc, il giorno successivo a rotazione, in modo da evitare di lavorare nello stesso momento allo stesso codice....

[fek]

Ok cisc. Scrivi qui l'elenco dei refactoring di cui abbiamo bisogno e li assegnamo.