PDA

View Full Version : [CICLO 12] Storia 1


VICIUS
06-03-2006, 15:26
Questa è una di refactoring, bugfix e aggiornamento. Come al solito prima di cominciare qualcosa annunciatelo sul forum e cercate di essere il più chiari possibili magari citando le classi che andrete a toccare.

Bugs:
1. Il layer delle gemme copre quello in cui viene mostrato la png delle combo Ufo13: completato
2. Le png delle combo vengono nascoste appena viene creata una nuova gemspair invece che essere mostrati per un tempo fisso
3. La pausa tra un ciclo di crush e l'altro viene onorata solo tra il primo e il secondo ciclo. thebol
4. Quando si preme il tasto ESC il gioco non esce.
5. Su linux OpenAL usa OSS invece di usare ALSA sul dispositivo default.
...

Refactoring:
1. Portare ad abstract GridTestCase. Togliere le getter. Sistemare i test secondi i cambiamenti della classe. VICIUS: completato
2. Eliminare tutte le Gem.diamond(3500) dai test e usare una createGem(DIAMOND). estendere GridTestCase dovrebbe bastare. VICIUS: completato
3. Togliere CrushBox e WarningBox da Grid ^TiGeRShArK^ + dnarod:completato
...

Varie:
1. Controllare se la versione 0.99 di Lwjgl risolve i problemi di stabilità su linux. VICIUS: completato
2. Provare a modificare il file di configurazione di eclipse per impedire che obj.funzione() venga spezzato. thebol: completato

La storia normale con i nuovi task da implementare sarà postata appena finiti tutti i refactoring.

ciao ;)

fek
06-03-2006, 16:28
Ottimo. Al lavoro che siamo vicini alla First Playable :)

Ufo13
06-03-2006, 16:37
Mi occupo del primo Bug. Tempo previsto 2 giorni. Posterò le previsioni il prima possibile :)

VICIUS
06-03-2006, 17:09
Mi son preso i due refacotring che ce li ho mezzi finiti nel mio tree.
Se avete altre idee postate pure che le aggiungiamo.

ciao ;)

thebol
06-03-2006, 17:48
Prendo il bug 3

Jocchan
06-03-2006, 17:59
Ottimo, così vi vogliamo. Famelici e spietati :sofico:

Ufo13
06-03-2006, 18:04
proporrei come refactoring:

Tirare fuori WarningBox da Grid

fek
06-03-2006, 18:09
proporrei come refactoring:

Tirare fuori WarningBox da Grid

Assolutamente si'. Non ci azzecca proprio nulla.

Ufo13
06-03-2006, 18:15
e stessa cosa per CrushBox direi

^TiGeRShArK^
06-03-2006, 18:49
Prenotazione per il task 3 di refactoring.
Io e dnarod in pair refactoring fisico da casa mia :p

fek
06-03-2006, 19:00
Mi piace di piu' quando cannibalizzate i task :)

thebol
06-03-2006, 21:27
risolto il bug(un assegnazione sbagliata...)

ecco il test
public void testCrushesDelay()
{
int delayCrush = getConfig().getInteger("DelayBetweenCrushes");

insertAndUpdate(createGem(EMERALD), 13, 2);
insertAndUpdate(createGem(DIAMOND), 12, 2);
insertAndUpdate(createGem(EMERALD), 11, 2);
insertAndUpdate(createGem(DIAMOND), 10, 2);
insertAndUpdate(createGem(DIAMOND_CHEST), 9, 2);
insertAndUpdate(createGem(EMERALD_CHEST), 8, 2);
insertAndUpdate(createGem(DIAMOND_CHEST), 7, 2);
insertAndUpdate(createGem(EMERALD_CHEST), 6, 2);

insertAndStopGemsPair();
makeAllGemsFall();

getController().update(getTimer());


makeAllGemsFall();
oneStepForward();


for (int i=1; i<delayCrush; i++)
{
checkCrush("at iteration:"+ i, 8, 1, 1);
oneStepForward();
}

checkCrush("second crush:",6,1,2);

makeAllGemsFall();

for (int i=0; i<delayCrush; i++)
{
checkCrush("at iteration:"+ i, 6, 1, 2);
oneStepForward();
}

checkCrush("third crush:", 4, 1, 3);

makeAllGemsFall();

for (int i=0; i<delayCrush; i++)
{
checkCrush("at iteration:"+ i, 4, 1, 3);
oneStepForward();
}

checkCrush("final update:", 2, 1, 4);



}


private void checkCrush(String string, int numberOfGems, int crushedGemsCounter, int chainCounter)
{
assertEquals(string, numberOfGems, getGrid().getNumberOfGems());
assertEquals(string, crushedGemsCounter, getGrid().getCrushedGemsCounter());
assertEquals(string, chainCounter, getGrid().getChainCounter());
}


private void oneStepForward()
{
getTimer().advance(1);
getController().update(getTimer());
}

ho dovuto introdurre 2 nuove funzioni se no checkstyle strippava sull'ncss (era 41, e voleva max 30...)

thebol
06-03-2006, 21:55
Ho esaminato il bug 2, conviene aspettare che venga tolta crushBox da grid(e magari spostato in gridController) per poi implementare il delay(ora come ora lo si potrebbe introdurre solo in grid.updateGemAnimations(), l'unica funzione di grid chiamata a ogni gridController.update())

ps.va introdotto un parametro di configurazione per quel delay

^TiGeRShArK^
06-03-2006, 21:59
sono dnarod, preso sotto la sua ala....mi sta aprendo la mente :O ....diciamo che ora sto quasi capendo quello che mi imbocca pazientemente.... mi suggeriscono dalla regia che "siamo a buon punto", diciamo che concordo.........se tutto va bene (benissimo) magari si finisce di stasera

^TiGeRShArK^
06-03-2006, 23:05
son sempre rod....oppure non sono piu io....mah non so, il mio cervello è prossimo all overflow, quindi non so piu nulla...siamo sulla retta via, ma non abbiamo ancora finito (complicissima la mia incapacita oggettiva, anche se spero di apprendere in fretta); sry per il ritardo, speriamo di terminare domani, io mo torno a casuccia e ripasso la parola al legittimo proprietario di questo pc...ciaoz...

fek
07-03-2006, 09:36
[/code]

ho dovuto introdurre 2 nuove funzioni se no checkstyle strippava sull'ncss (era 41, e voleva max 30...)

Ottimo!

30 e' ancora alto, puoi ridurre ulteriormente la complessita'? Mi posti il test che esercita il bug?

VICIUS
07-03-2006, 10:02
Dunque ieri ho fatto alcune prove con la nuova versione di Lwjgl. Devo dire che sono rimasto piuttosto soddisfatto. Niente più crash assurdi. Inoltre hanno risolto il problema con i nomi delle librerie che ci costringeva ad avere su .so uguali con nomi diversi.

Prima di aggiornare i file sul repository però vorrei una conferma da utenti windows e osx. Potete provare la nuova versione e vedere se avete problemi ?

ciao ;)

VICIUS
07-03-2006, 11:03
Per quanto riguarda i primi due refactoring c'è un problema. Nel mio tree funziona tutto e checkstyle non dice niente. Come porto la patch ad un checkout liscio ecco che comincia a dare errore sugli attributi. Ieri sera tra una publicità e l'altra ho cercato di capire cosa ci fosse di diverso ma le due cartelle sembravano identiche bit-a-bit.

Ora ho trovato un modo per disabilitare checkstyle in quella esatta porzione di codice tramite due commenti. Prima di procedere a sistemare la patch e fare il commit volevo un via libera anche da voi.

ciao ;)

fek
07-03-2006, 11:14
Bene, appena abbiamo l'ok anche dagl'altri possiamo passare al nuovo lwgl visto che sembra piu' stabile.

thebol
07-03-2006, 18:20
Ottimo!

30 e' ancora alto, puoi ridurre ulteriormente la complessita'? Mi posti il test che esercita il bug?

mi sa che cè stata un incomprensione =D

era il test (postato nel precedente post) che superava nella prima versione il valore di 40, e ho aggiunto 2 funzioni(riportate sempre nel precedente post).

^TiGeRShArK^
07-03-2006, 19:50
ehm...problemino...
ho il cervello in fumo oggi..
se mi metto a finire il refactoring di ieri riskio di fare + danni ke altro...
vedo se mi riprendo un pò + tardi altrimenti passa a domani sera.. :(

VICIUS
07-03-2006, 19:56
Vi do tempo fino a domani sera per fermarmi :p. Poi procedo al commit delle nuove librerie e del refactoring.

ciao ;)

Ufo13
07-03-2006, 20:35
l'introduzione delle nuove librerie richiede un refactoring del codice?

Non è semplicemente un aggiornamento a livello di libreria? Cambia anche l'interfaccia?

^TiGeRShArK^
07-03-2006, 21:17
OT ma x caso spartacus è down???
ho provato a fare un update ma mi da errore..
potreste verificare per favore xkè ogni tnt mi da problemi la connessione con SVN???

Jocchan
07-03-2006, 21:40
OT ma x caso spartacus è down???
ho provato a fare un update ma mi da errore..
potreste verificare per favore xkè ogni tnt mi da problemi la connessione con SVN???

Ora è up ;)

^TiGeRShArK^
07-03-2006, 21:50
ok tnx...
vedo ke riesco a fare stasera :D

VICIUS
08-03-2006, 00:40
l'introduzione delle nuove librerie richiede un refactoring del codice?

Non è semplicemente un aggiornamento a livello di libreria? Cambia anche l'interfaccia?
Il refactoring non è legato al aggiornamento della libreria.

ciao ;)

Ufo13
08-03-2006, 08:01
public void testCorrectlyDrawn()
{
MockEngine engine = MockEngine.createForTesting(800, 600);

crushBox = CrushBox.create(2, 20, 20);
crushBox.draw(engine);

assertEquals("Number of crushes not correctly drawn", 1, engine.getNumberOfQuadsDrawn());
}

Stavo guardando ora...

L'unico test per la draw di CrushBox è questo!

Quindi se cambio posizione della crushbox all'interno del codice non fallisce nessun test! Chiaramente questo codice non è scritto in test driven... Ricordiamoci che non basta testare che venga disegnato un quad ma bisogna controllare posizione, rectangle e tutto il resto :)

Ufo13
08-03-2006, 08:02
public void testCorrectlyDrawn()
{
MockEngine engine = MockEngine.createForTesting(800, 600);

crushBox = CrushBox.create(2, 20, 20);
crushBox.draw(engine);

assertEquals("Number of crushes not correctly drawn", 1, engine.getNumberOfQuadsDrawn());
}

Stavo guardando ora...

L'unico test per la draw di CrushBox è questo!

Quindi se cambio posizione della crushbox all'interno del codice non fallisce nessun test! Chiaramente questo codice non è scritto in test driven... Ricordiamoci che non basta testare che venga disegnato un quad ma bisogna controllare posizione, rectangle e tutto il resto :)

Aggiungo i test mancanti :)

^TiGeRShArK^
08-03-2006, 08:41
se non mi sto confondendo testPosition c'era......
ma cmq se non tocchi la classe testCrushBox è meglio che mi sballi tutto poichè estraendo la crushBox da Grid non si può testare nel modo in cui veniva testato prima :p
infatti ho quasi finito il lavoro di estrazione....
devo solo riuscire a capire come cavolo fare a testare il crushBox da gridController anzikè da Grid...
il problema è che quando faccio gridController.update() non riesco a fare entrare grid nel metodo updateCrushes() perchè il flusso del programma prende una strada diversa...:doh:
qualcuno sa come fare a forzare questo metodo???
Non lo posso richiamare direttamente perchè ora la logica di gestione di crushBox è in gridController, come richiesto, ma se non mi prende la strada giusta non chiamerà mai grid.UpdateCrushes al suo interno e quindi non aggiornerà mai il valore di crushCounter facendo fallire tutti i test relativi al CrushBox()...

qualche idea???

Ufo13
08-03-2006, 08:48
se non mi sto confondendo testPosition c'era......
ma cmq se non tocchi la classe testCrushBox è meglio che mi sballi tutto poichè estraendo la crushBox da Grid non si può testare nel modo in cui veniva testato prima :p
infatti ho quasi finito il lavoro di estrazione....
devo solo riuscire a capire come cavolo fare a testare il crushBox da gridController anzikè da Grid...
il problema è che quando faccio gridController.update() non riesco a fare entrare grid nel metodo updateCrushes() perchè il flusso del programma prende una strada diversa...:doh:
qualcuno sa come fare a forzare questo metodo???
Non lo posso richiamare direttamente perchè ora la logica di gestione di crushBox è in gridController, come richiesto, ma se non mi prende la strada giusta non chiamerà mai grid.UpdateCrushes al suo interno e quindi non aggiornerà mai il valore di crushCounter facendo fallire tutti i test relativi al CrushBox()...

qualche idea???


Più test ci sono meglio è (anche perchè i test dipendono solo da CrushBox e non da dove si trova).

So che la via più facile per estrarre CrushBox è infilarla in GridController ma è inutile saremmo da capo... Va tirata fuori completamente dal concetto di Griglia... Per me andrebbe in PlayField!

Mancano i test che vedono dove viene posizionata per player1 e player2... ci pensi tu alla fine? Io aspetto che finisci per iniziare il mio bugfix :)

fek
08-03-2006, 09:35
qualche idea???

Mi sembra una situazione ideale per un Observer. Grid o GridController possono pubblicare gli eventi relativi ai Crush ad esempio (perche' non anche relativi alla fine della partita?) e poi invocare un metodo Listener, in Java sarebbe una classe, che e' eseguito al momento del verificarsi dell'evento.

PlayField puo' registrare un suo Listener per l'evento "Update Crushes", mettersi in ascolto ed eseguire il codice necessario per aggiornare il warning box di conseguenza.

E' un'idea di soluzione, ma voglio che prima di imbarcarcisi sia valutato il suo impatto sul codice, perche' se lo complica l'Observer non deve essere implentato. Lo si implementa solo in caso sia un'effettiva semplificazione, e magari si porta dietro un'elegante semplificazione anche della risposta ad altri eventi. Occhio che l'Observer tende a complicare considerevolmente il flusso di esecuzione.

VICIUS
08-03-2006, 11:28
public void testCorrectlyDrawn()
{
MockEngine engine = MockEngine.createForTesting(800, 600);

crushBox = CrushBox.create(2, 20, 20);
crushBox.draw(engine);

assertEquals("Number of crushes not correctly drawn", 1, engine.getNumberOfQuadsDrawn());
}

Stavo guardando ora...

L'unico test per la draw di CrushBox è questo!

Quindi se cambio posizione della crushbox all'interno del codice non fallisce nessun test! Chiaramente questo codice non è scritto in test driven... Ricordiamoci che non basta testare che venga disegnato un quad ma bisogna controllare posizione, rectangle e tutto il resto :)
É strano. Mi pareva di averli visti i test per le posizioni. Hai provato a cambiare le coordinate e vedere se qualche test falliva ?

ciao ;)

Ufo13
08-03-2006, 12:08
Sì diciamo che mancava solo il Rectangle ed il test esplicito sulla draw...

Comunque ora è tutto a posto :)

^TiGeRShArK^
08-03-2006, 13:17
Più test ci sono meglio è (anche perchè i test dipendono solo da CrushBox e non da dove si trova).

So che la via più facile per estrarre CrushBox è infilarla in GridController ma è inutile saremmo da capo... Va tirata fuori completamente dal concetto di Griglia... Per me andrebbe in PlayField!

Mancano i test che vedono dove viene posizionata per player1 e player2... ci pensi tu alla fine? Io aspetto che finisci per iniziare il mio bugfix :)
avevo pensato di metterla in playField...
anzi in origine l'avevo messa proprio là..
ma purtroppo è gridController che si trova a gestire tutte le azioni compiute all'interno della griglia, e quindi la classe gridController si sarebbe trovata ad accedere diverse volte al crushBox memorizzato in playField....
Invece spostando crushBox in gridController le azioni da compiere sono tutte nella stessa classe, senza necessità di richiamare metodi esterni....
Il test per la posizione l'avevo creato solo per la prima griglia in quanto i due campi da gioco sono esattamente simmetrici e la posizione della crushBox, per come era strutturato prima dentro grid, veniva calcolata con un offset a partire dall'origine della griglia...
quindi a causa della simmetria se era corretta la posizione del crushBox per il P1, doveva essere corretta anke la posizione per il P2..
ora estraendo crushBox, però mi sa che quest'assunzione non sarà + valida, e quindi provvederò a testare singolarmente i due casi...

P.S. il task cmq l'avevo svolto in maniera test-driven, te lo posso assicurare :p
il test del disegno strutturato in quel modo era venuto fuori dal particolare approccio che avevo scelto...
infatti, poikè non esisteva alcun metodo per cambiare la texture di uno sprite avevo deciso di creare ogni volta un nuovo oggetto crushBox quando serviva, ponendolo a null quando non doveva essere visualizzato.
Portando però fuori la logica da grid, ho dovuto necessariamente aggiungere un metodo setTexture in Sprite che permetteva di aggiornare la texture una volta che l'oggetto era stato creato....
vabbè.. alla fine si fa prima a vedere il codice appena committo che a cercare di capire la mia spiegazione...
cmq stasera vedo di finire tutto

^TiGeRShArK^
08-03-2006, 13:22
Mi sembra una situazione ideale per un Observer. Grid o GridController possono pubblicare gli eventi relativi ai Crush ad esempio (perche' non anche relativi alla fine della partita?) e poi invocare un metodo Listener, in Java sarebbe una classe, che e' eseguito al momento del verificarsi dell'evento.

PlayField puo' registrare un suo Listener per l'evento "Update Crushes", mettersi in ascolto ed eseguire il codice necessario per aggiornare il warning box di conseguenza.

E' un'idea di soluzione, ma voglio che prima di imbarcarcisi sia valutato il suo impatto sul codice, perche' se lo complica l'Observer non deve essere implentato. Lo si implementa solo in caso sia un'effettiva semplificazione, e magari si porta dietro un'elegante semplificazione anche della risposta ad altri eventi. Occhio che l'Observer tende a complicare considerevolmente il flusso di esecuzione.

mmmm...
ad occhio non credo sia necessario...
se non ho fatto grossi errori il codice ora km ora dovrebbe funzionare, il problema sarebbe solo relativo alla testabilità...
immagino che comunque si possa testare anke senza implementare un Observer..
il problema e ke ki ha le idee + chiare sul funzionamento di GridController mi dovrebbe spiegare un pò come funziona, perchè quando lancio l'update non viene richiamato il metodo updateCrushes in grid.....
immagino sia un problema relativo allo stato del GemsPair... ieri sera ho fatto qualche tentativo ma non sono riuscito a risolvere.....
.....chi è che capisce qualcosa del metodo gridController.update e dei metodi che richiama che mi può spiegare come fare ad arrivare nel ramo che attivi grid.updateCrushes()???

Ufo13
08-03-2006, 13:29
mmmm...
ad occhio non credo sia necessario...
se non ho fatto grossi errori il codice ora km ora dovrebbe funzionare, il problema sarebbe solo relativo alla testabilità...
immagino che comunque si possa testare anke senza implementare un Observer..
il problema e ke ki ha le idee + chiare sul funzionamento di GridController mi dovrebbe spiegare un pò come funziona, perchè quando lancio l'update non viene richiamato il metodo updateCrushes in grid.....
immagino sia un problema relativo allo stato del GemsPair... ieri sera ho fatto qualche tentativo ma non sono riuscito a risolvere.....
.....chi è che capisce qualcosa del metodo gridController.update e dei metodi che richiama che mi può spiegare come fare ad arrivare nel ramo che attivi grid.updateCrushes()???


Tiger onestamente penso che GridController non vada ultreriormente complicato con l'aggiunta della gestione di WarningBox e CrushBox...

Il fatto che non capisci come si arriva all'interno di un ramo dimostra che la situazione richiede semplificazioni invece del contrario...

Perchè non vedi bene l'Observer? Secondo me è la strada migliore :)

^TiGeRShArK^
08-03-2006, 14:47
perchè a quanto dice fek occore valutare attentamente la sua fattibilità...

E' un'idea di soluzione, ma voglio che prima di imbarcarcisi sia valutato il suo impatto sul codice, perche' se lo complica l'Observer non deve essere implentato. Lo si implementa solo in caso sia un'effettiva semplificazione

ora come ora non ho il codice davanti che sono al lavoro....
stasera posso dare il mio parere, non prima :p

Ufo13
08-03-2006, 14:55
Secondo la mia interpretazione nel suo post Francesco non parla di fattibilità (tutto è fattibile basta ragionarci su) ma di utilità.

Se la scelta di tenere WarningBox all'interno di Grid o GridController porta ad un design sbagliato e l'unica strada possibile è l'Observer a mio parere vale la pena prendere quella strada.

Come vedi il codice di Grid è stato semplificato di molto in questi giorni adottando questa strada :)

^TiGeRShArK^
08-03-2006, 15:27
Secondo la mia interpretazione nel suo post Francesco non parla di fattibilità (tutto è fattibile basta ragionarci su) ma di utilità.

Se la scelta di tenere WarningBox all'interno di Grid o GridController porta ad un design sbagliato e l'unica strada possibile è l'Observer a mio parere vale la pena prendere quella strada.

Come vedi il codice di Grid è stato semplificato di molto in questi giorni adottando questa strada :)
si vabbè..intendevo fattibilità nel senso se ci conviene farlo oppure no :p
cmq ora come ora non me lo ricordo tutto il codice.....
appena stasera torno ci ragioniamo un pò su :p
al massimo posso iniziare committando tutto quello che ho fatto finora commentando i test che non passano e ci lavoriamo insieme :)

^TiGeRShArK^
08-03-2006, 17:56
eccomi arrivato...
commento i test che non passano, committo, vado in coma per mezz'oretta e poi vediamo il da farsi :D

Ufo13
08-03-2006, 18:16
Non posso assolutamente guardare il codice al momento...

Secondo me non è saggio committare del codice che non fa passare dei test nella speranza che qualcuno possa farci qualcosa!

fek
08-03-2006, 19:04
eccomi arrivato...
commento i test che non passano, committo, vado in coma per mezz'oretta e poi vediamo il da farsi :D

Assolutamente no, se i test non passano, non si committa.

Ufo13
08-03-2006, 19:13
Assolutamente no, se i test non passano, non si committa.

troppo tardi mi sa :p puoi revertarlo?

^TiGeRShArK^
08-03-2006, 19:31
Assolutamente no, se i test non passano, non si committa.
:fiufiu:
ora mi sono svegliato...
cmq era dalle 4 e mezzo ke avevo scritto ke avrei committato per farvi vedere il ocdice ke avevo fatto finora...
per me posso revertare...
ma ke si dovrebbe fare?
si deve proseguire in questo modo estraendo la logica di curshbox da grid nel modo + semplice, o si dovrebbe implementare il pattern Observer???
In pratica i 3 test che non passano sono quelli che verificano l'update corretto di crushCounter, che non ho capito ancora come testare dal gridController, gli altri sono tutti green...
cmq ormai ke ci sei dai un'okkiata a codice confontandolo con la versione precedente con crushBox in grid e vedi cosa è meglio fare :p

^TiGeRShArK^
08-03-2006, 19:49
ehm...
su consiglio di vicius sto provando a revertare per rilasciare poi una patch...
ma...come si fa a revertare con tortoise???
ho fatto il checkout della revision corretta, ma se provo a committare da quella cartella mi dice che nessun file è stato modificato e fallisce il commit...
intanto vedo se ci capisco qualcosa...cmq qualsiasi consiglio è ben accetto :D

^TiGeRShArK^
08-03-2006, 20:31
forse ho risolto..
se potete fare un update x verificare è meglio però :p

VICIUS
08-03-2006, 23:25
La transizione alla nuova versione di lwjgl è finita.

Ho anche aggiunto altri due bugs che mi sono venuti in mente ora. Quindi c'è ancora lavoro per chi è rimasto senza. :D

ciao ;)

Bonfo
09-03-2006, 10:58
Finalmente sto rincomincaindo a dare un occhio al codice....
...posso fare una proposta di stile per rendere il codice più chiaro??

Per ciò che riguarda le interfacce non si può usare una nomenclatura più intuitiva che faccia capire al volo che non sono classi?? :D

Esempio: (Interfaccia di un metodo Display)
IDisplay
IDisplayable
DisplayableInterface
...

No so ...ma io con la I maiuscala mi trovo sempre bene.

@Vicius: ho visto i 2 nuovi bug...ma per ciò che riguarda il bug 2 il tempo qual'è??

Ho controllato meglio...mi sembrava di ricordare che si era usata una nomenclatura a parte per le interfaccie....perchè GemAction, KeyboardListener e daltre sono orfane di ciò?? ;)

Bonfo
09-03-2006, 11:22
Ho guardato un attimo il bug relativo all'ESC.
Riporto un paio di "scoperte" ... o meglio un paio di cose che mi hanno lasciato :eekk:


public void testnotifyKeyEventEscape()
{
input.notifyKeyEvent(KeyEvent.RIGHT, KeyEvent.PRESSED);
input.notifyKeyEvent(KeyEvent.ESCAPE, KeyEvent.PRESSED);

assertTrue(input.extractKey().key() == KeyEvent.RIGHT);
}


Cosa si sta testando?? :mbe:


public void notifyKeyEvent(int code, boolean state)
{
if(code == KeyEvent.ESCAPE)
{
return;
}

queue.add(new KeyEvent(code, state, timer.getTime()));
}


Qui il tasto ESC viene completamente bypassato...perchè?? (dipende dal test di prima ??) :mbe:


public void notifyKeyEvent(int code, boolean state)
{
if(code == KeyEvent.ESCAPE)
{
System.exit(0);
}

queue.add(new KeyEvent(code, state, timer.getTime()));
}


Con questa piccola modifica i test passano tutti, l'applicazione esce quando premo ESC...ma esce anche quando premo un qualunque tasto :eekk: :eekk:

Specifico che i tasti non inerenti al gioco sono mappati sul KeyEvent.ESCAPE

Qualcuno sa colmare le mie lacune?? :help:

VICIUS
09-03-2006, 11:43
@Vicius: ho visto i 2 nuovi bug...ma per ciò che riguarda il bug 2 il tempo qual'è??
Non l'ho chiesto a jocchan. Rendilo configurabile cosi non sbagli.Per ora metti 2500 ms.

ciao ;)

fek
09-03-2006, 12:14
Finalmente sto rincomincaindo a dare un occhio al codice....
...posso fare una proposta di stile per rendere il codice più chiaro??

Per ciò che riguarda le interfacce non si può usare una nomenclatura più intuitiva che faccia capire al volo che non sono classi?? :D

Esempio: (Interfaccia di un metodo Display)
IDisplay
IDisplayable
DisplayableInterface
...

No so ...ma io con la I maiuscala mi trovo sempre bene.

Ti cito (a memoria) che cosa ne pensa Kent Beck a riguardo: "Quando penso di mettere la I di fronte al nome di un'interfaccia perche' non mi viene in mente un nome migliore e' perche' non ho chiaro il ruolo di quell'interfaccia. La soluzione e' chiarirsi le idee sul ruolo, non mettere la I".

Quindi, niente I :)

'Displayable' e' un buon nome perche' chiarisce in maniera univoca il significato dell'interfaccia. Ho visto alcune interfacce con dieci metodi, ecco, questo non mi piace affatto, per me un'interfaccia non deve avere piu' di tre o quattro metodi molto chiari.

fek
09-03-2006, 12:17
Con questa piccola modifica i test passano tutti, l'applicazione esce quando premo ESC...ma esce anche quando premo un qualunque tasto :eekk: :eekk:

Specifico che i tasti non inerenti al gioco sono mappati sul KeyEvent.ESCAPE

Qualcuno sa colmare le mie lacune?? :help:

Due cose:

1) Una exit(0) cosi' brutale non e' sicuramente il modo piu' elegante di uscire, cercherei una soluzione migliore, molto migliore (e il relativo test).

2) Secondo me gli altri tasti devono essere mappati verso una KEY_UNKNOWN che poi e' ignorato dall'InputReactor. Decisamente non e' una soluzione accettabile il mappare tutti i tasti su ESC. E' urgente cambiare questo comportamento, Vic, puoi aggiungere un task per favore? Ovviamente servono i relativi test.

Bonfo
09-03-2006, 12:42
La exit(0) era solo una prova di "basso livello" per vedere se era lì che il tasto ESC veniva "perso"...è ovvio che ci voglia qualcosa di meglio ;)
Però il comportamento è tutto tranne che chiaro....ci vorrebbe qualcuno mi spiegasse quel test. :muro:

Per ciò che riguarda le interfacce volevo dire un'altra cosa:
il nome deve essere ovviamente esplicativo come per tutti i componenti di un progetto software, ma io aggiungerei qualcosa che faccia capire al volo che è una interfaccia e non una classe

Esempio con GemAction: se uno legge capisce che si tratta di una attività che fa una gemma o che può essere fatta su una gemma (e già qua c'è una non chiarezza :O ) e poi si chiede...."Ma quale cacchio è questa azione?!?" si va ad aprire il codice e scopre che si trattava di una interfaccia e non di una classe che implementava l'azione.

In VisulaStudio ci sono le immmaginine che aiutano....su Eclipse nisba !! :(
Quindi proponevo un qualcosa che garantisse a colpo d'occhio di capire se si tratta di una interfaccia o meno.
Solo questo... :Prrr:

fek
09-03-2006, 14:04
In VisulaStudio ci sono le immmaginine che aiutano....su Eclipse nisba !! :(
Quindi proponevo un qualcosa che garantisse a colpo d'occhio di capire se si tratta di una interfaccia o meno.
Solo questo... :Prrr:

In Eclipse c'e' la finestrella in basso che ti fa vedere la dichiarazione di qualunque cosa sotto il cursore, e passa la paura :D

Ufo13
09-03-2006, 19:37
Come siamo messi col terzo refactoring?

thebol
10-03-2006, 07:10
penso che il keyEvent escape, sia quello che fek indicava come unknow.

bisognerebbe creare un nuovo keyEvent(quit), legarlo a un eventHandler che contenga il riferimento a gameloop, ed esporre il metodo quit come pubblico.

L'unica "problema" e che la gestione degli handler e in gameField... ma il tasto quit puo essere associato solo al player1 e si evita il doppione.

VICIUS
10-03-2006, 16:53
Mi sono appena accorto di aver corretto il bug che avevo assegnato ad Ufo13. Si trattava semplicemente di metterere le chiamate alla draw(engine) nel ordine giusto in Grid.

ciao ;)

^TiGeRShArK^
10-03-2006, 17:32
Come siamo messi col terzo refactoring?
in questi giorni dopo il revert non ho potuto fare niente...
dovrei farlo stasera in teoria....
ma ancora non ho capito se procedere direttamente col pattern Observer o no...
cmq stasera decido :p

^TiGeRShArK^
10-03-2006, 20:32
ho fatto una breve ricerchina sull'Observer Pattern:

1.2 Applicability
Use the observer pattern in any of the following situations:
o When the abstraction has two aspects with one dependent on the other. Encapsulating these aspects in separate objects will increase the chance to reuse them independently.
o When the subject object doesn't know exactly how many observer objects it has.
o When the subject object should be able to notify it's observer objects without knowing who these objects are.

le ultime due non si applicano al nostro caso...
la prima onestamente non l'ho capita:
"Quando l'astrazione ha due aspetti di cui uno dipendente dall'altro. L'incapsulazione di questi aspetti in oggetti separati aumenterà le possibilità di riutilizzarli."

La sua implementazione è di questo tipo:
Il soggetto (subject) che nel nostro caso sarebbe la classe grid ha i seguenti metodi per registrare e deregistrare gli osservatori (observers):
attach(Observer o)
detach(Observer o)
inoltre possiede il metodo notify() che notifica tutti gli observer registrati dei cambiamenti avvenuti richiamando il metodo update() che essi implementano.
Questo medoto viene utilizzato principalmente nell'implementazione di interfacce grafiche, soprattutto utilizzando il pattern Model/View/Controller.

mmmm...
alla luce di tutto questo non so quanto ci possa essere utile, questo per i seguenti motivi:
1)E' abbastanza simile alla prima implementazione che mi era venuto in mente di adottare. Per meglio dire avevo pensato di mettere all'interno di grid direttamente i metodi per fare l'update di crushBox e di warningBox presenti in playField. La differenza è che (ovviamente :D), non avevo fatto alcuna considerazione sulla generalità in modo da usare un metodo update che notificasse entrambi gli oggetti dei cambiamenti avvenuti, ma nella mia idea l'oggetto coinvolto veniva direttamente aggiornato da grid, seguendo la filosofia "make it simple, make it work" (o qualcosa del genere :fagiano: ).
2)Mi pare che il desing si complichi + di quanto si semplifichi...certo se avessimo 5, 6 o + observer il vantaggio di questo pattern sarebbe stato evidente. Ma dato che ora come ora abbiamo due observer vale la pena implementarlo??? Facendo questa riflessione mi lampeggia nella mente la parola YAGNI.
3)A parte il primo use case, che come ho detto non ho capito bene, gli altri due non rispecchiano affatto la nostra situazione in quanto sappiamo a priori esattamente quanti sono gli observer e sappiamo inoltre quale oggetto è necessario aggiornare in un determinato momento.
Concludendo.....
Io sarei dell'idea che per ora questo design pattern non ci può essere d'aiuto.
Voi che ne pensate???:fagiano:

^TiGeRShArK^
11-03-2006, 03:19
committato...
ho estratto solo crushbox x ora però....
guardate se va bene così posso fare warningBox...
però domenica ke domani nn ci sono....

Ufo13
11-03-2006, 08:05
Ah ok :)

Sì io direi che è già un buon inizio

Bonfo
11-03-2006, 18:08
Allora...visto che non ce la faccio più a non fare nulla per tenermi il tempo per fare i miei "doveri", rubo il tempo ai miei doveri e faccio i miei piaceri !!! :angel: :asd:

Allora mi occupo del bug 4. Sto mooolto largo coi tempi perchè non si sa mai....5 giorni. :flower:

Faccio un attimo la lista di cosa mi propongo di fare così mi dite se è OK.
1) Aggiungere il keyEvent UNKNOW come proponeva fek;
2)Aggiungere il test relativo all'exit
3) Trovare un modo più elegante per uscire rispetto la System.exit(0);
4)Correggere il bug....finalmente :D

Bonfo
11-03-2006, 18:22
public void testnotifyKeyEventEscape()
{
input.notifyKeyEvent(KeyEvent.RIGHT, KeyEvent.PRESSED);
input.notifyKeyEvent(KeyEvent.ESCAPE, KeyEvent.PRESSED);

assertTrue(input.extractKey().key() == KeyEvent.RIGHT);
}


Questo è il famoso test che non mi convinceva.
Dopo averci ragionato un po' su mi sembar che controllic he se c'è il tasto RIGHT premuto e il tasto ESC premuto venga notificato il tasto RIGHT e annullato il tasto ESC.
Ho capito bene??

In ogni caso il nome del test non mi sembra corretto...dovrebbe essere
testNotNotifyKeyEventEscape()

Nel caso abbia capito bene...
1) è il comportamento che vogliamo??
2) perchè non c'è il test anche per gli altri tasti??

Bonfo
11-03-2006, 18:32
Cosa ne dite di questo test (che fallisce... :O )

public void testNotifyKeyEventEscape()
{
input.notifyKeyEvent(KeyEvent.ESCAPE, KeyEvent.PRESSED);

assertTrue(input.extractKey().key() == KeyEvent.ESCAPE);
}

VICIUS
11-03-2006, 20:40
3) Trovare un modo più elegante per uscire rispetto la System.exit(0);
Basta settare finished su loop in GameLoop.

ciao ;)

Bonfo
11-03-2006, 23:12
Ok....ma così fermo solo il loop...mi ca esco dall'applicazione.
o no??? :confused: :confused:

VICIUS
12-03-2006, 00:45
Ok....ma così fermo solo il loop...mi ca esco dall'applicazione.
o no??? :confused: :confused:
Una volta uscito da loop() in game c'è una chiamata a quit().

ciao ;)

Bonfo
12-03-2006, 01:09
Una volta uscito da loop() in game c'è una chiamata a quit().

ciao ;)

OK!!! :mano:

Bonfo
12-03-2006, 01:17
public void testnotifyKeyEventEscape()
{
input.notifyKeyEvent(KeyEvent.RIGHT, KeyEvent.PRESSED);
input.notifyKeyEvent(KeyEvent.ESCAPE, KeyEvent.PRESSED);

assertTrue(input.extractKey().key() == KeyEvent.RIGHT);
}


Questo è il famoso test che non mi convinceva.
[...]
Nel caso abbia capito bene...
1) è il comportamento che vogliamo??
2) perchè non c'è il test anche per gli altri tasti??

Ho bisogno che uqlacuno mi illumini...altrimenti ho un po' di dubbi su come procedere !!! :mc:

ALT!!! Forse ho capito...il test controlla solamente che la queue gestisca i keyEvent in ordine. Ho indovinato??? :sofico:
Bhè...allora il nome del test è proprio sbagliato :ncomment:

Bonfo
12-03-2006, 17:48
IAggiunto il test in TestInput:

public void testNotifyKeyEventEscape()
{
input.notifyKeyEvent(KeyEvent.ESCAPE, KeyEvent.PRESSED);

assertTrue(input.extractKey().key() == KeyEvent.ESCAPE);
}

Sì chiamo come uno che c'è già ma ha la N di notify maiuscola....ormai lo avete capito che mi sta un po' adosso l'altro test :Prrr:
Il test fallisce.

Ecco le modifiche fatte per introdurre il KeyEvent.UNKNOWN:
1) in it.diamonds.engine.input.KeyEvent aggiunto

public static final int UNKONWN = -1;


2)in it.diamonds.engine.input.KeyMappings modificato il metodo

public int translateKey(int keyOgl)
{
if(!keyMappings.containsKey(keyOgl))
{
return KeyEvent.UNKONWN;
}

return keyMappings.get(keyOgl);
}


3) ovviamente ora dei test fallliscono. Ecco le correzzioni (it.diamonds.tests.engine.input.KeyMappings):


public void testEmptyMappings()
{
assertEquals(KeyEvent.UNKONWN, keyMappings.translateKey(0));
}

public void testUnexistingMappings()
{
keyMappings.addMapping(10, 20);

assertEquals(KeyEvent.UNKONWN, keyMappings.translateKey(20));
}


4) Ora fallisce questo test : it.diamonds.tests.engine.input.TestKeyboard.testAddMultipletInputToKeyboard
Per risolverlo ho impiegato un po'...ma andando a naso ho cambiato in it.diamonds.engine.input.Input questo:

public void notifyKeyEvent(int code, boolean state)
{
if(code == KeyEvent.UNKONWN)
{
return;
}

queue.add(new KeyEvent(code, state, timer.getTime()));
}


5) GREEN

Pronto a committare l'introduzione di KeyEvent.UKNOWN.
Siccome è la prima volta che lavoro da solo sul codice, aspetto conferma per committare. ;)

P.S.: torno a casa stasera....fino ad allora non posso committare. In ogni caso ho scritto tutte le modifiche

VICIUS
12-03-2006, 22:44
Pronto a committare l'introduzione di KeyEvent.UKNOWN.
Siccome è la prima volta che lavoro da solo sul codice, aspetto conferma per committare. ;)

P.S.: torno a casa stasera....fino ad allora non posso committare. In ogni caso ho scritto tutte le modifiche
Procedi pure.

ciao ;)

VICIUS
12-03-2006, 22:48
committato...
ho estratto solo crushbox x ora però....
guardate se va bene così posso fare warningBox...
però domenica ke domani nn ci sono....
Stavo provando ora l'ultima build dopo il tuo commit e le crushbox ora sono disegnate dietro la griglia e le gemme. Non ho guardato il codice ma cosi a naso ho come l'impressione che hai usato opaque come tipo di layer. Quando finie il task controllate anche questa regression.

ciao ;)

Bonfo
13-03-2006, 00:19
Commit eseguito. :D
Prossimo obiettivo: formalizzare un test che alla pressione di ESC il GameLoop si interrrompa.

Mi sa che qualcuno mi dovrà dare un piccola ripassata sugli handler. :stordita:

Ultima cosa: il mio "amatissimo" test testnotifyKeyEventEscape è ancora lì senza che nessuno sia riuscito a giustificarne il significato o il nome.
Siccome nessuno mi aiuta ( :cry: ) lo lascio lì ancora 48 ore e poi lo sposto in IGNORE. :Prrr:

Bonfo
13-03-2006, 00:57
Bene...mi sono guardato gli Handler e mi sembra di aver capito la strada da seguire.

Ora però mi sorge il dubbio su dove andare a mettere il test.
Infatti i test in test.handlers non c'entrano poi molto con gli handler ma controllano molto di più che sia corretto il codice che invocano gli handler in seguito.
Gli Handler infatti mi sembrano essere tutti testati in test.TestGridReactionToInput.... :mc: :mc:

Inoltre il mio test riguarderà anche Game e GameLoop...quinidi :wtf:

Ora però vado a letto :zzz: :D

^TiGeRShArK^
13-03-2006, 17:23
Stavo provando ora l'ultima build dopo il tuo commit e le crushbox ora sono disegnate dietro la griglia e le gemme. Non ho guardato il codice ma cosi a naso ho come l'impressione che hai usato opaque come tipo di layer. Quando finie il task controllate anche questa regression.

ciao ;)
ehm.. si...ho suato opaque....
non doveva essere disegnato dietro il crushbox??? :fagiano:
cmq stasera dovrei fare anke il warningbox..

VICIUS
13-03-2006, 17:35
ehm.. si...ho suato opaque....
non doveva essere disegnato dietro il crushbox??? :fagiano:
cmq stasera dovrei fare anke il warningbox..
No il crushbox va disegnato sopra altrimenti la griglia e le gemmo lo coprono e non si riesce a leggere.

ciao ;)

^TiGeRShArK^
13-03-2006, 21:56
fatto...
task completato....

finalmente aggiungerei... uff...

VICIUS
13-03-2006, 22:16
fatto...
task completato....

finalmente aggiungerei... uff...
Ottimo lavoro a tutti e due :)

ciao ;)

^TiGeRShArK^
13-03-2006, 22:49
ho appena scoperto un buggettino :D
lo sto correggendo :fiufiu:

^TiGeRShArK^
13-03-2006, 23:43
risolto...
prima avevo invertito playfield1 e playfield2 nei test e nel gioco e poi ho dovuto lottare col mockgenerator ke mi ritornava delle flashing gem uguali ke aumentavano ovviamente di due il numero di crush:muro:

Bonfo
15-03-2006, 01:03
Vi aggiorno sulla situazione del bug 4. :D

Non ho ancora deciso dove aggiungerò il test...per adesso sto copiando molto codice da it.diamonds.tests.TestGridReactionToInput. Forse alla fine il test finirà lì.

Ora però salta all'occhio il dubbio. In realtà non è grid che deve reagire alla pressione del tasto ESC, ma l'applicazione stessa. Quindi Game o, meglio, GameLoop.

Il test infatti dovrebbe controllare una proprietà di GameLoop che mi dica se sta eseguendo il loop o meno. Insomma avrei bisogno di esporre lo stato della variabile finished.
Inoltre, cosa spinosa, dovrei poter modificare il valore di finished per costringere la consclusione del loop.
La prima idea che mi è venuta è esporre un metodo pubblico del tipo STOP, FINISH,...
L'idea però di mettere tale metodo pubblico così non mi piace proprio...è GameLoop che deve decidere se terminare o no.
IDEE??? :confused: :confused:

Per adesso vado avanti come mi viene più semplice :sofico:

Bonfo
15-03-2006, 01:41
Per andare avanti volevo aggiungere i due metodi pubblici alla classe GameLoop, soluzione più semplice anche se forse la più brutta.
Prima di farlo ovviamente test ;)


gameLoop.loop();
assertFalse(gameLoop.hasFinished());
gameLoop.finishLoop();
assertTrue(gameLoop.hasFinished());


Ovviamente prima devo settare correttamente GameLoop perchè looppi.
Con un paio di Cut&Paste:

public void setUpGameLoop()
{
gameLoop.setConfig(Config.createForTesting());

gameLoop.setAudio(new Audio());

gameLoop.setEngine(Engine.createEngineForTesting(gameLoop.getConfig().getInteger("width"), gameLoop.getConfig().getInteger("height")));
gameLoop.setLayerManager(new LayerManager());

gameLoop.setBackground(new Background("back000", ".jpg", gameLoop.getConfig()));

gameLoop.setTimer(new MockTimer());
timeStamp = gameLoop.getTimer().getTime();

setUpInput(Keyboard.createKeyboardForTesting());

setUpPlayFields(timeStamp);
}

private void setUpPlayFields(long timeStamp)
{
randomSeed = System.nanoTime();

gameLoop.createPlayFieldOne(timeStamp, randomSeed);
gameLoop.createPlayFieldTwo(timeStamp, randomSeed);

gameLoop.attachFields();
}

private void setUpInput(Keyboard keyboard)
{
gameLoop.setKeyboard(keyboard);

gameLoop.setPlayerOneInput(new Input(gameLoop.getKeyboard(), gameLoop.getTimer()));
gameLoop.getPlayerOneInput().setKeyMappings(KeyMappings.createForPlayerOne());

gameLoop.setPlayerTwoInput(new Input(gameLoop.getKeyboard(), gameLoop.getTimer()));
gameLoop.getPlayerTwoInput().setKeyMappings(KeyMappings.createForPlayerTwo());
}


Ma ecco cosa mi viene detto: :cry: :cry:

[junit] Tests run: 1, Failures: 0, Errors: 1, Time elapsed: 3,329 sec
[junit] Testcase: testFinish(it.diamonds.tests.ignore.TestGameLoopFinish): Caused an ERROR
[junit] Keyboard must be created before you can poll the device
[junit] java.lang.IllegalStateException: Keyboard must be created before you can poll the device
[junit] at org.lwjgl.input.Keyboard.poll(Keyboard.java:345)
[junit] at it.diamonds.engine.input.Keyboard.update(Keyboard.java:42)
[junit] at it.diamonds.GameLoop.processInput(GameLoop.java:211)
[junit] at it.diamonds.GameLoop.loop(GameLoop.java:275)
[junit] at it.diamonds.tests.ignore.TestGameLoopFinish.testFinish(TestGameLoopFinish.java:46)



PERCHE' ??? :muro: :muro:


P.S.: io sto testando un loop...mi salvo perchè la cosa è semplice e posso "rompere" lo stile puro e scrivere 2 test e risolverli in un colpo solo.
Ma come si farebbe in puro stile TDD??
Ora mi attende il letto :zzz: :zzz: :ronf:

thebol
15-03-2006, 07:10
è gameloop che decide se finire, ma se vogliamo dare la possibilità anche all'utente bisogna mettere a disposizione un metodo pubblico per l'handler.

se proprio si vuole evitare il metodo pubblico, si puo scrivere l'handler come innerclass di gameLoop, cosi che possa utilizzare i metodi privati di gameLoop,
ma non è il max della pulizzia.

Bonfo
16-03-2006, 01:20
A spizzichi e bocconi sto andando avanti :D ;)
Vi aggiorno sullo stato dei lavori.

1) il test di loop non serve più. Si riesce benissimo a testare il valore della variabile finished in GameLoop anche senza invocare il metodo loop().
La cosa, per me da capogiro, è che da nessuna parte la variabile viene inizializzata a false !!!! :eekk: :wtf:

2) Ho creato la classe EscapeCommandHandler ed ecco i test relativi

public void testGameNotExit()
{
generateKeyPressed(KeyEvent.ESCAPE);
inputReactor.reactToInput(timer);

assertFalse(gameLoop.hasFinished());
}

public void testGameExit()
{
generateKeyPressed(KeyEvent.ESCAPE);
generateKeyReleased(KeyEvent.ESCAPE);
inputReactor.reactToInput(timer);

assertTrue(gameLoop.hasFinished());
}


Il problema è che per riuscire a fare questo deve conoscere GameLoop

3) Ho scritto l'ultimo test che riguarda l'inserimento dell'handler in GridController, più precisamente in addGridInputHandlers().
Sarebbe una cavolata ... ma il problema è il riferimento a GameLoop!!!! :muro: :muro:

Nessuno lo conosce.
L'unico modo plausibile per adesso è farlo "scendere" come parametro tra le varie chiamate dei costruttori fino alla creazione del GridController.
Questa soluzione non mi piace molto.
Attendo idee e proposte :D :asd:

fek
16-03-2006, 13:34
Nessuno lo conosce.
L'unico modo plausibile per adesso è farlo "scendere" come parametro tra le varie chiamate dei costruttori fino alla creazione del GridController.
Questa soluzione non mi piace molto.
Attendo idee e proposte :D :asd:

Questa soluzione non piace assolutamente neppure a me :)

Dove e' creato l'handler di preciso?

Bonfo
16-03-2006, 13:41
Non sono a casa quindi vado a memoria.

Tra i vari costruttori di GridController c'è quello con più parametri, il più "interno", che un metodo addHandlersToInputReactor() o qualcosa di simile, al cui interno vengono creati tutti gli Handler.

Un'altra soluzione che mi era venuta in mente era, invece di far scendere il riferimento a GameLoop, di far salire fino a GameLoop la creazione dell'inputReactor e poi passare quello come parametro verso il basso. ;)
Questa soluzione manterebbe coerente l'incapsulamento anche se vuol dire modificare una sacco di costruttori.

Altre idee??? :help:

fek
16-03-2006, 13:49
Un'altra soluzione che mi era venuta in mente era, invece di far scendere il riferimento a GameLoop, di far salire fino a GameLoop la creazione dell'inputReactor e poi passare quello come parametro verso il basso. ;)
Questa soluzione manterebbe coerente l'incapsulamento anche se vuol dire modificare una sacco di costruttori.


Questa e' una soluzione piu' allettante.

Bonfo
16-03-2006, 18:35
Ok....procedo con questa soluzione.
Mi sa che dovrò scombinare un po' tutto....insomma un bel refactoring!! :D

Bonfo
16-03-2006, 18:50
Il refactoring che dta venendo fuori mi piace molto...mi sa che il tutto diventa pure più semplice ;)

Per aadesso gli effetti sono solo la riscrittura delle invocazioni dei costruttori....un impatto molto minore di quello che mi aspettavo.

Tra oggi domani rifaccio tutto passo passo per evitare di lasciare in giro schifezze che ho aggiunto facendo varie prove.

Domani posto la fine del task :D

cionci
16-03-2006, 18:53
Ho in mente una possibile soluzione...
E se si aggiunge un input reactor per tutti i tasti che non appartengono ai play field ?
Ora come ora ogni GridController ha il suo input reactor...niente vieta dic rearne uno anche per gestire l'ESC e gli altri tasti di cui avremo bisogno in futuro...

fek
17-03-2006, 09:51
Ho in mente una possibile soluzione...
E se si aggiunge un input reactor per tutti i tasti che non appartengono ai play field ?
Ora come ora ogni GridController ha il suo input reactor...niente vieta dic rearne uno anche per gestire l'ESC e gli altri tasti di cui avremo bisogno in futuro...

E ma sai che e' un'idea proprio furba? Lo si mette alla fine del game loop cosi' aggancia tutti gli eventi dopo che sono stati gestiti dai playfield.

fek
17-03-2006, 09:52
Il refactoring che dta venendo fuori mi piace molto...mi sa che il tutto diventa pure più semplice ;)

Se semplifica le cose procedi con il refactoring. Poi valutiamo l'idea di cionci.

Stai attento a non passare piu' di 3/4 parametri ai costruttori. Abbiamo gia' un PlayFieldDescriptor con 7 parametri che devono sparire in qualche modo con un refactoring/.

cionci
17-03-2006, 10:10
E ma sai che e' un'idea proprio furba? Lo si mette alla fine del game loop cosi' aggancia tutti gli eventi dopo che sono stati gestiti dai playfield.
Sì, mi sembra la cosa migliore... Senza contare che l'abbiamo già adottata per duplicare la gestione dei tasti delle due griglie... Quindi mi sembra la strada più semplice...

Bonfo
18-03-2006, 01:20
BUG 4: RISOLTO.

Faccio il report di quello che ho fatto:
Ho modificato gameLoop fornendogli un metodo pubblico per terminare il loop di gioco per permettere l'uscita.

Ho creato la classe EscapeCommandHandler che non fa nient'altro che invocare il metodo di GameLoop che interrompe il ciclo.

Ora c'era il problema di aggiungere all'InputReactor un istanza del nuovo Handler, il quale ha bisogno di un riferimento a GameLoop.
Per non far "scendere" tra i vari costruttori il gameLoop rompendo l'incapsulamento, si è decisco di far "salire" l'inputReactor tra i vari costruttori. ;)

Ho eseguito il refactoring facendo finire la salita nel metodo createPlayField() di GameLoop. Il refactoring non è stato traumatico, praticamente dove nei cotruttori appariva Input ora appare InputReactor :D
L'unica conseguenza risulta l'inserimento all'interno di numerosi test della costruzione di un InputReactor :(

A questo punto ero felice perchè avevo finito. Mancava il test per l'inserimento dell'Handler all'interno del gioco (l'avevo già preparato per quando l'inserimento andava in GridController).

Bene...in 2 ore non ho cavato un ragno dal buco :cry: :muro: :cry: :muro:
Salendo così tanto sono finto a trattare con delle classi molto chiuse, per fortuna, considerando il loro compito, ma che rendono moolto difficile riuscire a testarle.
La mia idea si basava sul creare un gameLoop, notificare la pressione del tasto ESCAPE, fare loopare GameLoop e testare che il loop terminasse.
Bhè...fare loopare GameLoop per test è difficilissimo. Ho provato a copiare parte di Bootstrap, ma il checkstyle non mi dava pace. Ho provato allora ad usare Bootstrap e Game, ma così non posso agire sull'input...

....insomma una serata agitata (Venerdì 17 non poteva finire diversamente :rotfl: )

Quindi
ATTENZIONE
Ho inserito il codice che risolve il bug ed è garantito al 100%.
Ma siccome MANCA IL TEST RELATIVO, lo commento con un "TODO: make test" e faccio il commit.

Così ci lavoriamo tutti insieme...anche perchè non si sa mai che la soluzione di cionci risolva il problema ;)

Ora letto...notte :ronf:

P.S.: spero di non avere gli incubi di Fek che mi spezza le ditine :asd: :asd:

VICIUS
18-03-2006, 12:31
[...]Ho inserito il codice che risolve il bug ed è garantito al 100%.
Ma siccome MANCA IL TEST RELATIVO, lo commento con un "TODO: make test" e faccio il commit.
Ok. Per questa volta sei perdonato anche perché testare GameLoop in questo momento è praticamente impossibile. Ma che non accada piu :p

ciao ;)

Bonfo
18-03-2006, 12:38
Grazie Vic... :ave: :ave: :ave:
Ma il problema rimane :( :muro:

Altra cosa che ricordo.
I test relativi agli handler sono un po' sparsi in giro tra i vari package. Penso che non sia capitato per caso, ma che si debba decidere come gestirli...almeno così la prossima volta so da dove copiare :D :D

I miei test gli ho messo in it.diamonds.tests.handlers.

fek
18-03-2006, 13:21
Ok. Per questa volta sei perdonato anche perché testare GameLoop in questo momento è praticamente impossibile. Ma che non accada piu :p

ciao ;)

Pero' per espiare le sue colpe adesso deve testare GameLoop :)

cionci
18-03-2006, 14:43
BUG 4: RISOLTO.
Ma non era più semplice fare come avevo detto io ?

Bonfo
18-03-2006, 15:24
Ma non era più semplice fare come avevo detto io ?


Eccoti la risposta.

Se semplifica le cose procedi con il refactoring. Poi valutiamo l'idea di cionci.


Infatti alla fine ho ceduto perchè sapevo che la soluzione, che porta a risolvere il problema nel modo più semplice, anche se probabilmente non il più furbo, sarebbe stata da ricontrollare ;)

VICIUS
19-03-2006, 22:47
Ciclo terminato. Chiudo il thread.

ciao ;)