View Full Version : [CICLO 1] Abbiamo il primo bug!
Non e' proprio un bug, ma vediamo come reagirebbe il Customer al task che richiede l'input della tastiera.
Il Customer lancia l'applicazione, preme un tasto freccia e vede il diamante che si muove. Yay! Preme un altro tasto freccia e vede il diamante muoversi in un'altra direzione. Yay! Poi preme due tasti freccia contemporaneamente, vorrebbe vedere il diamante muoversi in diagonale, ma il nostro gioco registra solo l'ultimo tasto premuto. Sguardo perplesso del customer che pensa: "C'e' un bug".
In realta' no, le sue specifiche non richiedevano questo comportamento ed erano piuttosto vaghe in merito, c'era spazio per l'interpretazione, che per caso non era quello che il Customer pensava.
Comunque sia ora il Customer ha le idee piu' chiare, e qui si puo' vedere il valore dell'avere un processo di sviluppo che ogni due settimane consegna qualcosa di usabile al Customer: il Customer puo' chiarirsi le idee usando l'applicazione e puo' chiarirle al team di sviluppo che puo' velocemente correggere i problemi.
Ora il Customer ha le idee piu' chiare: "Quando premo due tasti freccia (ad esempo alto e destra) in contemporanea, vorrei che il diamante si muovesse in diagonale".
La palla passa a noi per correggere il bug, guardiamo il codice, si' il problema forse e' li', lo correggiamo e noooooooo!
Prima di correggere un qualunque bug si scrive un test che fallisce in presenza della condizione che genera il bug.
Si lancia il test e si verifica che fallice, poi si corregge il bug, si lancia il test di nuovo e si verifica che non fallisce piu'. Il bug e' risolto, abbiamo un modo automatico per verificarlo e sappiamo che, se in futuro cambieremo il codice da qualche parte e introducessimo il bug nuovamente, il test ci avvertira' subito facendoci risparmiare tempo.
La logica dietro a questa regola e' che un bug non e' altro che il verificarsi di una condizione indesiderata e nella nostra rete di test non ce n'e' uno che fallisca al verificarsi di quella condizione, avvertendoci della presenza del bug. La nostra rete di test non e' abbastanza fitta ed un bug e' passato. Bisogna renderla piu' fitta e assicurarsi che questo bug non passi piu' inosservato.
Ora, guardate la classe KeyboardImpl e suggeritemi qualche test che possa rilevare la condizione di due tasti premuti contemporaneamente e solo l'ultimo e' registrato.
Diagonale e ritorno verso l'alto in un gioco a-la tetris :confused:
ciao ;)
Diagonale e ritorno verso l'alto in un gioco a-la tetris :confused:
ciao ;)
Sai come sono i Customer, non hanno mai le idee chiare :D
(in realta' ho solo sfruttato la scusa per introdurre il discorso sui bug e i test ;))
Sai come sono i Customer, non hanno mai le idee chiare :D
(in realta' ho solo sfruttato la scusa per introdurre il discorso sui bug e i test ;))
eheh ora è tutto piu chiaro. :D
ciao ;)
Domanda, ma è un bug ? A quanto pare la classe KeyboardImpl gestisce anche la possibilità di più tasti premuti contemporaneamente, imho questa potrebbe essere semplicemente una feature non ancora implementata dal codice (visto che non è stata richiesta)...
Ad esempio per implementarla basterebbe un refactoring alla classe input... Oltre a richiedere alla classe input lo stato dei tasti possiamo richiedergli anche la direzione scelta... Ad esempio con metodi di questo tipo:
public Boolean isLeftDirection()
{
return isKeyLeft() && !isKeyRight() && !isKeyLeft() && !isKeyUp();
}
Tecnicamente il "bug" e' nell'uso che la classe Game fa della classe Keyboard.
Ne abbiamo parlato io e Cesare, il problema della classe Keyboard e' che al momento molto del suo comportamento non e' di fatto testato, quindi vogliamo modificarla di modo da essere piu' facilmente testabile.
Una volta che abbiamo abbastanza confidenza nella classe test, possiamo cambiare il codice in Game e correggere il "bug".
Cesare e' giunto a definire il codice del test che non passa e che vogliamo che passi per risolvere il bug:
keyboard.generateKeyPressEvent(left);
keyboard.generateKeyPressEvent(right);
assertTrue(input.isLeftPressed());
assertTrue(input.isRightPressed());
keyboard.generateKeyReleaseEvent(right);
assertFalse(input.isRightPressed());
assertTrue(input.isLeftPressed());
Ora non resta che scrivere il codice che fa passare questo test (e tutti gl'altri).
Sai come sono i Customer, non hanno mai le idee chiare :D
(in realta' ho solo sfruttato la scusa per introdurre il discorso sui bug e i test ;))
Ma io ho le idee chiare :Prrr:
Ma io ho le idee chiare :Prrr:
E invece no!
Ti sei accorto che premendo prima la freccia a destra e poi la freccia a sinistra contemporaneamente il diamante si sposta a destra, mentre vuoi che stia fermo. Bug!
Da correggere, ma non prima di un bel test che lo esponga :)
come? Mi dici che non te n'eri accorto? Dettagli implementatitivi...
cdimauro
29-09-2005, 14:34
Diabolico... :asd:
cisc, ho visto che hai corretto un bug.
Scritto anche il test che lo espone?
Me lo posti qui per favore? :)
ecco il test, messo in testSprite.java:
public void testCollision()
{
sprite.move(-100,0);
assertEquals(texture.getWidth()/2, sprite.getX(), 0.001f);
assertEquals(200, sprite.getY(), 0.001f);
sprite.move(0,10);
assertEquals(210, sprite.getY(), 0.001f);
}
uso texture.getWidth() perchè in setup ho messo sprite.setSizeX(texture.getWidth()); , questo magari è una cosa che si può modificare...
Ottimo!
Una volta che abbiamo il test possiamo modificare quello che ci pare :)
mi sa che ho trovato un altro bug, se è avvenuta una collisione e scorro sulla parete fino ad arrivare ad un'altra collisione sulla parete ortogonale, non si sente il suono...cmq il bug di prima è stato trovato da DanieleC88
mi sa che ho trovato un altro bug, se è avvenuta una collisione e scorro sulla parete fino ad arrivare ad un'altra collisione sulla parete ortogonale, non si sente il suono...cmq il bug di prima è stato trovato da DanieleC88
Vero. Test e poi fix :)
ciao ;)
C'è un'altro bug...anche se credo che sia difficile da testare... In pratica avvicinando il più lentamente possibile al bordo il suono viene eseguito tante volte (provate diverse volte, non è facile arrivare nella posizione giusta)...
Probabilmente il bug sia ha quando con un movimento si arriva al bordo precisi al pixel...
Il problema è qui (o almeno dove si verifica)...la correzione andrà fatta nell'algoritmo di collisione che dovrà chiamare stopPulse...
moved è falso, mentre isPulsing ritorna vero:
public boolean move(float dx, float dy)
{
boolean moved = super.move(dx, dy);
if(moved)
{
if(!isPulsing())
{
startPulse();
}
}
else
{
if(isPulsing())
{
stopPulse();
/* TODO: manage double collision
* between two orthogonal walls and
* play sound on second collision
*/
if (collisionSound != null)
{
collisionSound.play();
}
}
}
return moved;
}
Il problema è ancora più strano... Mettendo un breakpoint nella playSound si raggiunge il break point non tute le volte che si la move di Gem, ma una volta sì ed una volta no (quindi la prima volta viene fermato il pulse, la seconda viene ristabilito)... Veramente strano... Indago meglio...
Non ne sto venendo a capo... Ora il suono viene suonato una volta sì e N no... Probabilmente p una situazione diversa di errore...
cionci a quanto ho capito il problema è che quando ti avvicini in un certo modo, il diamante sta pulsando e quindi, dato che il codice delle collisioni controlla valori che in realtà stanno variando, si crea questo bug, penso che la soluzione più semplice sia nel mantenere altre due variabili che memorizzino le dimensioni "statiche" del diamante, l'unico problema che sto avendo è il fare un test che mi rilevi il bug..
ecco il test in gemTestCollisionSound:
public void testCollisionSoundNearWall()
{
Input input=Input.createInputForTesting();
gem.setCollisionSound(sound);
gem.move(-67.99f,0);
assertEquals(gem.getWidth()/2+0.01f, gem.getX(), 0.001f);
assertTrue (gem.isPulsing());
assertFalse (sound.wasPlayed());
gem.draw(null);
gem.reactToInput(input);
assertFalse (sound.wasPlayed());
}
questo codice testa il fatto che senza dare un move (ma solo a causa del "pulsing") si verifica una collisione che genera un sound, testare i suoni a ripetizioni che in realtà si verifichiano è impossibile!!!
BlueDragon
06-10-2005, 22:32
cionci a quanto ho capito il problema è che quando ti avvicini in un certo modo, il diamante sta pulsando e quindi, dato che il codice delle collisioni controlla valori che in realtà stanno variando, si crea questo bug, penso che la soluzione più semplice sia nel mantenere altre due variabili che memorizzino le dimensioni "statiche" del diamante, l'unico problema che sto avendo è il fare un test che mi rilevi il bug..
Concordo con cisc...tra l'altro io suggerirei anche qualche refactoring...
Per esempio a me CheckForCollision risulta molto più chiaro così:
private boolean checkForCollision(float dx, float dy)
{
boolean collided = false;
if(testLeftEdgeCollision())
{
x = (width / 2);
collided = true;
}
if(testRightEdgeCollision())
{
x = (displayWidth - (width / 2));
collided = true;
}
if(testTopEdgeCollision())
{
y = (height / 2);
collided = true;
}
if(testBottomEdgeCollision())
{
y = (displayHeight - (height / 2));
collided = true;
}
return collided;
}
Rispetto alla versione attuale in cui ci sono 3 variabili boolean. una per il return finale, una per l'angolo altosinistra ed una per l'angolo in basso a destra...che poi confondono ancora di più se si considera che la variabile collisionBottomRight viene settata dopo aver controllato Top e Bottom collision anziché dopo aver controllato Bottom e Right come suggerirebbe il nome...
Inoltre nei metodi testBottomEdgeCollision (e Left,Right,Top) non serve passare alcun valore visto che usano direttamente le variabili interne, ed infatti nella mia versione locale gli ho tolto i parametri come potete vedere dalle chiamate del metodo checkForCollision suggerito sopra.
Inoltre, visto che nel codice il movimento è true quando non ci sono collisioni, io direi che le collisioni devono essere true quando la texture tenta di superare i limiti, non quando ci arriva pari pari.
Cioè, tradotto in codice:
private Boolean testLeftEdgeCollision(float dx)
{
return ((x - (width / 2)) < 0); // < 0 anziché il <= 0
}
Infatti quando un oggetto muove scende preciso lungo un bordo verticale (tralasciando la questione delle pulsazioni) non sta collidendo con il bordo, ma ci si sta muovendo tranquillamente accanto...quindi il movimendo dovrebbe true e la collisione false (e quindi niente suono mentre scendi o sali).
Concordo con cisc...tra l'altro io suggerirei anche qualche refactoring...
Procedi pure al refactoring. Cerca di scrivere anche un test che verifichi la condizione che hai esposto.
Bravi tutti e due.
BlueDragon
06-10-2005, 22:56
Procedi pure al refactoring. Cerca di scrivere anche un test che verifichi la condizione che hai esposto.
Bravi tutti e due.
Adesso penso di andare a nanna ma domani magari tiro fuori qualche test se non l'ha già fatto qualcun'altro.
Per quanto riguarda il "procedere al refactoring", la mia copia locale è già modificata, i test non sembrano dare segnali rossi (spero di aver impostato bene ;)) però non so come si fa il commit verso il repository (serviva anche una pass no?).
BlueDragon
06-10-2005, 23:50
Adesso penso di andare a nanna ma domani magari tiro fuori qualche test se non l'ha già fatto qualcun'altro.
Per quanto riguarda il "procedere al refactoring", la mia copia locale è già modificata, i test non sembrano dare segnali rossi (spero di aver impostato bene ;)) però non so come si fa il commit verso il repository (serviva anche una pass no?).
Ok, anziché andare a nanna ho fatto il commit del refactoring di
checkForCollision(float dx, float dy)
e dei 4 metodi di check collisioni sui bordi (ma senza modificare il <=0 in <0 per adesso).
Grazie fek per la disponibilità su Msn :)
E invece no!
Ti sei accorto che premendo prima la freccia a destra e poi la freccia a sinistra contemporaneamente il diamante si sposta a destra, mentre vuoi che stia fermo. Bug!
Da correggere, ma non prima di un bel test che lo esponga :)
come? Mi dici che non te n'eri accorto? Dettagli implementatitivi...
Della serie a volte ritornano... :rolleyes:
Appena fatto l'update, provato a far passare i test e passano tutti....ok, è tutto a posto fin'ora....lanciamo il tutto e....nooo, il background non viene caricato completamente (256*512) ma solo una piccola parte (256*40), mah..sarà normale? si passa avanti a "giocare" con il movimento,
destra --> destra
wow
sinistra --> sinistra
figata!
su --> come può andare su qualcosa che deve scendere per forza?
giu --> niente, bè, si vede che non è possibile mandare giu più velocemente la gemma...
diagonali --> ovviamente nulla
destra+sinistra (e contrario) --> nooooooooo, cos'è successo? il test fatto ai tempi non ci ha segnalato questo? come è possibile, qua urge qualche modifica...noi (o meglio jocchan :Prrr: ) lo vogliamo fermo, non questo:
Exception in thread "main" java.lang.IllegalArgumentException
at it.diamonds.Grid.insertGem(Grid.java:72)
at it.diamonds.Grid.updateGemAfterInput(Grid.java:320)
at it.diamonds.Grid.reactToInput(Grid.java:158)
at it.diamonds.Game.processInput(Game.java:156)
at it.diamonds.Game.main(Game.java:52)
Lo metterei a posto io, però c'è un piccolo problema... sono ancora in fase di studio di java e pian piano mi sto guardando tutto il codice di diamond... a voi esperti quindi... :D
ahio... sto iniziando a pensare che il refactoring di fek del codice di cionci non sia stata una gran bella idea dopotutto... :D
ok: è colpa di chet!! chi sistema il codice? :)
(io non mi smentisco!!! :D)
ahio... sto iniziando a pensare che il refactoring di fek del codice di cionci non sia stata una gran bella idea dopotutto... :D
ok: è colpa di chet!! chi sistema il codice? :)
(io non mi smentisco!!! :D)
Tu :)
E prima scrivi il test relativo.
Tu :)
E prima scrivi il test relativo. ok, mi prudevano giusto le mani per l'inattività :p
ragazzi, la sequenza di input da generare per mettere in risalto l'errore è piuttosto complessa, ho dovuto farmi aiutare dal debugger, comunque ecco qua il test fallimentare che ho aggiunto in TestGridReactionToInput:
public void testRapidInputs()
{
grid.insertGemIntoColumn(1, gem);
input.generateKey(KeyCode.vk_Right);
input.generateKey(KeyCode.vk_Left);
grid.reactToInput(input);
input.generateKey(KeyCode.vk_Left, false);
grid.reactToInput(input);
grid.reactToInput(input);
}
cercherò di risolverlo dopo pranzo, ma penso che sarà un lavoro molto lungo O_o'
errata coccige: il problema si verifica a causa dell'inserzione della stessa gemma in posizioni diverse; per correggere il problema degli "input rapidi" basta correggere quest'altro; ora provo a scrivere un altro test per quest'altro problema (ma quando pranzo oggi? :cry: )
eccolo qua, si trova in TestGrid:
public void testSameInsertion()
{
grid.insertGem(4, 3, gem);
try
{
grid.insertGem(4, 4, gem);
}
catch(IllegalArgumentException e)
{
return;
}
fail("same gem inserted at different positions");
}
l'altro test comunque non lo elimino, può sempre servire; ancora non ho committato nulla, cercherò di risolvere i test dopo pranzo.
Ottimo lavoro. della serie, "ottimo lavoro, e mo' ad implementarlo ti ci voglio!!" :D
questo problema non è di semplice soluzione; a me vengono in mente 3 possibili maniere di risolverlo:
1) doppio for in insertGem che cicla su tutta la matrice per vedere se la gemma è già inserita da qualche altra parte
2) aggiunta di un campo row e column in Gem che identificano la posizione nella rispettiva Grid (da impostarsi a (-1, -1) nel caso la Gem non sia inserita in nessun oggetto Grid)
3) implementazione di una hash map in Grid per ritrovare facilmente tutte le Gem inserite.
mi riesce difficile giudicare quale sia la soluzione più semplice: la prima è troppo complessa (troppo codice in una volta sola), la seconda comporta una duplicazione dell'informazione che potrebbe rivelarsi difficile da gestire e potrebbe portare errori, la terza ha lo stesso problema della prima ed inoltre non so se sia fattibile perché non conosco le hash map in Java e non so se il puntatore ad un oggetto può essere usato come hash dell'oggetto stesso.
beh, io per adesso opterei per la prima ma credo che alla fine della storia sia opportuno un bel refactoring in cui eventualmente si applicherà la terza.
Per ora vai con 1). non mi ero accorto che è stato aggiunto un metodo findGemCell: grazie a questo la soluzione 1 diventa molto più semplice; ora cerco di risolvere l'altro test.
PS: ho anche aggiunto un piccolo test a findGemCell che serve per il modo in cui uso quel metodo in insertGem.
ed anche questa è fatta ^^
ho committato tutto e la build dovrebbe essere verde.
resta solo da vedere perché l'abbassamento veloce non funziona più; lo faccio io?
ed anche questa è fatta ^^
ho committato tutto e la build dovrebbe essere verde.
resta solo da vedere perché l'abbassamento veloce non funziona più; lo faccio io?
C'e' un task che richiede di implementare l'abbassamento veloce?
C'e' un task che richiede di implementare l'abbassamento veloce? lo chiede il customer ^^
comunque hai ragione, aspetterò il task
C'e' un task che richiede di implementare l'abbassamento veloce?
C'era in una delle storie precedenti. La funzione è scomparsa durante questa storia.
ciao ;)
L'abbassamento veloce possiamo dire che era già incluso nei vecchi task del primo ciclo (o era il secondo?), comunque c'era. Quindi, 71104, puoi agire quando vuoi ^^
ora non c'è più perché la gestione dell'input è passata da Gem a Grid, ma non ci sono abbastanza test; provo a farne qualcuno che fallisce, poi li propongo qui sul forum, me li confermate e li implemento.
no, un momento, credo che invece abbia ragione fek: la funzionalità è cambiata e secondo me bisogna aspettare un altro task.
inizialmente noi abbiamo implementato un diamante fermo in mezzo allo schermo che si muoveva in 8 direzioni coi tasti freccia, poi abbiamo ignorato i movimenti verso l'alto, e poi abbiamo aggiunto il movimento lento dovuto alla gravità; tutto questo era gestito da Gem.
adesso invece la gestione dell'input da parte di Gem è stata rimossa e ne è stata implementata una nuova in Grid, diversa dalla precedente: i movimenti laterali sono istantanei e quello veloce verso il basso deve essere ancora implementato; il movimento dovuto alla gravità è un discorso a parte perché non fa parte della gestione dell'input, è una proprietà del particolare tipo di Sprite, Gem appunto.
quindi l'abbassamento veloce per ora non lo faccio: tutto quello che posso fare per ora invece è di finire di rimuovere completamente la vecchia gestione degli input di Gem: che ci sta a fare Gem.reactToInput che non viene usato? :D
comunque aspetto conferme.
Ok, allora aspettiamo il prossimo ciclo.
Credevo fosse un semplice fix, ma evidentemente non è così ;)
il movimento veloce in un certo senso l'ho aggiunto io, senza che venisse richiesto espressamente nel task :stordita: , il vecchio task infatti chiedeva:
Aggiungere un movimento continuo verso il basso di qualche pixel
al diamante che si deve fermare solo in caso di collisione col bordo
inferiore. (cisc: completato)
quindi penso che sia giusto lasciarlo ad una prossima storia
il movimento veloce in un certo senso l'ho aggiunto io, senza che venisse richiesto espressamente nel task :stordita: , il vecchio task infatti chiedeva:
Aggiungere un movimento continuo verso il basso di qualche pixel
al diamante che si deve fermare solo in caso di collisione col bordo
inferiore. (cisc: completato)
quindi penso che sia giusto lasciarlo ad una prossima storia cisc, il task che hai riportato tu non c'entra: il movimento verso il basso dovuto alla gravità è un discorso diverso dal movimento della gemma nelle 8 direzioni: il movimento verso il basso non l'hai aggiunto tu, faceva parte delle 8 direzioni e c'era ancora prima che tu mettessi l'effetto gravità. ;)
tu hai svolto quel task in maniera corretta.
vBulletin® v3.6.4, Copyright ©2000-2025, Jelsoft Enterprises Ltd.