View Full Version : [Refactoring] Tasks
Ok allora per vedere lo stato di attività del team propongo un po' di task, Iniziamo dalle cose più semplici e urgenti:
Ref.1 - Eliminazione Logging - Ufo13, Completato
Ref.2 - Eliminazione Dynamite - Bonfo, Completato
Ref.3 - Eliminazione Modalità Advanced - Baol, Quasi Completato
Ref.4 - La modalità advanced non deve più essere selezionabile dal menu - Jappilas, Completato
Ref.5 - Sistemare i TODO nel codice, non per forza tutti una persona.
Ref.6 - Refactoring dei test in it.diamonds.tests (non sottopackage per ora), anche qui non tutti una persona.
Ref.7 - Eliminazione del Bug postato da Bonfo in questo topic.
Vorrei la lista delle persone al lavoro per ogni task :)
jappilas
04-11-2006, 00:23
4o fatto, ora il menu funziona come prima dell' introduzione dell' advanced mode :)
Eccomi !!! :D
...un task, un task, un task.... :stordita:
Però nei prossimi giorni non ci sono. :cry:
Mi prendo il 2 e considerando che ho un sacco di cose da fare prevedo per mercoledì sera di aver committato il task. ;)
Si rinizia !!! :yeah:
Tanto sono task veloci, il primo l'ho finito in 30 minuti :)
Fatto.
Pero' la cosa non e' stata indolore; avevamo scritto un bel po' di codice per le dinamiti e toccavano molti punti nel codice.
Ha richiesto un po' di tempo e c'e' una cosa che ho fatto a sensazione.
Infatti per controllare lo stato delle Dinamiti era stato fatto un nuovo stato.
Ora eliminato questo stato non sapevo piu' in quale stato far transitare gli stati che passavano nello stato delle dinamiti (precisamente WaitBeforeNextGremsPair e GemsPairOnControlState).
Sono andato ad intuito, anche se dopo un po' non e' cosi' facile.
Ora compila e tutti i test sono verdi.
Pero' giocando ho beccato un bug
Che faccio..committo??
Ora non sono proprio in grado di beccare quel bug .... :muro: :muro:
Che faccio..committo??
Ora non sono proprio in grado di beccare quel bug .... :muro: :muro:
Nei tempi d'oro non te lo avrei permesso ma secondo me ora puoi fare il commit del tuo task anche cosi. Quando hai tempo scrivi il test per scatenare il bug e poi lo correggi.
ciao ;)
ALT...
...mi sono scordato che eliminando le dinmiti bisogna pure eliminare il padding.
Non dovrebbe incidere per nulla sul bug...ma non si sa mai.
Domani ci riguardo ;)
c'era un bug simile anche nell'ultima versione che mi ricordo...
Ho eliminato anche il padding ... ma ovviamente non e' cambiato nulla.
Sicuramente il mio bug dipende da un errone passagio tr aun state e l'altro.
Se quacno ha idea di come modifcare GemsPairOnControlState e WaitBeforeNextGemsPairState per eliminare il passaggio CheckDynamiteState saa' premiato con un nuovo task :D :D :D
Ottimo lavoro, appena ho tempo vedo di fixare il bug anche se non conosco quella parte. Se qualcuno lo fa prima mi avverta per piacere :P
Ho aggiornato il topic
Ottimo lavoro, appena ho tempo vedo di fixare il bug anche se non conosco quella parte. Se qualcuno lo fa prima mi avverta per piacere :P
Ho aggiornato il topic
Aspetta...io non ho mica ancora fatto il commit.
E' piu' forte di me, ho ancora paura per le mie ditine :D :D :D
Aspetta...io non ho mica ancora fatto il commit.
E' piu' forte di me, ho ancora paura per le mie ditine :D :D :D
Tanto siamo in fase refactoring, uscirà qualche bug, quando il codice sarà semplificato sistemeremo tutti i bug e rifattorizzeremo tutti i test :)
jappilas
08-11-2006, 00:10
Tanto siamo in fase refactoring, uscirà qualche bug, quando il codice sarà semplificato sistemeremo tutti i bug e rifattorizzeremo tutti i test :)
non vorrei introdurre regressioni e bug che poi non si riesca a stimolare con test aggiunti in seguito (direi che in questo periodo avere un' indicazione sulla test coverage sia piuttosto utile, per non dire cruciale) :O
poi, personalmente sarei tentato di riportare in auge la prassi introdotta da fek "a chi fa casino (me compreso) ditine spezzate " :p
nel frattempo mi sono dotato di un trinciasigari per ogni evenienza...
...scherzo eh :D
Dopo una lunga pensata (30 secondi) ho deciso di committare lo stesso...
...almeno rendo il codice disponibile a tutti ;)
Prendo il task 3, entro domenica pomeriggio dovrei avere concluso.
Cosa faccio con il codice e i test? Commento o cancello?
ps: eseguendo i test ottengo un messaggio di test fallito:
[junit] Testcase: TestAllTexturesLoadedBeforeStartPlaying (it.diamonds.tests.TestGameLoop): FAILED
[junit] expected:<44> but was:<43>
Ho controllato anche in GameLoop, in pratica ci sono 43 texture da caricare ora (probabilmente Bonfo ha tolto quella della Dinamite) quindi se non sbaglio il numero in quel test è da aggiornare (non si potrebbe metterlo dinamico? correggetemi se dico idiozie , eh :flower: ).
Prendo il task 3, entro domenica pomeriggio dovrei avere concluso.
Cosa faccio con il codice e i test? Commento o cancello?
ps: eseguendo i test ottengo un messaggio di test fallito:
[junit] Testcase: TestAllTexturesLoadedBeforeStartPlaying (it.diamonds.tests.TestGameLoop): FAILED
[junit] expected:<44> but was:<43>
Ho controllato anche in GameLoop, in pratica ci sono 43 texture da caricare ora (probabilmente Bonfo ha tolto quella della Dinamite) quindi se non sbaglio il numero in quel test è da aggiornare (non si potrebbe metterlo dinamico? correggetemi se dico idiozie , eh :flower: ).
No no :nonsifa: ....
...io sono passato da 45 a 44 (ho tolto solo la dynamite) e in locale da me i tets passano tutti ;)
No no :nonsifa: ....
...io sono passato da 45 a 44 (ho tolto solo la dynamite) e in locale da me i tets passano tutti ;)
Si', si', non volevo "accusarti" di nulla :D , semplicemente mi sono ritrovato quel problema e ho provato a fare ipotesi ;)
Sta di fatto che io mi ritrovo in GameLoop (ho appena fatto update) con solo 43 elementi in TEXTURES_TO_PRELOAD, quindi l'unico problema che mi è venuto in mente è che sia sbagliato il numero nel test. :stordita:
Se però a te i test passano... :boh:
Adesso provo a ricontrollare tutto. ;)
Task 3 completato.
Aspetto a committare perchè ho ancora il problema postato qui sopra.
Inserendo "43" a posto di "44" nel test a me funziona tutto, in locale l'ho modificata, ma aspetto che mi diciate cosa fare.
Tra l'altro giocando mi è comparso più volte il bug "scovato" da Bonfo e, sorpresa :rolleyes: , ne ho trovato un altro (BigGem che si eliminano invece di ingrossarsi) ma penso di avere una mezza idea sulla loro causa: secondo me l'ultimo refactoring di Bonfo non c'entra nulla, quei bug può darsi siano stati inseriti nel famoso necessario refactoring BigGem/Droppable.
Penso di poter scrivere i test che li scatenano, posso provare a lavorarci
Prenoto quindi il task 7 (e il 7 bis, ovvero la soluzione dell'altro bug) e cerco di finirlo per mercoledì sera (sto lungo perchè ho impegni :D )
ps: il refactoring di BigGem era ultimato? Mi ricordo che Bonfo ci aveva passato le notti ( :ave: ), alla fine BigGem si comporta definitivamente come Droppable? O mancava ancora qualcosa?
ps2: comunque (checchè ne diciate :Prrr: ) il codice di tutto il gioco è uno dei sorgenti più chiari che abbia mai letto, non ho fatto alcuna fatica a capire il funzionamento generale.
Che dire, complimenti! :sofico:
Task 3 completato.
Aspetto a committare perchè ho ancora il problema postato qui sopra.
Inserendo "43" a posto di "44" nel test a me funziona tutto, in locale l'ho modificata, ma aspetto che mi diciate cosa fare.
Tra l'altro giocando mi è comparso più volte il bug "scovato" da Bonfo e, sorpresa :rolleyes: , ne ho trovato un altro (BigGem che si eliminano invece di ingrossarsi) ma penso di avere una mezza idea sulla loro causa: secondo me l'ultimo refactoring di Bonfo non c'entra nulla, quei bug può darsi siano stati inseriti nel famoso necessario refactoring BigGem/Droppable.
Penso di poter scrivere i test che li scatenano, posso provare a lavorarci
Prenoto quindi il task 7 (e il 7 bis, ovvero la soluzione dell'altro bug) e cerco di finirlo per mercoledì sera (sto lungo perchè ho impegni :D )
ps: il refactoring di BigGem era ultimato? Mi ricordo che Bonfo ci aveva passato le notti ( :ave: ), alla fine BigGem si comporta definitivamente come Droppable? O mancava ancora qualcosa?
ps2: comunque (checchè ne diciate :Prrr: ) il codice di tutto il gioco è uno dei sorgenti più chiari che abbia mai letto, non ho fatto alcuna fatica a capire il funzionamento generale.
Che dire, complimenti! :sofico:
Per il problema del test:
// Total textures: 45
private static final String TEXTURES_TO_PRELOAD[] = { "back000.jpg",
"grid-background",
COMMON + "font_14x29", COMMON + "font_8x8", COMMON + "gameover",
COMMON + "main.jpg", COMMON + "main_menu", COMMON + "score_16x24",
COMMON_CRUSH + "02", COMMON_CRUSH + "03", COMMON_CRUSH + "04",
COMMON_CRUSH + "05", COMMON_CRUSH + "06", COMMON_CRUSH + "07",
COMMON_CRUSH + "08", COMMON_CRUSH + "09", COMMON_CRUSH + "over",
BOXES + "diamond", BOXES + "emerald", BOXES + "ruby",
BOXES + "sapphire", BOXES + "topaz",
FLASHING + "nocolor",
GEMS + "diamond", GEMS + "emerald", GEMS + "ruby", GEMS + "sapphire",
GEMS + "topaz",
STONES + "diamond", STONES + "emerald", STONES + "ruby",
STONES + "sapphire", STONES + "topaz",
TILES + "diamond", TILES + "emerald", TILES + "ruby",
TILES + "sapphire", TILES + "topaz",
LAYOUT + "counter", LAYOUT + "warning",
ICONS + "clock", ICONS + "desperation", ICONS + "tnt" };
questo e' quello che ho io in GameLoop, come vedi il commento dice ancora 45...e io ne ho tolta una sola :D
Confronta e vediamo che succede ;)
Per BigGem e Droppable...non e' finito! :muro:
Il problema e' che non mi ricordo piu' quali erano i problemi :( :(
Sicuramente uno me lo ricordo.
In quel refactoring la griglia da array a 2 dimendioni e' passata a lista di Droppbale, quindi ci operiamo sopra con un foreach per ottenre il riferimento per ogni Droppable.
Il problema e' che le action per ogni Droppable fanno dei foreach per fare quello che devono fare... e ovviamente tutto si incasina perche' ad un foreach vengono cambiate le carte in tavola dall'altro. :cry:
Dato che il codice era identico mi è venuto il dubbio...
Ho provato a far partire Windows (sono mesi che uso quasi solo Linux ormai), ho fatto Update e provato ad eseguire i test: passava tutto.
Allora ho fatto partire il debug (scusate :ave: ma non mi è venuto in mente alcun test per evidenziare la situazione...) e ho notato che le texture caricate erano effettivamente 44, ma una era caricata due volte: "main.jpg" era caricata sia come "gfx/common/main.jpg" sia come "gfx\\common\\main.jpg".
Gironzolando nel codice ho trovato questa linea in it.diamonds.GameLoop.setBackgrounMenu():
Background background = new Background(environment, "gfx" + File.separator + "common" + File.separator + "main", ".jpg");
Quello che penso io: in pratica su Windows il "File.separator" impostato da Java è "\\", quindi il gioco non carica la texture già preloadata ("gfx/common/main.jpg") ma ne carica una nuova ("gfx\\common\\main.jpg") che aumenta così il numero di Textures caricate (setBackgroundMenu viene richiamata nel costruttore di GameLoop, quindi la richiama anche il test).
Su Linux invece (File.separator = "/") setBackgroundMenu carica semplicemente la texture già precaricata, senza aggiungerne una nuova.
(Questo spiegherebbe inotre come mai in TEXTURES_TO_PRELOAD sono effettivamente solo 43 textures)
Per ora committo senza modificare quel test (che qui su Linux non passa neanche revertando tutto, quindi è comunque un problema della versione sul server), ma se mi confermate che stavolta l'ho imbroccata e che su altri utenti Linux quel test non passa elimino quei "File.separator" e li sostituisco con "/" (che tanto funzionano anche su Windows), quindi abbasso le textures a 43 nel test.
Ma File.Separator non era fatto apposta per evitare sti problemi?
Quoto TigerShark dal thread "Problemi":
I path sono assolutamente indipendenti dalla piattaforma anche con "/" anzi, è sbagliato usare il java.io.FileSeparator perchè quando ho fatto lo spike con java webstart ho dovuto buttare sangue a toglierli tutti perchè su windows NON potevano funzionare, in quanto java web start non usa il file system, ma utilizza direttamente il classloader per accedere ai files e si ritrovava dei backslash che non vogliono dire assolutamente nulla dato che il classloader usa gli slash.
Cmq dopo averli cambiati tutti ora mi ritrovo che anzichè caricare 44 texture ne carica 45...
ne avevamo parlato anche con cesare ai tempi e avevamo deciso di usare lo slash se non ricordo male perchè altrimenti ci possiamo dimenticare di usare java web start...
Oltre il fatto che secondo me la leggibilità del codice ci guadagna veramente tanto......
Questo tra l'altro era un cambiamento che era stato fatto parecchio tempo fa, ma solo nel branch java web start.
...
Ma cmq la questione dei file separator direi di lasciarla con gli slash dato che non hanno alcun svantaggio rispetto a java.io.FileSeparator, ma piuttosto rendono il codice più leggibile e soprattutto ci svincolano dal file system.
In sintesi, avete deciso di usare lo slash :D
Comunque non ci sarebbero stati problemi ad usare il FileSeparator (a parte con WebStart) se li aveste usati dappertutto, ma dato che nei path dichiarati in GameLoop avete usato "/"... su Linux fa casini.
E tra l'altro li faceva anche a TigerShark a quanto pare...
sto assistendo ad una cosa stranissima...
sto eliminando tutti i java.io.FileSeparator in moda da aumentare la leggibilità del codice e eventualmente nel caso in cui riprendiamo il progetto e usiamo webstart...
ma mi sono accorto di una cosa stranissima...
testAllTexturesLoadedBeforeStartPlaying ogni volta che faccio una sostituzione fallisce perchè si ritrova + texture di quelle che si aspetta...
Da cosa può dipendere questo comportamento?
Dopo aver letto queste righe sono sempre più convinto della mia soluzione (tra l'altro ho testato in Windows a fare come ho proposto e funziona, passa tutto, compila tutto, il gioco va, le textures vengono visualizzate).
Ah, dimenticavo, task 7 completato! :sofico:
C'era ancora CanMoveDown di AbstractDroppable che ragionava senza BigGems, l'ho modificata aggiungendoci l'altezza della Droppable e il bug non si ripresenta più.
Ho scritto un test relativo (o meglio, un paio di test), il problema è che tutti i test esistenti sono nello stato precedente, ovvero ragionano ancora pensando a BigGem e Droppable come cose separate. Non sapevo dove piazzare quel test, visto che tra l'altro testerebbe una funzionalità di AbstractDroppable e non di BigGem...
Lo piazzo in TestAbstractDroppable sebbene per ora ci sia tutt'altro? O aspettiamo di concludere definitivamente il refactoring di BigGem?
ps: se non vi piacciono i nomi delle variabili o della funzione "di supporto" cambiateli :D :Prrr:
pps: per l'altro bug la cosa è più complicata, mi sa che per quello è meglio aspettare il refactoring :stordita:
Ah, dimenticavo, task 7 completato! :sofico:
C'era ancora CanMoveDown di AbstractDroppable che ragionava senza BigGems, l'ho modificata aggiungendoci l'altezza della Droppable e il bug non si ripresenta più.
Ho scritto un test relativo (o meglio, un paio di test), il problema è che tutti i test esistenti sono nello stato precedente, ovvero ragionano ancora pensando a BigGem e Droppable come cose separate. Non sapevo dove piazzare quel test, visto che tra l'altro testerebbe una funzionalità di AbstractDroppable e non di BigGem...
Lo piazzo in TestAbstractDroppable sebbene per ora ci sia tutt'altro? O aspettiamo di concludere definitivamente il refactoring di BigGem?
ps: se non vi piacciono i nomi delle variabili o della funzione "di supporto" cambiateli :D :Prrr:
pps: per l'altro bug la cosa è più complicata, mi sa che per quello è meglio aspettare il refactoring :stordita:
GOOD WORK !!! :mano:
Appeno ho tempo scaricoe guardo ;)
AHIA....
....BUILD ROSSA. :mad:
La cosa da risolvere e' una cavolata, ma e' meglio che la metta a posto chi ha introdotto il problema ;)
... Jappy il checkstyle non lo guardiamo piu'!?! :stordita:
jappilas
17-11-2006, 12:59
... Jappy il checkstyle non lo guardiamo piu'!?! :stordita:quante storie per un' iniziale maiuscola invece che minuscola :D
ora checkstyle è contento, invece continuo ad avere la build rossa per un test che fallisce in una classe che SVN non ha mai nè riportato in conflitto nè di cui ha fatto il merge, con le due modifiche che ho fatto io... (sto controllando la version history dei 3 file coinvolti ma... ) :stordita:
infatti il problema era causato dalla diversa forma del path nel test e nel codice
in GameLoop.setMenuBackground():new Background(environment, "gfx/common/main", ".jpg");
in TestGameLoop.testMenuBackgroundIsShown():
new Background(environment, "gfx"
+ File.separator + "common" + File.separator + "main", ".jpg");risolto per adesso riportando la seconda forma anche in GameLoop, ma forse si era optato per eliminare il file.separator? ;)
risolto per adesso riportando la seconda forma anche in GameLoop, ma forse si era optato per eliminare il file.separator? ;)
Esatto, come ho postato più sopra (evito di ripetermi, va :D)
Su Linux fa casini coi test, quindi o si usa dappertutto File.separator o dappertutto "/".
E per i motivi che diceva ^TigerShark^ (anche questi postati più sopra) e visto che file.separator praticamente c'è solo in qualche test, direi che "/" è la cosa migliore :)
Ci pensi tu o converto tutto io? :cool:
jappilas
18-11-2006, 01:03
Esatto, come ho postato più sopra (evito di ripetermi, va :D)
Su Linux fa casini coi test, quindi o si usa dappertutto File.separator o dappertutto "/".
E per i motivi che diceva ^TigerShark^ (anche questi postati più sopra) e visto che file.separator praticamente c'è solo in qualche test, direi che "/" è la cosa migliore :)
Ci pensi tu o converto tutto io? :cool:
nell' ambito dell' eradicazione defiitiva del log ho lasciato la mia versione per il solo motivo che dovevo comunque modificare gameloop e passava anche il test sul numero delle textures
ma riconverti pure se hai voglia ;)
Convertito tutto. Test Modificato.
Qui su Linux passa tutto, fatemi sapere se è tutto ok anche con Windows :D
Se funziona, quel test rognoso non dovrebbe dare più problemi. ;)
Adesso provo a dare un'occhiata ai test, dato che i TODO che ho visto mi sono abbastanza incomprensibili :D (o comunque non saprei se e come mettere le mani :stordita: )
Eventualmente posso dare un'occhiata al bug rimasto e vedere se riesco a cavarne qualcosa...
Convertito tutto. Test Modificato.
Qui su Linux passa tutto, fatemi sapere se è tutto ok anche con Windows :D
Se funziona, quel test rognoso non dovrebbe dare più problemi. ;)
Adesso provo a dare un'occhiata ai test, dato che i TODO che ho visto mi sono abbastanza incomprensibili :D (o comunque non saprei se e come mettere le mani :stordita: )
Eventualmente posso dare un'occhiata al bug rimasto e vedere se riesco a cavarne qualcosa...
Ottimo lavoro. Per il bug mi sembra una buona idea :)
Anche l'altro bug è eliminato.
Modificato AddColumn di BigGem che aveva struttura diversa da AddRow e non doveva essere così. :D
A meno di future segnalazioni dovremmo essere a nessun bug aperto.
Adesso provo a mettermi sotto con i test.
jappilas
10-12-2006, 20:56
bene, molto bene :)
^TiGeRShArK^
11-12-2006, 13:11
vojo una connessione decente così posso fare qualcosa ank'iooooo!!!:cry::cry::cry:
vBulletin® v3.6.4, Copyright ©2000-2025, Jelsoft Enterprises Ltd.