Refactoring Grid con GridController TDD (cisc vs 71104)

[cisc]

Implementare il pattern Observer in modo essenziale, introducento una classe GridController che deve attendere una notifica da parte di Grid dell'avvenuta collisione, e agire di conseguenza inserendo una nuova Gem.

test list:

- se grid non ha controllore, verificare che non venga aggiunta nessuna gemma dopo una collisione

- aggiungere un controller a grid e controllare che l'aggiunta abbia avuto successo

- verificare che dopo una collisione venga effettuta la notifica al controllore

- verificare che il controllere inserisca la nuova gemma dopo una notifica

primo test:


public void testNoGemUnderController()
{
grid.setGravity(12);
grid.insertGem(13,2,gem);
grid.update();
assertTrue("No Gem must be under control", grid.getGemUnderControl()==gem);
}

[71104]

basta commentare la chiamata a insertNewGem(); all'interno di update e il test passa ;)
a questo punto però la build non è rotta, è fracassata!! :D

[cisc]

la build è fracassata perchè stiamo facendo un bel cambiamento in grid, prox test:


public void testSetController()
{
grid.setController(new GridController());
assertTrue("Grid must be under control", grid.isControlled()) ;
}



a te

[71104]

per farlo passare basta aggiungere:

public boolean isControlled()
{
return true;
}


e poi bisogna anche mettere tutto il necessario per farlo compilare: nuova classe GridController, relativo costruttore, e metodo setController (che non fa nulla ;))

committato.

[cisc]

ok, prossimo test:


public void testNotSetController()
{
assertFalse("Grid must be under control", grid.isControlled()) ;
}

[71104]

public void setController(GridController controller)
{
controlled = true;
}

public boolean isControlled()
{
return controlled;
}

oggi sono bastardo :D

ma in fondo c'è da chiedersi: serve realmente che Grid memorizzi l'istanza del suo controller?? lo sapremo continuando il task ;)

ed ecco il prossimo test:

public void testControllerNotifiedAfterDrop()
{
grid.setController(controller);
grid.insertGem(13, 4, gem);
grid.update();
assertTrue(controller.notified());
}


PS: si, mi sa che era necessario :D

[cisc]

minima modifica, aggiunto questo metodo a GridController:


public boolean notified()
{
return true;
}

[71104]

e se io ci metto questo? :D

public void testControllerNotNotified()
{
grid.setController(controller);
assertFalse(controller.notified());
}

[cisc]

ho dovuto fare un po di modifiche a grid:


private GridController controller;

public void setController(GridController controller)
{
this.controller = controller;
controlled = true;
}



//in update
.
.
//se una gemma è droppata
if (controller != null)
{
controller.notifyDrop();
}
.
.



e in GridController:



public class GridController
{
private boolean notified = false;

public GridController()
{
}

public boolean notified()
{
return notified;
}

public void notifyDrop()
{
notified = true;

}

}




prossimo test:



public void testControllerInsertNewGem()
{
grid.setController(controller);
grid.insertGem(13, 4, gem);
grid.update();
assertFalse("No Gem must be under control", grid.getGemUnderControl() == gem);
}

[71104]

ok, un piccolo refactoring prima di continuare: il flag controlled e l'istanza del controller in grid costituiscono una duplicazione di informazione, percui questo codice

public void setController(GridController controller)
{
this.controller = controller;
controlled = true;
}

public boolean isControlled()
{
return controlled;
}

lo trasformo in quest'altro

public void setController(GridController controller)
{
this.controller = controller;
}

public boolean isControlled()
{
return null != controller;
}

eliminando il flag controlled, e i test passano ancora :)
(tranne quelli che abbiamo rotto :D lol ^^')

[71104]

ho fatto passare il test decommentando insertNewGem e spostandola all'interno dell'if; ora però siccome è un attimino tardino, direi caro cisc di ripristinare la build e andare a nanna ^^

[cisc]

per riprisrinare la build abbiamo adottato una soluzione temporanea che in definitiva non tiene conto delle modifiche che abbiamo effettuato fino ad adesso, a domani 71104;)

[fek]

Ragazzi, mi dispiace interrompervi, ma per favore fate il revert di questi cambiamenti.

Grid non deve sapere nulla dell'esistenza di un oggetto che la controlla. E' un punto importante del design che voglio mantenere.

Al massimo ci deve essere un solo punto di contatto per implementare l'Observer usando un meccanismo di callback.

[71104]

Al massimo ci deve essere un solo punto di contatto per implementare l'Observer usando un meccanismo di callback. hum, ok, np: adesso tolgo GridController e TestGridController.
ma il meccanismo di callback in Java come lo realizziamo? funzioni virtuali?

[71104]

fatto il revert.

[71104]

ma il meccanismo di callback in Java come lo realizziamo? funzioni virtuali? mi rispondo da solo: creaiamo un'interfaccia con un metodo astratto (l'handler appunto); Grid conosce l'interfaccia ma non conosce GridController :)

[fek]

mi rispondo da solo: creaiamo un'interfaccia con un metodo astratto (l'handler appunto); Grid conosce l'interfaccia ma non conosce GridController :)

Ti rispondo io invece, usiamo la soluzione di cionci che e' semplice e banale :)

Mi piace pensarla come una dimostrazione di quanto funzioni avere piu' persone che lavorano su ampie porizioni di codice: io sono partito subito con l'Observer, perche' ero entrato in quell'ordine di idee, ed ho ignorato totalmente possibili soluzioni alternative. Il mio errore e' stato corretto da cionci che guardando il problema da un lato differente ha proposto una soluzione alternativa piu' semplice.

[71104]

io e cisc abbiamo steso la nuova test list:

- verificare che grid non inserisca da sola la nuova gemma
- verificare che GridController.update chiami Grid.update
- verificare che GridController inserisca una nuova gemma
- verificare che GridController faccia tutto questo dopo un timeout
- verificare che il controller catturi l'eccezione lanciata da Grid in caso di game over e disegni lo sprite "Game Over"

sono solo gli acceptance test, sicuramente ce ne saranno altri.
allora cisc, cominciamo?

[71104]

ecco la nuova classe TestGridController:

public class TestGridController extends TestCase
{
private Grid grid;

private Gem gem;

public void setUp()
{
grid = Grid.createForTesting();
gem = Gem.createForTesting();
}

public void testGridDoesntInsertNewGems()
{
grid.insertGem(13, 4, gem);
grid.update();
assertEquals(gem, grid.getGemUnderControl());
}

}

ora, questo primo test è problematico perché è in aperto contrasto con un paio di altri test già esistenti; non c'è modo di farlo passare senza rompere gli altri, e questo perché i precedenti adesso non saranno più validi: le specifiche del programma stanno cambiando e andranno aggiustati.

[fek]

- verificare che il controller catturi l'eccezione lanciata da Grid in caso di game over e disegni lo sprite "Game Over"

Il "Game Over" non va gestito mediante un'eccezione. Non e' un design corretto, le eccezioni vanno usate solo per situazioni eccezionali. Il Game Over e' una situazione perfettamente normale alla fine del gioco.

[fek]

ora, questo primo test è problematico perché è in aperto contrasto con un paio di altri test già esistenti; non c'è modo di farlo passare senza rompere gli altri, e questo perché i precedenti adesso non saranno più validi: le specifiche del programma stanno cambiando e andranno aggiustati.

Elimina pure i test vecchi.

[cisc]

mi è bastato commentare insertNewGem in update di Grid, nuovo test:



public void testGridControllerUpdateGrid()
{
grid.insertGem(13, 4, gem);
GridController controller = new GridController(grid);
controller.update();
assertEquals(14, grid.getGemUnderControl().getCellRow());
}

[fek]

Ragazzi, non rompete la build per favore :)

[71104]

Il "Game Over" non va gestito mediante un'eccezione. Non e' un design corretto, le eccezioni vanno usate solo per situazioni eccezionali. Il Game Over e' una situazione perfettamente normale alla fine del gioco. ti ci stavo giusto aspettando!! :D
hai ragione (quante volte l'avrò scritto negli ultimi due mesi T_T), dunque propongo una soluzione alternativa: l'eccezione viene catturata in insertNewGem, la quale restituisce un booleano che indica se l'inserzione è andata a buon fine; dal momento che insertNewGem non verrà più chiamata da Grid, bensì da GridController (adoro i design perfetti che nascono da soli :D) se GridController riceve false scrive "Game Over" :)
che ne dici?

[71104]

mi è bastato commentare insertNewGem in update di Grid, nuovo test: ok, una piccola puntualizzazione:

public void testGridControllerUpdateGrid()
{
GridController controller = new GridController(grid);
grid.insertGem(13, 4, gem);
controller.update();
assertEquals(14, grid.getGemUnderControl().getCellRow());
}

il controller mi sembra che abbia più senso crearlo prima di inserire la gemma :)

[fek]

ti ci stavo giusto aspettando!! :D
hai ragione (quante volte l'avrò scritto negli ultimi due mesi T_T), dunque propongo una soluzione alternativa: l'eccezione viene catturata in insertNewGem, la quale restituisce un booleano che indica se l'inserzione è andata a buon fine; dal momento che insertNewGem non verrà più chiamata da Grid, bensì da GridController (adoro i design perfetti che nascono da soli :D) se GridController riceve false scrive "Game Over" :)
che ne dici?

Dico che non devi usare eccezioni per comunicare questa condizione :)

Dai che c'e' una soluzione piu' semplice.
Che cos'e' il Game Over? E' la colonna centrale piena, ok?

Quindi, ecco la soluzione in un test:


grid.fillColumWithGems(4);
AssertTrue(grid.IsColumnFull(4));


GridControl testera' prima di inserire una nuova gemma che la colonna non sia piena e se diventa piena scatta il game over.

Ragazzi, la build e' ancora rotta. Mi sto arrabbiando.

[cisc]

vabbè, non vedo la differenza nel crearlo prima o dopo l'inserimento della gemma, non influisce su ciò che si va a testare...........

[71104]

ordunque, per far passare il test ho creato GridController:

public class GridController
{
private Grid grid;

public GridController(Grid grid)
{
this.grid = grid;
}

public void update()
{
grid.update();
}
}

e per riaggiustare la build (OMG, giusto in tempo O.o) ho commentato la riga per la rimozione delle gemUnderControl in un paio di test sulla brightness delle gemme attaccate (li avevamo fatti io e DanieleC88) e ho commentato testNewGemCreation, che non serve più.

[71104]

prossimo test:

public void testNewGemInsertion()
{
GridController controller = new GridController(grid);
grid.insertGem(13, 4, gem);
controller.update();
assertTrue(gem != grid.getGemUnderControl());
}

[71104]

GridControl testera' prima di inserire una nuova gemma che la colonna non sia piena e se diventa piena scatta il game over. e manco lo sapevo io che esisteva un metodo grid.isColumnFull :D
a questo punto secondo me tutto il fatto del Game Over non tocca nemmeno Grid: GridController chiamera isColumnFull prima di fare insertNewGem e se la colonna è piena mostra Game Over.
in Grid non cambia una virgola :p

[cisc]

allora, fatto passare il test con questo codice in GridController:



public void update()
{
grid.update();
if (grid.getGemUnderControl().isNotFalling())
{
grid.insertNewGem();
}
}



prossimo test (dopo aver aggiunto il timer in setUp):



public void testWaitBeforeInsertionNewGem()
{
GridController controller = new GridController(grid);
controller.setDelay(300);
controller.setTimer(timer);
grid.insertGem(13, 4, gem);
controller.update();
assertTrue(gem == grid.getGemUnderControl());
}

[71104]

per risolvere il test ho introdotto un flag all'interno di GridController (ho incollato di sotto la nuova versione della classe).

PS: cisc, quella setTimer secondo me va eliminata: il timer non è meglio passarlo nel costruttore? ;)


public class GridController
{
private Grid grid;

private boolean delayed = false;

public GridController(Grid grid)
{
this.grid = grid;
}

public void update()
{
grid.update();
if (grid.getGemUnderControl().isNotFalling())
{
if (!delayed)
{
grid.insertNewGem();
}
}
}

public void setDelay(int delay)
{
delayed = true;
}

public void setTimer(AbstractTimer timer)
{
}
}

[cionci]

71104: infatti non esiste isColumnFull...
Secondo me basta una funzione isGameOver in gridController che viene chiamata quando una gemma controllata isStopped... La funzione controlla la posizione della gemma che si è fermata, se la posizione è (0, 4) allora mostra il "game over"...

[fek]

e manco lo sapevo io che esisteva un metodo grid.isColumnFull :D
a questo punto secondo me tutto il fatto del Game Over non tocca nemmeno Grid: GridController chiamera isColumnFull prima di fare insertNewGem e se la colonna è piena mostra Game Over.
in Grid non cambia una virgola :p

Infatti non esiste, lo devi scrivere :)

[71104]

ho refattorizzato un po': la setTimer non serviva realmente, quindi l'ho tolta: il timer viene passato dal costruttore (nei test usiamo il mock).
ecco il nuovo test, per ora commentato:

public void testGemInsertedAfterTimeout()
{
controller.setDelay(300);
grid.insertGem(13, 4, gem);
timer.setTime(300);
controller.update();
assertTrue(gem != grid.getGemUnderControl());
}

fallisce correttamente :p

maro', che stanchezza B-|

cionci, grazie per la spiegazione (e fesso io che interpreto quello che scrive fek alla lettera :D)

edit: WOW (non Windows Over Windows, era un'esclamazione di stupore :p), questo è il mio post numero 2000!!! :yeah: :yeah:
qui le date in omaggio le gif animate per i membri assatanati? :sofico:
sul Night Sun Network una volta lo facevamo LOOOL :rotfl:
bei tempi, allora ci chiamavamo ancora "Destruction Carriers" se ben ricordo...

[cisc]

modifiche apportate a GridController:



public class GridController
{
private Grid grid;

private int delay = 0;

private AbstractTimer timer;

public GridController(Grid grid, AbstractTimer timer)
{
this.grid = grid;
this.timer = timer;
}

public void update()
{
grid.update();
if (grid.getGemUnderControl().isNotFalling())
{
if (delay == 0)
{
grid.insertNewGem();
}
else
{
if (timer.getTime()>= delay)
{
grid.insertNewGem();
}
}

}
}

public void setDelay(int delay)
{
this.delay = delay;
}
}



prossimo test:


public void testWaitBeforeInsertionNewGemWithTimeBaseNotZero()
{
controller.setDelay(300);
grid.insertGem(13, 4, gem);
timer.setTime(150);
controller.update();
timer.advance(150);
controller.update();
assertEquals(gem, grid.getGemUnderControl());
}

[71104]

CISC!!! che cos'è questo OBBROBBRIO??? :mad:

public void update()
{
grid.update();
if (grid.getGemUnderControl().isNotFalling())
{
if (delay == 0)
{
grid.insertNewGem();
}
else
{
if (timer.getTime()>= delay)
{
grid.insertNewGem();
}
}

}
}

ti rendi conto che ha complessità 4 quando potrebbe avere complessità 3??? :D

meglio farlo così ;)

public void update()
{
grid.update();
if (grid.getGemUnderControl().isNotFalling())
{
if (timer.getTime()>= delay)
{
grid.insertNewGem();
}
}
}

che guarda che i test passano ancora tutti!! ;)
e se questo codice non va bene sai cosa vuol dire? che non abbiamo abbastanza test da documentare lo scopo del task. (ma a me cmq sembra che vada bene...)

[cisc]

mamma mia, stavo giusto fermandoti in chat per farmi fare il refactoring!!!!!

avevo scritto troppa roba, che poi ho cancellato, per fare un passo più piccolo, solo che ho cancellato male:D

[cisc]

aaa, il tuo test l'ho modificato così:


public void testGemInsertedAfterTimeout()
{
controller.setDelay(300);
grid.insertGem(13, 4, gem);
controller.update();
timer.setTime(301);
controller.update();
assertTrue(gem != grid.getGemUnderControl());
}


passava anche prima, ma come logica è più corretto così

[71104]

hanf, hanf, hanf... questo test mi ha fatto sputare sangue :|
per un motivo idiota passava ma ne falliva un altro...
vabbè, ecco la soluzione:

private long timeBase = -1;

.
.
.

public void update()
{
grid.update();
if (grid.getGemUnderControl().isNotFalling())
{
if (-1 == timeBase)
{
timeBase = timer.getTime();
}
if (timer.getTime() - timeBase >= delay)
{
grid.insertNewGem();
}
}
}

[71104]

detto questo dovremmo fare quello che era il mio task, solo che cisc si è disconnesso... ha detto che tornava ma l'ha eoni fa :D
non vorrei farlo io da solo perché il suo task l'abbiamo fatto insieme (anzi, l'ho fatto io... -_-') e quindi mi sembrava giusto che il mio lo implementasse lui coi test fatti da me...
vabbè, mentre aspetto che torna ho un refactoring da fare, anzi ben due:
1) tutto questo popodiché di controller è tanto fuffoso quanto inutile se Game non lo istanzia e non lo usa
2) come la mettiamo col parametro AbstractTimer di Grid.update?? :|
l'abbiamo ignorato, mi sa che quando avrò usato il controller all'interno di Game la temporizzazione funzionerà a fallo di canide, dovrò metterci un test e correggere il bug

[71104]

ebbene... il parametro timer di Grid.update non viene mai usato, e quindi funziona tutto :|
bella pe' me :Prrr:
lo rimuovo :p

[71104]

ho refattorizzato Game, la quale adesso utilizza il controller, aggiungendo un semplice metodo createController(), un campo privato GridController, e sostituendo nel game loop grid.update() con controller.update().

adesso però mi sono accorto di un problema di GridController, perciò scrivo un test e cerco di correggerlo (se ci fosse cisc poi sarebbe meraviglioso... -.-')

[cionci]

Non viene mai usato timer ? Dovrebbe servire per fare l'update delle gemme... Non puoi passargli un MockTimer...

Anche Gridcontroller.update dovrebbe avere una parametro timer da passare a Grid.update...

[fek]

ho refattorizzato Game, la quale adesso utilizza il controller, aggiungendo un semplice metodo createController(), un campo privato GridController, e sostituendo nel game loop grid.update() con controller.update().


Bene :)

Ora correggi sta porcata che metto in grassetto:

public void update()
{
grid.update();
if (grid.getGemUnderControl().isNotFalling())
{
if (-1 == timeBase)
{
timeBase = timer.getTime();
}
if (timer.getTime() - timeBase >= delay)
{
grid.insertNewGem();
}
}
}


"-1 == timeBase" non mi comunica proprio che cosa intende, quel -1 mi sa tanto di costante messa li' ad hoc, e puzza. Puzza uguale refactoring. Al lavoro sfaticato! :)

[71104]

"-1 == timeBase" non mi comunica proprio che cosa intende, quel -1 mi sa tanto di costante messa li' ad hoc, e puzza. be', ecco, asd, inizialmente usavo un flag settato a false al posto del valore -1 e settato a true al posto del valore diverso da -1, ma siccome mi sembrava una duplicazione (visto che la variabile long la dovevo usare lo stesso) ho fatto sta cosa del valore strano ^^

rimetto il flag :|

*prego non so quale santo che cisc mi semplifichi il metodo :|*

Puzza uguale refactoring. Al lavoro sfaticato! :) :cry:

scusa, sarà la stanchezza ma... cos'ha che non va il refactoring...? O.o'

[71104]

Non viene mai usato timer ? Dovrebbe servire per fare l'update delle gemme... Non puoi passargli un MockTimer... boh, forse mi sbaglio io, controlla tu...

Anche Gridcontroller.update dovrebbe avere una parametro timer da passare a Grid.update... ce l'ha: gli viene dal costruttore ;)

[fek]

scusa, sarà la stanchezza ma... cos'ha che non va il refactoring...? O.o'

Il vostro refactoring va benissimo, siete stati molto bravi :)

Intendevo dire che c'e' uno 'smell', e quando c'e' uno 'smell' ci vuole un refactoring per eliminarlo. Era una battuta.

[71104]

allora, prossimo test: questo test evidenzia il problema che ho notato avviando il gioco, e cioè che il delay nell'inserzione della nuova gemma funziona solo la prima volta. non so se il test abbia molto senso, dopo 4 ore di lavoro ho il cervello in condizioni penose, cisc aiutami tu... :cry:

public void testAfterSecondDelay()
{
controller.setDelay(300);
grid.insertGem(13, 4, gem);
controller.update();
timer.advance(300);
controller.update();
gem = grid.getGemUnderControl();
timer.advance(300);
controller.update();
assertNotSame(gem, grid.getGemUnderControl());
}

[71104]

Il vostro refactoring va benissimo, siete stati molto bravi :)

Intendevo dire che c'e' uno 'smell', e quando c'e' uno 'smell' ci vuole un refactoring per eliminarlo. Era una battuta. aaaahhh LOL avevo capito "il refactoring puzza uguale" riferito al mini-refactoring che ho fatto in game :D

[fek]

allora, prossimo test: questo test evidenzia il problema che ho notato avviando il gioco, e cioè che il delay nell'inserzione della nuova gemma funziona solo la prima volta. non so se il test abbia molto senso, dopo 4 ore di lavoro ho il cervello in condizioni penose, cisc aiutami tu... :cry:

Lascia stare, continui domani. Preferisco che scrivi il codice fresco e non con la testa in pappa :)

[71104]

Lascia stare, continui domani. Preferisco che scrivi il codice fresco e non con la testa in pappa :) non possiamo, cisc dice che domani non c'è, però mi ha dato un OTTIMO suggerimento: vado a cenare :D
lui l'ha già fatto e si è un attimino ripreso :p
e poi dovremmo aver quasi finito: manca solo il game over (credo)

PS: mi sa che solo col mio pulse di oggi su WhatPulse il Night Sun Network arriva prima in Italia ^^'
www.whatpulse.org

[cionci]

boh, forse mi sbaglio io, controlla tu...
for(int y = grid.length - 1; y >= 0; y--)
{
for(int x = 0; x < grid[y].length; x++)
{
Gem gem = getGemAt(y, x);
if (gem != null)
{
gem.update(timer);
}
}
}
;)

[cionci]

e poi dovremmo aver quasi finito: manca solo il game over (credo)
Mmmmhhh...

- test che genera due gemme (cioè quando la prima raggiunge il fondo, la seconda viene generata) e che verifichi che entrambe rispettino il timer
- test che verifica che l'input sia disattivato durante l'attesa

[cisc]

allora, il test di 71104 l'ho modificato così:



public void testAfterSecondDelay()
{
controller.setDelay(300);
grid.insertGem(13, 4, gem);
controller.update(timer);
timer.advance(300);
controller.update(timer);
gem = grid.getGemUnderControl();
grid.setGravity(32);
for (int i=0; i<12; i++)
{
controller.update(timer);
}
controller.update(timer);
timer.advance(300);
controller.update(timer);
assertTrue(gem!= grid.getGemUnderControl());
}


quel for non mi piace, per adesso funziona, se qualcuno ha una soluzione + elegante, ben venga;


La mia modifica:


public void update(AbstractTimer timer)
{
grid.update(timer);

if (grid.getGemUnderControl().isNotFalling())
{
if (!timeBaseSet)
{
timeBase = timer.getTime();
timeBaseSet = true;
}
if (timer.getTime() - timeBase >= delay)
{
grid.insertNewGem();
timeBaseSet = false;
}

}


cionci, ho reintrodotto il timer nella chiamata ad update di grid, adesso preparo il prossimo test, relativo alla gestione dell'input durante il delay

[cionci]

cionci, ho reintrodotto il timer nella chiamata ad update di grid, adesso preparo il prossimo test, relativo alla gestione dell'input durante il delay
Per fare questo potrebbe anche essere necessario modificare inputReactor...

[cisc]

fek mi ha suggerito di metterlo in GridController, mi sto apprestando a farlo, cmq posta come risolveresti tu il problema, è sempre meglio avere diversi pareri (come con l'observer:D)

[71104]

quel for non mi piace, per adesso funziona, se qualcuno ha una soluzione + elegante, ben venga; potremmo usare Gem.dropGem(), solo che non capisco per quale motivo non è stata esplicitata la sua visibilità... secondo me dropGem deve essere public perché Grid lo usa, quindi potrei dichiarare dropGem come public e usarlo nel test.

[cionci]

Chiaramente richiederebbe modifiche da entrambe le parti...ma è comunque necessario che gli input registrati vengano eliminati dalla coda durante l'attesa (cioè mentre gemUnderControl == null)...

[cionci]

potremmo usare Gem.dropGem(), solo che non capisco per quale motivo non è stata esplicitata la sua visibilità... secondo me dropGem deve essere public perché Grid lo usa, quindi potrei dichiarare dropGem come public e usarlo nel test.
Oppure mettete la gravità molto alta e fate un update...

[cionci]

ma è comunque necessario che gli input registrati vengano eliminati dalla coda durante l'attesa (cioè mentre gemUnderControl == null)...
Ovviamente IMHO...

[cover]

Chiaramente richiederebbe modifiche da entrambe le parti...ma è comunque necessario che gli input registrati vengano eliminati dalla coda durante l'attesa (cioè mentre gemUnderControl == null)...

Penso che sia d'obbligo e una motivazione è questa(non sò se dipende dal fatto che non vengono cancellati gli input, ma penso di sì..): http://www.hwupgrade.it/forum/showpost.php?p=10561119&postcount=386
Voi che dite?

[Jocchan]

Oppure mettete la gravità molto alta e fate un update...

Vorrei committare GameConfig con GravityMultiplier a 8, invece di 22.
Posso farlo o rischio di inciampare in qualche vostro commit contemporaneo?
In tal caso, per favore, può modificare questo valore uno di voi, così tagliamo la testa al toro? :sofico:
Grazie ;)

[71104]

Oppure mettete la gravità molto alta e fate un update... non conviene pià usarlo quel metodo imho: da quando abbiamo eliminato Bounds non esiste più il meccanismo del clipping, quindi la gemma non se ne va perfettamente a terra: o resta in aria o va fuori dalla griglia, non so bene; in entrambi i casi cmq è un metodo che non mi piace :Prrr:
e poi non ignoriamo il problema di dropGem: la visibilità di un metodo secondo me andrebbe sempre esplicitata, lì qualcuno se l'è scordata.

[Ufo13]

hmmm sono un pochino di fretta quindi non posso leggere tutto... Comunque la gemma resta controllabile dopo che ha toccato il fondo...

[cisc]

allora, con 71104 ci siamo divisi il lavoro, io mi sono occupato della gestione dell'input durante l'attesa della creazione di una nuova gemma, ecco i tests (fatti inTDD):



public void testInputWhileWaiting()
{

controller.setDelay(300);
grid.insertGem(13, 4, gem);
controller.update(timer);
input.generateKey(Input.KeyCode.vk_Left);
controller.reactToInput();
controller.update(timer);
assertTrue(grid.isGemAt(13,4));
}

public void testInputAfterWaiting()
{

controller.setDelay(300);
grid.insertGem(13, 4, gem);
controller.update(timer);
timer.setTime(300);
controller.update(timer);
input.generateKey(Input.KeyCode.vk_Left);
controller.reactToInput();
assertTrue(grid.isGemAt(0,2));
}

public void testEmptyQueueBeforeGenerateNewGem()
{
controller.setDelay(300);
grid.insertGem(13, 4, gem);
controller.update(timer);
input.generateKey(Input.KeyCode.vk_Left);
timer.setTime(300);
controller.update(timer);
assertTrue(input.isEmpty());
}

public void testNormalGravityAfterNewGem()
{
controller.setDelay(300);
grid.setGravity(10);
grid.insertGem(13, 4, gem);
input.generateKey(Input.KeyCode.vk_Down);
controller.reactToInput();
controller.update(timer);
timer.advance(300);
controller.update(timer);
gem.dropGem();
controller.update(timer);
timer.advance(300);
controller.update(timer);
assertEquals (10f,grid.getActualGravity(),0.01);
}

//ho aggiunto inoltre il seguente test per InputReactor

public void testEmptyInputQueuefromInputReactor()
{
input.generateKeyEvent(KeyCode.vk_Left, KeyState.Pressed, timer.getTime());
inputReactor.emptyQueue();
assertTrue(input.isEmpty());
}




ecco GridController dopo le modifiche:



public class GridController
{
private Grid grid;

private int delay = 0;

private boolean waitState = false;
private long timeBase = -1;

private InputReactor inputReactor;

public GridController(Grid grid, InputReactor inputReactor)
{
this.grid = grid;
this.inputReactor = inputReactor;
}

public void update(AbstractTimer timer)
{
grid.update(timer);

if (grid.getGemUnderControl().isNotFalling())
{
if (!waitState)
{
timeBase = timer.getTime();
waitState = true;
}
if (timer.getTime() - timeBase >= delay)
{
inputReactor.emptyQueue();
grid.setNormalGravity();
grid.insertNewGem();
waitState = false;
}

}
}

public void setDelay(int delay)
{
this.delay = delay;
}

public void reactToInput()
{
if (!waitState)
{
inputReactor.reactToInput(grid);
}
}
}



ho aggiunto inoltre il metodo getActualGravity() a Grid

[cionci]

Io sinceramente toglierei reactToInput da GridController... reactToInput per Grid deve rimanere solo in InputReactor...
Inoltre questa soluzione comunque non risolve il problema della registrazione dei tasti premuti...

Ad esempio attualmente questo test dovrebbe fallire:


public void testInputWhileWaiting()
{
controller.setDelay(300);

grid.insertGem(13, 4, gem);

input.generateKey(Input.KeyCode.vk_Left);
controller.update(timer);
controller.reactToInput();

timer.advance(301);
controller.update();
controller.reactToInput();

assertTrue(grid.isGemAt(13,4));
}

[cionci]

Al limite InputReactor e GridController potrebbero anche essere fuse insieme...ma non so se conviene...

[cisc]

scusami, perchè dovrebbe fallire, tu chiami reactToInput dopo update, cioè nel momento in cui la gemma è dropped e stiamo attendendo il delay prima di creare la nuova, quindi l'evento pressione tasto è visibile mentre siamo in attesa, ovvero quando non questo evento deve essere ignorato, ed infatto la gemma resta dov'è

[cionci]

Ma la seconda volta che chiamo reactToInput la gemma non dovrebbe essere nuovamente libera perchè è passato il delay ?

[cionci]

Ah...ho visto ora che chiamate emptyQueue... allora passa, ma sinceramente io non mischierei la gestione dei tasti con la gestione di grid (dal punto di vista delle gemme)...

Se durante l'attesa settate a null la gemma sotto controllo dopo basta da Input.reactToInput scrivere:

if(getGemUnderControl() == null)
inputQueue.emptyQueue();

che in effetti è anche un controllo utile da porre in reactToInput...

[cisc]

effettivamente la tua soluzione semplifica il tutto, evitando anche di mettere InputReactor in GridController...

[fek]

Ma bravissimi tutti. Mi sta piacendo molto come vi comunicate le possibili soluzioni per mezzo dei test.

Mi sto sempre piu' convincendo che se vi prendessi tutti in blocco, finiremmo il motore di fable2 nella meta' del tempo...

[71104]

Mi sto sempre piu' convincendo che se vi prendessi tutti in blocco, finiremmo il motore di fable2 nella meta' del tempo... ottimo, quand'è che ci assumi alla Lionhead?? :D

[71104]

ecco i test che ho appena scritto per il mio task (quello dello sprite "Game Over"):

public void testGridNotGameOver()
{
assertFalse(grid.isGameOver());
}

public void testGridGameOver()
{
grid.insertGem(0, 4, gem);
gem.dropGem();
assertTrue(grid.isGameOver());
}

public void testControllerGameOver()
{
try
{
grid.insertGem(0, 4, gem);
gem.dropGem();
controller.update(timer);
timer.advance(300);
controller.update(timer);
}
catch(Exception e)
{
fail("GridController doesn't manage \"game over\" conditions");
}
}

[71104]

consigliandomi con fek, ho cambiato i test: per testare la situazione "game over" riempio l'intera colonna con un metodo fillColumn presente in TestGridController.

private void fillColumn(int column)
{
for (int i = 0; i < 14; i++)
{
Gem gem = Gem.createForTesting();
grid.insertGem(i, column, gem);
gem.dropGem();
}
}

.
.
.

public void testGridNotGameOver()
{
assertFalse(grid.isColumnFull(4));
}

public void testGridGameOver()
{
fillColumn(4);
assertTrue(grid.isColumnFull(4));
}

public void testControllerGameOver()
{
try
{
fillColumn(4);
controller.update(timer);
timer.advance(300);
controller.update(timer);
}
catch(Exception e)
{
fail("GridController doesn't manage \"game over\" conditions");
}
}

[cisc]

allora, dopo una bella dormita, si hanno le idee più chiare, io lascerei InputReactor in GridController, ma cmq adotterei anche la soluzione di cionci di mettere la gemma sotto controllo a null durante l'attesa, perchè logicamente è più corretto, non non c'è nessuna gemma sotto controllo durante l'attesa, il problema è che fino a lunedì non posso fare nulla, se qualcuno è disposto a fare questo refactoring, è meglio;)

[fek]

ma cmq adotterei anche la soluzione di cionci di mettere la gemma sotto controllo a null durante l'attesa, perchè logicamente è più corretto, non non c'è nessuna gemma sotto controllo durante l'attesa,

E' una buona soluzione.