View Full Version : Refactoring Post FirstPlayable
Allora...
...pronti a riniziare a lavorare :D :D
Io ho iniziato lavorando sugli state.
Sto facendo 3 cose:
1) spostando i metodi in modo che le prime tre cose che si incontrano siano:
- costruttore
- isCurrentState()
- update();
2) ogni stato che si occupa di gravità dovrà occuparsene internamente, ovvero se setta la gravità ad un valore diverso da normal, dopo deve esso stesso rimetterlo a normal
3) diminuire l'istanziazioni di classi state
Ho modificato WaitStateBeforeNewGemsPair.
Ora ritorna:
return new GemsPairOnControlState(config).update(timer,gridController);
mentre prima ritornava, praticamente, solamente
new GemsPairOnControlState(config);
e tutto funziona.
Invece in CrushState c'è questo:
return new GemFallState(config);
e con
return new GemFallState(config).update(timer, gridController);
invece non passano i test ??
Ora ci riguardo .... :D
Aspettate con i refactoring, per favore.
Ottimo refactoring!!!
...ha introdotto un bug che i test non mi hanno segnalato :cry: :cry:
Ora la gemsPair quando si spezza, se il tasto giù è premuto, non scende a gravità Strongest ma a Normal.
:muro: :muro:
Avete fatto commit per caso????
P.S.: Fek spezzerà personalmente le manine al primo che proverà a fare un commit da venerdi a lunedi' :p
Avete fatto commit per caso????
Sì...io ...ma fek ha detto che aveva preso il tag e che si poteva ripartire con i refactoring.... :cry: :cry:
Ora che la First Playable e' chiusa ed ho preso la tag dalla quale verranno prodotte le build da distribuire, il trunk e' di nuovo libero e potere iniziare i refactoring dei quali abbiamo parlato.
http://www.hwupgrade.it/forum/showpost.php?p=12094917&postcount=1
Ho perso le dita???
Sì...io ...ma fek ha detto che aveva preso il tag e che si poteva ripartire con i refactoring.... :cry: :cry:
http://www.hwupgrade.it/forum/showpost.php?p=12094917&postcount=1
Ho perso le dita???
Ha preso il tag, ma sono sorti altri problemi che ci conviene provare a risolvere.
Questo weekend doveva essere destinato al playtesting, credo sia sbagliatissimo cercare di accelerare i tempi e crearci da soli altri problemi.
Ok...reverto tutto!!!
O almeno ci provo :(
...fino a lunedì non tocco più nulla :D
Ok...reverto tutto!!!
O almeno ci provo :(
Aspetta, vediamo prima che dicono anche Fek e Vicius.
E' la mia opinione, ma se hanno detto così un motivo ci sarà.
REVERT ESEGUITO.
Fino a lunedì fermo i lavori "PUBBLICI"... :D :D
Oooops...non aqvevo letto. :cry:
in ogni caso mi sono salvato le modifiche che ho fatto...quindi quando vogliamo le rifaccio in 5 sec ;)
Ora vedo di andare avanti con il refactoring sulla mia copia locale ... poi per i play test uso quella della build. :D
Grazie Bonfo (e meno male che hai salvato le modifiche ;) ), vedrò di parlarne con Fek e Vic per cosa ci conviene fare.
Intanto, vediamo di aprire un altro topic per vedere se riusciamo a risolvere questi problemi di compatibilità sorti all'ultimo minuto?
Sicuramente sono più urgenti.
Certo...concentriamoci su quelli. :D
Sugli state però devo dire una cosa.
Ho notato come in crushState si faccia un return new FallGemState senza update. Lo capisco perchè rende difficili moltissimi test.
A questo punto mi viene da dire....o tutto o niente.
Ovvero o tutti fanno un return new STATE.Update() oppure non lo fa nessuno!!! :muro:
è una semplice scelta stilistica....tutto funziona uguale...solo bisogna mettere a posto i test di conseguenza.
Capire cosa succede quando faccio update è una delle cose che mi ha sempre fatto spendere un sacco di tempo nei test. Renderlo chiaro sarebbe fantastico.... :D :D :D
A questo punto mi verrebbe da dire di togliere gli update dai return....forse diventa tutto un po' più deterministico ;)
Colpa mia che ho fatto un po' di confusione, scusate. Ho preso il tag per lasciare il trunk libero per il refactoring. Potete tranquillamente continuare.
Bonfo, scrivi prima il test che copre la condizione che si e' verificata a seguito del tuo refactoring, cosi' da andare sul tranquillo.
Ora non posso....sto uscendo.
Appena torno a casa scrivo il test. ;)
Sarebbe utile anche avere un'opinione da parte di tutti per sapere che fare con questi update. :D
Fatto il test...ma anche con la versione corretta non passa.
Ecco il test:
public void testSlaveGemFallFaster()
{
controller.getGemsPair().rotateClockwise();
controller.getGemsPair().getPivotGem().drop();
controller.update(timer);
assertEquals(Config.createForTesting().getInteger("StrongestGravityMultiplier") * 0.5, grid.getActualGravity(), 0.01);
input.notifyKeyEvent(KeyEvent.DOWN, KeyEvent.PRESSED);
controller.reactToInput(timer);
assertEquals(Config.createForTesting().getInteger("StrongestGravityMultiplier") * 0.5, grid.getActualGravity(), 0.01);
input.notifyKeyEvent(KeyEvent.DOWN, KeyEvent.RELEASED);
controller.reactToInput(timer);
assertEquals(Config.createForTesting().getInteger("StrongestGravityMultiplier") * 0.5, grid.getActualGravity(), 0.01);
}
Il test non passa perchè in realtà sia grid che la slaveGem lo sentono benissimo che la gravità ritorna normale, solo che GemsPairOnControlState prima di far cadere la slaveGem risetta sempre e comunque la gravità a Strongest.
A questo punto o il test è sbagliato "concettualmente" oppure il modo in cui viene fatta cadere la salveGem non è pulitissimo.
Ora ci sono 2 possibiltà nel caso il test sia corretto:
o si spezza lo stato in GemsPairOnControlState in 2 aggiungendo lo stato SlaveGemFall in modo da cambiare questo codice
public void reactToInput(TimerInterface timer)
{
if(!currentState.isCurrentState("StoneFall") && !currentState.isCurrentState("GemFall")) && !currentState.isCurrentState("SlaveGemFall "))
{
inputReactor.reactToInput(timer);
}
}
oppure si fanno 2 metodi pubblici di gridController in modo da abilitare e disabilitare gli input e il relativo getter...così gli stati abilitano meno gli input e il metodo suddetto usa il getter per capire che fare.
A me paice molto più la soluzione 1 :D :D :D
P.S.: ma dove vengono disabilitati gli input per la salveGem dopo che la gemsPair si è separata??? Dentro gemsPair??
Sarà anche :spam:
..ma riuppo il thread perchè il test che ho postato è ancora lì che deve capire che fine deve fare.
Ripeto:il comportamento è strano.
In sti giorni ci sarò a sprazzi...ma dopo questo gli state sono abbastanza puliti.
Dopo passerò all'attaco degli action .... e lì si che ci sarà da fare a botte :boxe: :asd:
Ufo, puoi dare una mano a Bonfo che tu conosci benone questa parte?
Ufo, puoi dare una mano a Bonfo che tu conosci benone questa parte?
Certo, Bonfo contattami su MSN, non ci sono oggi pomeriggio quindi dovremmo fare sul tardi o subito :P
vorrei proporre una forma di organizzazione del refactoring, stile assegnazione del refactoring delle varie parti del codice stile task, però a rotazione, tipo un giorno 1 guarda l'audio, un'altro guarda l'engine, ecc, ecc, il giorno successivo a rotazione, in modo da evitare di lavorare nello stesso momento allo stesso codice....
Ok cisc. Scrivi qui l'elenco dei refactoring di cui abbiamo bisogno e li assegnamo.
cdimauro
24-04-2006, 11:55
Il problema principale è che roba come Engine/Texture richiederà parecchio tempo per la rifattorizzazione: questo vuol dire che MOLTO probabilmente alla sua conclusione, quando finalmente Ant ejetterà il suo misericordioso "SUCCESSFUL", qualcun altro avrà committato 2 miliardi di cambiamenti e alla sola idea della risoluzione dei conflitti e del merging del codice, l'harakiri rappresenterebbe la soluzione liberatoria... :(
Questo vuol dire che dobbiamo trovare il modo per spezzettare questo refactoring di Engine/Texture in piccoli passi.
E si', c'e' sempre il modo di spezzettare un grosso refactoring in piccoli passi, ci lavoriamo assieme se vuoi :)
io proponevo più un'assegnazione a pezzi, ovvero, tipo cdimauro oggi si occupa di rifattorizzare come crete Engine/Texture, dato che il lavoro è grosso lo possono fare più di uno su msn e magari prendersi 2 giorni di tempo, dopo qualcun'altro cerca do rifattorizzarlo...gli altri magari si occupano di rifattorizzare, chi l'auido, cioè, quello che voglio dire io è cercare di far studiare tutto il codice a tutti, evitando di lavorare sullo stesso codice e quindi evitando di creare conflitti
cdimauro
24-04-2006, 12:29
Questo vuol dire che dobbiamo trovare il modo per spezzettare questo refactoring di Engine/Texture in piccoli passi.
E si', c'e' sempre il modo di spezzettare un grosso refactoring in piccoli passi, ci lavoriamo assieme se vuoi :)
"Capitano, o mio capitano". :p
OK, per me va benissimo. :)
cdimauro
24-04-2006, 12:32
io proponevo più un'assegnazione a pezzi, ovvero, tipo cdimauro oggi si occupa di rifattorizzare come crete Engine/Texture, dato che il lavoro è grosso lo possono fare più di uno su msn e magari prendersi 2 giorni di tempo, dopo qualcun'altro cerca do rifattorizzarlo...gli altri magari si occupano di rifattorizzare, chi l'auido, cioè, quello che voglio dire io è cercare di far studiare tutto il codice a tutti, evitando di lavorare sullo stesso codice e quindi evitando di creare conflitti
Francamente lo vedo molto difficile anche da mettere in pratica: se modifichi, ad esempio, Engine in modo da generare le Texture, dovresti committare il codice (ergo: build rotta) e aspettare poi che un gruppo di persone, che s'è preso una fetta della rifattorizzazione, completi la loro parte di lavoro, che poi dovrebbero committare; poi bisognerebbe lanciare Ant e pregare che vada tutto a buon fine.
si, forse è un po complicato da mettere in pratica, però così rischiamo di lavorare tutti sullo stesso pezzo di codice.....
cdimauro
24-04-2006, 12:49
Per quello basta mettersi d'accordo e definire delle "aree di competenza".
si, forse è un po complicato da mettere in pratica, però così rischiamo di lavorare tutti sullo stesso pezzo di codice.....
Ho un'altra soluzione, cerchiamo di fare molti commit molto velocemente e ridurre al minimo la dimensione di ogni passo dei refactoring. Cosi' minimizziamo l'entita' di conflitti, soprattutto se prima di ogni commit facciamo un update.
Un altro vantaggio e' che ci permette di pensare meglio ai refactoring e ci costringe a suddividerli il piu' possibile.
Per quello basta mettersi d'accordo e definire delle "aree di competenza".
Preferirei che tutti lavorassero su tutto il codice e fossero competenti riguardo a tutto il codice.
Ufo, Bonfo, a che punto siamo con i refactoring?
Io purtroppo fino a domani sono un po' molto impegnato....
...quindi nada :(
In ogni caso ci sta lavorando Ufo ;)
Il test di Bonfo ora passa, adesso procedo con ancora un po' di refactoring :)
consiglio sul refactoring degli stati.
facciamoci dire da joachan quale deve essere l'esatta successione dei vari stati. Io in quel refactoring ho avuto parecchi problemi perche c'erano dei test che cozzavano un po fra di loro..
Gli state attuali in che ordine sono?
cdimauro
26-04-2006, 07:33
consiglio sul refactoring degli stati.
facciamoci dire da joachan quale deve essere l'esatta successione dei vari stati. Io in quel refactoring ho avuto parecchi problemi perche c'erano dei test che cozzavano un po fra di loro..
Tra l'altro non è semplice avere un quadro chiaro dei vari stati e delle possibili transizioni: forse una funzione o matrice di transizione renderebbe la vita molto più facile a tutti. ;)
Tra l'altro non è semplice avere un quadro chiaro dei vari stati e delle possibili transizioni: forse una funzione o matrice di transizione renderebbe la vita molto più facile a tutti. ;)
Si', sarebbe utile disegnare un grafo di transizione degli stati prima di concludere il refactoring.
cdimauro
26-04-2006, 14:43
Nel frattempo ho completato una mega rifattorizzazione (la maggior parte dei file sono stati modificati), introducendo la classe Environment (e relativi test) e cambiando il codice in modo da propagarla al posto di Config.
Per ovvi motivi ho dovuto cambiare molti test e ho trovato anche qualche magagna che ho sistemato, tipo metodo setUp del test padre non richiamato dal figlio oppure l'uso di Sound senza utilizzare Audio per crearlo (quest'ultimo l'ho sistemato soltanto creando un'istanza di Audio e inserendola in Environment, ma andrebbero cambiati proprio i test in modo da usare Audio.createSound())).
I test passano, la build è verde e il gioco sembra funzionare (anche se in slow motion, ho visto che è partito e che le gemme scendevano).
Nel frattempo ho completato una mega rifattorizzazione (la maggior parte dei file sono stati modificati), introducendo la classe Environment (e relativi test) e cambiando il codice in modo da propagarla al posto di Config.
Per ovvi motivi ho dovuto cambiare molti test e ho trovato anche qualche magagna che ho sistemato, tipo metodo setUp del test padre non richiamato dal figlio oppure l'uso di Sound senza utilizzare Audio per crearlo (quest'ultimo l'ho sistemato soltanto creando un'istanza di Audio e inserendola in Environment, ma andrebbero cambiati proprio i test in modo da usare Audio.createSound())).
I test passano, la build è verde e il gioco sembra funzionare (anche se in slow motion, ho visto che è partito e che le gemme scendevano).
Ho notato anche io :)
cdimauro
26-04-2006, 15:06
Ho anche spostato Environment come primo parametro, dove serviva: prima Config era messo all'inizio o alla fine.
Ho anche provveduto a sistemare la creazione delle istanze cercando di seguire il seguente ordine, ove possibile:
- Config
- Engine
- Audio
- Keyboard
- Timer
Ci sarebbe ancora da andare a caccia di qualche createForTesting di Config & co., in modo da verificare che siano stati fatti esclusivamente usando environment.setClasse(), ma non ho avuto tempo. Sono a pezzi. :(
Ho fatto un refactoring di Timer per sincronizzare tutti gli eventi interni al gioco...
Ho fatto un refactoring di Timer per sincronizzare tutti gli eventi interni al gioco...
Ci posti i dettagli?
Ho guardato il codice di Timer a casa, ma non ho avuto il tempo di postarlo.
A parole quello che fa e' memorizzare il tempo trascorso dalla creazione del timer al momento della chiamata ad advance() e poi restituisce quel tempo in getTime().
Questo cambiamento risolve il problema evidenziato da cionci, quindi va bene, e' un comportamento del quale abbiamo bisogno. Questo cambiamento pero' rompe il contratto di Timer, quello che ha scritto cionci non e' piu' un Timer, e' qualcos'altro: abbiamo un problema fondamentale di design che va risolto subito.
Il problema nasce dal fatto che Timer non fa' piu' quello che c'e' scritto sulla scatola, non rispetta piu' il suo contratto naturale, che puo' essere scritto in questa forma:
long timeStamp1 = timer.getTime();
Sleep(timerResolution);
long timeStamp2 = timer.getTime();
assertNotEqual(timeStamp1, timeStamp2);
In parole: due chiamate al timer successive separate da un tempo pari alla risoluzione del timer tornano due time stamp differenti.
Un nuovo programmatore su Diamonds, all'oscuro di tutto, guarderebbe la classe Timer, vedrebbe un metodo getTime() (dovrebbe essere chiamato getCurrentTime() per essere ancora piu' precisi), non vedrebbe alcun test a descriverne il contratto e la userebbe in maniera naturale seguendo il contratto che ho scritto sopra. Intuitivamente e' la prima cosa alla quale si pensa vedendo un timer, perche' modella lo scorrere del tempo: due istanti di tempo diversi hanno valori diversi. Scoprirebbe pero' amaramente, magari dopo molto tempo di debugging, che il nostro Timer non rispetta quel contratto!
Rispetta invece questo:
long timeStamp1 = timer.getTime();
Sleep(timerResolution);
long timeStamp2 = timer.getTime();
assertEqual(timeStamp1, timeStamp2);
timer.advance(timerResolution);
long timeStamp3 = timer.getTime();
assertNotEqual(timeStamp1, timeStamp3);
In parole: per far avanzare il Timer si deve interporre una chiamata ad advance(). Se tutto il codice fosse scritto cosi', dove una classe non rispetta il suo contratto naturale che il suo nome e il nome dei suoi metodi suggeriscono intuitivamente, il progetto diventerebbe presto ingestibile. E ci sono molti progetti cosi'. Non Diamonds.
La soluzione al problema di design e' togliere Timer dall'imbarazzo, restituirle il suo contratto naturale e introdurre un nuovo concetto, una nuova classe, che posso chiamare al momento GameTurn magari, ma che deve uscire dall'uso che se ne fa nel codice, che modella esattamente questo nuovo comportamento e che ha il nuovo contratto, ed e' implementata in termini del Timer che rispetta il suo contratto.
Una possibile soluzione veloce che fa scrivere poco codice e' ereditare GameTurn dal Timer, magari chiamarlo GameTimer e passarlo laddove il gioco ha bisogno di un Timer. Questa soluzione continuerebbe a non essere ideale perche' il nuovo GameTimer non "e' un" Timer, non rispetta il contratto di Timer, non rispetta il principio di sostituzione, non puo' essere usato ovunque puo' essere usato Timer.
Favor Composition Over Inheritance: abbiamo bisogno di un nuovo concetto, che non abbia legami con il Timer, che si limiti ad usarlo, che rispetti il nuovo contratto e fornisca il comportamento voluto. E dobbiamo usarlo nel gioco al posto del Timer. In Eclipse sono pochi colpi di refactoring e i nuovi programmatori di Diamonds ci ringrazieranno. Va fatto subito.
jappilas
04-05-2006, 11:21
Ottima lezione Fek ;)
stavo pensando la tessa cosa :)
e che quasi quasi sarebbe il caso di raccogliere le sue lezioni (e il materiale e' molto) in un thread ad hoc (se non un volume), come promemoria :O
stavo pensando la tessa cosa :)
e che quasi quasi sarebbe il caso di raccogliere le sue lezioni (e il materiale e' molto) in un thread ad hoc (se non un volume), come promemoria :O
Grazie, ma ci sono gia' i volumi a riguardo, io non invento nulla :)
jappilas
04-05-2006, 12:05
Grazie, ma ci sono gia' i volumi a riguardo, io non invento nulla :)
of course not... ma mi pare che in un volume si cerchi un deteminato argomento se si sa almeno cosa cercare, o lo si approfondisca se lo si conosce almeno superficialmente, o se ne indaghi la base teorica conoscendo gia' li' implicazione pratica...
pensavo al "fek - alogo" invece come a una raccolta di perle di saggezza ed esperianza in forma di vademecum e promemoria da leggere, capire e ricordare in modo immediato, e applicare sempre, pena la rottura delle ditine :D
OK, smetto di inquinare il thread :O
Leggi Code Complete 2, Exceptional C++ di Sutter e Coding Standards sempre di Sutter e Alexandrescu e ci trovi il 99% di quello che scrivo qui e tantissime altre cose in piu'.
Poi aggiungi Refactoring, Refactoring to Patterns e TDD e ne sai molto piu' di me :D
Sono d'accordo, l'avevi già evidenziato...ma la situazione è questa:
- abbiamo bisogno di una classe Timer ?
- forse abbiamo bisogno di una classe Timestamp ?
Il problema è che secondo me advance non aveva senso nemmeno prima in Timer...
La mia proposta:
Rinominare TimerInterface in TimestampInterface
Rinominare TimerInterface in TimestampInterface
Rinominare MockTimer in MockTimestamp...
A questo punto mi sembra che il contratto di Timestamp sia rispettato...
Anche l'idea di aggiungere una classe GameTurn non è male, ma secondo me questo non è un GameTurn...perchè il turn deve essere qualcosa di statico e prevedibile... Al contrario l'avanzamento del turno in questo caso non porterebbe a risultati prevedibili (nel senso che il turno 110 di una partita si ptorebbe svolgere ad un timestamp diverso da quello di un'altra partita)...
L'entità GameTurn deve essere IMHO accoppiata a degli intervalli di tempo costanti (ad esempio InputRate e UpdateRate)...
Il GameTurn potrebbe essere l' n-esima GemsPair inserita.
:sofico: :sofico:
Ovviamente in base alla partita cambiano i tempi...ma penso che il gameTurn nel log non serva per controllare i tempi, ma per poter gestire la successione degli eventi.
EDIT: comunque non capisco cosa ci servano 2 log...con la presenza delle stone, qualunque log da solo non definisce in modo univoco l'andamento della partita.
Il GameTurn potrebbe essere l' n-esima GemsPair inserita.
:sofico: :sofico:
Ovviamente in base alla partita cambiano i tempi...ma penso che il gameTurn nel log non serva per controllare i tempi, ma per poter gestire la successione degli eventi.
EDIT: comunque non capisco cosa ci servano 2 log...con la presenza delle stone, qualunque log da solo non definisce in modo univoco l'andamento della partita.
Dipende da come scrivi il log :)
Dipende da come scrivi il log :)
Tanto va rimesso a posto...ed il log delle stone sarà obbligatorio ;)
è uno di quegli eventi da scrivere nel log(fra l'altro uno dei piu particolari, visto che vanno scritte tutte le stone(con tipo inserite).
dovevo guardarci ieri sera, ma un aperitivo + cena imprevisto nn mi ha permesso di farlo..
Anche l'idea di aggiungere una classe GameTurn non è male, ma secondo me questo non è un GameTurn...perchè il turn deve essere qualcosa di statico e prevedibile... Al contrario l'avanzamento del turno in questo caso non porterebbe a risultati prevedibili (nel senso che il turno 110 di una partita si ptorebbe svolgere ad un timestamp diverso da quello di un'altra partita)...
L'entità GameTurn deve essere IMHO accoppiata a degli intervalli di tempo costanti (ad esempio InputRate e UpdateRate)...
Secondo me ha un senso che il GameTurn memorizzi il TimeStamp all'inizio del turno e si faccia carico di dare le temporizzazioni. Sostanzialmente quello che fa Timer ad esso. GameTurn non credo debba essere un'interfaccia, puo' essere mokkato in maniera piuttosto semplice.
Puoi provare ad esplorare quest'idea?
Ci posso provare solo lunedì...comunque ti presento l'idea...
GameTurn viene creato dove si controlla che venga eseguito l'update (a questo punto si contano gli update)... Ad ogni ciclo di update ho un nuovo GameTurn...l'InputRate quindi va tolto e dimensionato sul GameTurn... Bisogna fare una reazione agli Input ogni 4 GameTurn per matenere le proporzioni attuali... Il timestamp verrebbe memorizzato in GameTurn...
Il problema è che non vedo la necessità di propagare GameTurn, ma solo quella di propagare il long del timestamp... Infatti l'informazione sul GameTurn serve solo al momento della scrittura del log che si trova in PlayField, proprio dove dovremmo incrementare il GameTurn...
Quindi a occhio, se vuoi riportare il Timer alla struttura originale mi sembra meglio propagare il long relativo al timestamp e modificare un po' la struttura degli update e di reactToInput per lavorare sullo stesso timestamp...
è uno di quegli eventi da scrivere nel log(fra l'altro uno dei piu particolari, visto che vanno scritte tutte le stone(con tipo inserite).
Non basta il pattern (identificato con un intero) ed il numero di pietre ?
Non basta il pattern (identificato con un intero) ed il numero di pietre ?
puo essere, devo capire ancora come funziona il pattern
Sto effettuando un po' di revisione del codice...
Questo metodo in Grid non è testato:
public void moveDroppableToCell(Droppable droppable, int row, int column)
Questo metodo in AbstractDroppable non è testato:
public void moveToCell(int row, int column)
Io ho 0 tempo fino a venerdì. :(
Però, siccome abbiamo codice un po' in bilico, direi di fare il punto della situazione sui BIG REFACTORING, ovvero quelli in atto e quelli che sarebbero da fare.
Se siete d'accordo posso adirittura modificare il primo post del topic per tenerne traccia.
Refactoring:
- Environment (cdimauro)
- Grid e trasformazione di BigGem in Droppable
- Omogenizazzione degli update degli State
- Action: definire meglio le responsabilità rispetto a Grid
- SoundBank
- TextureBank
Tra parentesi ho messo il nome di chi per adesso ha sotto-mano il refactoring.
Se ho sbagliato qualcosa, se ho dimenticato oppure aggiunto troppa roba... faccio in fretta a modifcare :D :D
Ci posso provare solo lunedì...comunque ti presento l'idea...
GameTurn viene creato dove si controlla che venga eseguito l'update (a questo punto si contano gli update)... Ad ogni ciclo di update ho un nuovo GameTurn...l'InputRate quindi va tolto e dimensionato sul GameTurn... Bisogna fare una reazione agli Input ogni 4 GameTurn per matenere le proporzioni attuali... Il timestamp verrebbe memorizzato in GameTurn...
Il problema è che non vedo la necessità di propagare GameTurn, ma solo quella di propagare il long del timestamp... Infatti l'informazione sul GameTurn serve solo al momento della scrittura del log che si trova in PlayField, proprio dove dovremmo incrementare il GameTurn...
Quindi a occhio, se vuoi riportare il Timer alla struttura originale mi sembra meglio propagare il long relativo al timestamp e modificare un po' la struttura degli update e di reactToInput per lavorare sullo stesso timestamp...
Si'. mi pare un buon piano, anche il propagare solo il timestamp.
Io ho 0 tempo fino a venerdì. :(
Però, siccome abbiamo codice un po' in bilico, direi di fare il punto della situazione sui BIG REFACTORING, ovvero quelli in atto e quelli che sarebbero da fare.
Se siete d'accordo posso adirittura modificare il primo post del topic per tenerne traccia.
Refactoring:
- Environment (cdimauro)
- Grid e trasformazione di BigGem in Droppable
- Omogenizazzione degli update degli State
- Action: definire meglio le responsabilità rispetto a Grid
- SoundBank
- TextureBank
Tra parentesi ho messo il nome di chi per adesso ha sotto-mano il refactoring.
Se ho sbagliato qualcosa, se ho dimenticato oppure aggiunto troppa roba... faccio in fretta a modifcare :D :D
Abbiamo bisogno che questi refactoring siano conclusi prima di Venerdi', oppure nel prossimo Ciclo presentiamo una Storia solo di refactoring. Prenotatevi.
Abbiamo bisogno che questi refactoring siano conclusi prima di Venerdi', oppure nel prossimo Ciclo presentiamo una Storia solo di refactoring. Prenotatevi.
EDIT: Possiamo aggiungere questi task alla storia già presente, ed al limite rinviamo quelli non conclusi, così nessuno si annoia perchè non ci sono task :p
Che dici?
EDIT: Possiamo aggiungere questi task alla storia già presente, ed al limite rinviamo quelli non conclusi, così nessuno si annoia perchè non ci sono task :p
Che dici?
Buona idea :)
cdimauro
08-05-2006, 08:02
Sono d'accordo, l'avevi già evidenziato...ma la situazione è questa:
- abbiamo bisogno di una classe Timer ?
- forse abbiamo bisogno di una classe Timestamp ?
Il problema è che secondo me advance non aveva senso nemmeno prima in Timer...
La mia proposta:
Rinominare TimerInterface in TimestampInterface
Rinominare TimerInterface in TimestampInterface
Rinominare MockTimer in MockTimestamp...
A questo punto mi sembra che il contratto di Timestamp sia rispettato...
A mio avviso Timer deve rimanere, perché serve perfettamente per lo scopo per cui è stato creato: ritornare il "tempo (di sistema)" corrente e mettere in attesa in task per un certo lasso di tempo.
Se servono nuove funzionalità (TimeStamp e/o GameTurn), si possono benissimo creare a parte. ;)
cdimauro
08-05-2006, 08:07
Io ho 0 tempo fino a venerdì. :(
Però, siccome abbiamo codice un po' in bilico, direi di fare il punto della situazione sui BIG REFACTORING, ovvero quelli in atto e quelli che sarebbero da fare.
Se siete d'accordo posso adirittura modificare il primo post del topic per tenerne traccia.
Refactoring:
- Environment (cdimauro)
- Grid e trasformazione di BigGem in Droppable
- Omogenizazzione degli update degli State
- Action: definire meglio le responsabilità rispetto a Grid
- SoundBank
- TextureBank
Tra parentesi ho messo il nome di chi per adesso ha sotto-mano il refactoring.
Se ho sbagliato qualcosa, se ho dimenticato oppure aggiunto troppa roba... faccio in fretta a modifcare :D :D
SoundBank è già implementato (è un lavoro che ha fatto fek).
Per TextureBank il lavoro da fare è lo stesso e puoi inserirlo nel refactoring di Environment.
cdimauro
08-05-2006, 08:08
Abbiamo bisogno che questi refactoring siano conclusi prima di Venerdi', oppure nel prossimo Ciclo presentiamo una Storia solo di refactoring. Prenotatevi.
Io sono di nuovo in pista: spero di riuscire a completare il tutto entro venerdì.
In ogni caso il lavoro di rifattorizzazione per me non finisce venerdì: è un processo continuo fino alla fine del progetto. :p
cdimauro
08-05-2006, 09:05
Per prima cosa farei saltare Config.createForTesting, visto che il codice di produzione fa esattamente la stessa cosa: lo rinominerei in Create e farei usare questo metodo al codice di produzione e a quello di test.
Inoltre, visto che sia per i test sia per il codice di produzione oltre a Environment abbiamo sempre bisogno di creare e usare un'istanza di Config, farei in modo che venisse creata automaticamente alla creazione di Environment, togliendo quindi di mezzo tutti i metodi e i riferimenti alla creazione di Config dalla code base.
Che ne dite?
Refactoring:
- Environment (cdimauro)
- Grid e trasformazione di BigGem in Droppable
- Omogenizazzione degli update degli State
- Action: definire meglio le responsabilità rispetto a Grid
- SoundBank
- TextureBank
Prima di tutto direi che va finito il refactoring di Droppable ed AbstractDroppable creando le ultime interfacce ed eliminando alcune duplicazioni.
Inoltre sono presenti metodi non testati...
Tutti i TODO del codice dovrebbero sparire in fretta perchè poi si finisce a portarseli dietro a lungo (addirittura c'è un test commentato con scritta una cosa tipo "qualcuno lo faccia passare" :D)
Si'. mi pare un buon piano, anche il propagare solo il timestamp.
Ok...mi butto a farlo...
Prima di tutto direi che va finito il refactoring di Droppable ed AbstractDroppable creando le ultime interfacce ed eliminando alcune duplicazioni.
Inoltre sono presenti metodi non testati...
Tutti i TODO del codice dovrebbero sparire in fretta perchè poi si finisce a portarseli dietro a lungo (addirittura c'è un test commentato con scritta una cosa tipo "qualcuno lo faccia passare" :D)
Anche questo va aggiunto alla Storia 2. Anche il prossimo Ciclo avra' una Storia interamente dedicata a organizzare e razionalizzare questi refactoring.
cdimauro
08-05-2006, 13:25
Finito un altro grosso refactoring che ha interessato Environment, Engine, Audio, Keyboard, Timer & derivati & test.
Adesso per creare un'istanza di Engine, Audio, ecc. sono a disposizione i rispettivi metodi di Environment (createEngine, createAudio, ecc.).
Non esistono più riferimenti diretti ai Mock o a metodi "forTesting" (es: Engine.createForTesting), ma tutto è stato incapsulato dentro Environmente, che a seconda di come viene creato (con create o createForTesting) si prende la briga di creare Engine o MockEngine, ad esempio.
Ho aggiunto anche un metodo a Environment, isForTesting(), per sapere se si sta lavorando in ambiente di test o di produzione: sarà utile per togliere di mezzo tutti i vari createForTesting che sono ancora sparsi per il codice in mezzo alle altre classi (Grid, Gem, ecc. ecc. ecc.), ma per questa rifattorizzazione se ne parlerà in futuro (intanto un esempio è rappresentato da GameLoop: ho tolto di mezzo i due metodi createForTesting, e il codice s'è accorciato ed è diventato più leggibile ;)).
Ho lasciato ancora "in vita" i precedenti metodi set (setEngine, setAudio, ecc.) che avevo creato nella precedente rifattorizzazione, ma se siete d'accordo li toglierei di mezzo, perché non servono più.
Un'altra cosa che vorrei fare, ma dopo la rifattorizzazione di Texture che attualmente ha priorità più elevata, è quella di aggiungere un controllo ai vari createEngine, createAudio, ecc. di Environment in modo dasollevare un'eccezione nel caso in cui il corrispondente elemento sia già stato creato.
Ma come :cry proprio mentre facevo il rifactoring del Timer me lo cambi ?!?!?!
cdimauro
08-05-2006, 13:32
Mi spiace, non potevo sapere. :(
Non puoi fare il merge del codice?
No...praticamente tutti i file che avevo modificato andavano in conflitto...
cdimauro
08-05-2006, 13:56
ARGH. :( Io pure avevo modificato tantissimi file. :(
A questo punto è meglio che aspetto che tu finisca prima di rimettere mano alle altre rifattorizzazioni che devo apportare al codice...
Ho lasciato ancora "in vita" i precedenti metodi set (setEngine, setAudio, ecc.) che avevo creato nella precedente rifattorizzazione, ma se siete d'accordo li toglierei di mezzo, perché non servono più.
Toglili pure.
Dunque mezz'ora fa ho fatto un po di pulizia in diamonds.grid ed ora vorrei mettere un po' di ordine anche in diamonds.gems.
Quello che ho in mente è rinominare .gems in .droppable e lasciare solo le classi più importanti come Droppable* Direction ecc. per spostare le varie Gem, Stone, BigGems in un sotto package .ancora_da_definire.
Ho fatto una prova e i file modficati sono praticamente tutti. Quindi vorrei il via libera da tutti perché non vorrei distruggere il lavoro di qualcuno :)
Inoltre che nome proponete per il package? .gems non mi sembra appropriato ma è l'unica cosa che mi è venuta in mente. :(
ciao ;)
Aspetta un attimo...ci sono quasi...
cdimauro
08-05-2006, 14:26
x VICIUS Aspetta che finisca il lavoro di Riccardo e poi procedi, altrimenti succede un disastro. :p
Io per oggi non arrivo a fare altre rifattorizzazioni: se ne parla domani, ma non dovrei modificare molti sorgenti, spero (devo rifattorizzare "TextureBank").
EDIT: ho appena visto che ha risposto lui. :p
Non riesco a risolvere alcuni problemi... Sono arrivato a rifattorizzare playField, ma mi falliscono un mucchio di test e non ne capisco il motivo... Poi con quell'problema ad Eclipse sto impazzendo...
Non riesco a cavarci le gambe e devo andare... Per ora mi fermo qui... Se volete andare avanti fate pure... Non riesco a capire come mai questi test falliscono...
Nessun problema. I vostri hanno priorità maggiore. Domani sera se avete finito committo tutto.
ciao ;)
cdimauro
08-05-2006, 15:24
Fino a domattina io di sicuro non posso lavorarci: se vuoi, puoi benissimo farlo tu e committare. ;)
Vai pure Vic...non riesco proprio a venirne a capo...
cdimauro
08-05-2006, 15:54
Se hai bisogno di una mano domani mattina apriamo un apposito thread e ne discutiamo assieme (anche agli altri, ovviamente).
Ok. Commit fatto. Ne ho approfittato per fare il merge anche un'altra modifica a GemsPair che avevo qui in locale. :)
ciao ;)
Bel lavoro tutti quanti, lo sto seguendo dalla build machine.
Ora mi servono le seguenti cose:
- Timer riportato al suo originale comportamento
- Il test sulle texture caricate disaccoppiato dagl'altri test
Ho beccato un bel baco: Le animazioni delle gemme non funzionano più. :eek:
Escluderei i miei due ultimi commit come causa anche perché non mi sembra di aver toccato engine o sprite. almeno non direttamente.
ciao ;)
Provo a fixare...
Comunque chiedo l'aiuto di qualcuno perchè non riesco a venire a capo ai problemi che ho incontrato cercando di riportare dalla situazione attuale:
PlayField.update() e PlayFiled.reactToInput()
a
PlayField.update(long timeStamp) e PlayFiled.reactToInput(long timeStamp)
L'obiettivo è quello di passare ad entrambi i metodi lo stesso timeStamp...
Circa 7-8 test alla fine non mi passano, comunque tutti test a livello di GameLoop...
IMHO riportare Time al comportamente originario può essere fatto anche subito, reintroducendo quei piccoli problemi che si avevano prima e che avevo rivelato con il mio fix... Problemi che poi si dovrebbero risolvere automaticamente passando lo stesso timeStamp ad entrambe i metodi...
Ho trovato il problema relativo alle animazioni...
Non capisco una cosa... Perchè le action che non hanno uno stato interno significativo sono state preistanziate in Grid ?
IMHO andrebbero istanziate al momento dell'uso, istanziarle prima è altrimenti è YAGNI...
Il problema per la nimazioni era proprio questo...
public void updateDroppableAnimations(long timer)
{
if(updateAnimationsAction == null)
{
updateAnimationsAction = new UpdateAnimationsAction(timer);
}
forEachDroppable(updateAnimationsAction);
}
Corretto in questo modo:
public void updateDroppableAnimations(long timer)
{
forEachDroppable(new UpdateAnimationsAction(timer));
}
senza contare che cambiare quell'if non ha provocato una minima reazione nei test...e quindi vuol dire che quello era codice non testato...
Ho trovato il problema relativo alle animazioni...
Non capisco una cosa... Perchè le action che non hanno uno stato interno significativo sono state preistanziate in Grid ?
IMHO andrebbero istanziate al momento dell'uso, istanziarle prima è altrimenti è YAGNI...
Ok YAGNI....ma scordarsi che istanziare ad ogni update una nuova action riempa l'heap mi sembra troppo!!! :D
Ok...c'è il garbage collector....ma se riusciamo ad istanziare solo una volta le classi che usiamo sempre penso sia meglio ;)
EDIT:
comunque lì il problema non era la preistanziazione.
Il problema era che la creazione è dipendente dal valore del timer.... una creazione così a me sinceramente non piace. :mad: :mad:
Guardando il nuovo package it.diamonds.droppable.gems salta subito all'occhio qualcosa di strano:
-BigGemList
-BigGemTileType
Perchè non ci sono le liste anche degli altri droppables? ...forse perchè bigGem per adesso non è ancora un droppable?? :D
E il tileType forse non sarebbe meglio se fosse all'interno di BigGem...casomai come classe privata?? Alla fine in tutta l'applicazione solo bigGem ha bisogno di conoscere quella classe...e come al solito questa cosa puzza di refactoring. ;)
Sbaglio??
Ok YAGNI....ma scordarsi che istanziare ad ogni update una nuova action riempa l'heap mi sembra troppo!!! :D
Ok...c'è il garbage collector....ma se riusciamo ad istanziare solo una volta le classi che usiamo sempre penso sia meglio ;)
Non sono d'accordo...abbiamo scelto Java per scordarci della gestione della memoria... Se vediamo che questa scelta crea leak non recuperabili oppure piena lo heap prima dell'intervento del garbage collector allora possiamo tornare indietro, ma istanziarli al momento dell'uso non fa che semplificare Grid...
E il tileType forse non sarebbe meglio se fosse all'interno di BigGem...casomai come classe privata??
Abbiamo scelto di non usare classi private...
Guardando il nuovo package it.diamonds.droppable.gems salta subito all'occhio qualcosa di strano:
-BigGemList
-BigGemTileType
Perchè non ci sono le liste anche degli altri droppables? ...forse perchè bigGem per adesso non è ancora un droppable?? :D
E il tileType forse non sarebbe meglio se fosse all'interno di BigGem...casomai come classe privata?? Alla fine in tutta l'applicazione solo bigGem ha bisogno di conoscere quella classe...e come al solito questa cosa puzza di refactoring. ;)
Sbaglio??
Stai complicando il codice in base ad un'assunzione di carattere prestazionale che non deriva dal risultato di un profiler?
Se due soluzioni hanno complessita' equivalente, scegli quella che instanzia meno oggetti. Se due soluzioni hanno complessita' differente, scegli quella piu' semplice, a meno che un profiler non ti dica che quel codice presenta un problema di carattere prestazionale.
Tutte le ottimizzazione vanno fatte con in mano i risultati di un profiler.
E poi, anche con i dati di un profiler in mano, la prima ottimizzazione che devi affrontare e' di carattere algoritmico, non un'ottimizzazione a basso livello come togliere l'allocazione di un oggetto.
Solo dopo aver esaurito tutte le ottimizzazioni algoritmiche puoi passare alle ottimizzazioni a basso livello.
Abbiamo scelto di non usare classi private...
ok...non lo sapevo :flower:
Stai complicando il codice in base ad un'assunzione di carattere prestazionale che non deriva dal risultato di un profiler?
Se due soluzioni hanno complessita' equivalente, scegli quella che instanzia meno oggetti. Se due soluzioni hanno complessita' differente, scegli quella piu' semplice, a meno che un profiler non ti dica che quel codice presenta un problema di carattere prestazionale.
Tutte le ottimizzazione vanno fatte con in mano i risultati di un profiler.
E poi, anche con i dati di un profiler in mano, la prima ottimizzazione che devi affrontare e' di carattere algoritmico, non un'ottimizzazione a basso livello come togliere l'allocazione di un oggetto.
Solo dopo aver esaurito tutte le ottimizzazioni algoritmiche puoi passare alle ottimizzazioni a basso livello.
Capito capo :D
P.S.: come si fa a fare un profiler??..lo faremo??
cdimauro
09-05-2006, 07:48
Bel lavoro tutti quanti, lo sto seguendo dalla build machine.
Ora mi servono le seguenti cose:
- Timer riportato al suo originale comportamento
- Il test sulle texture caricate disaccoppiato dagl'altri test
Avverà automaticamente una volta spostata la creazione delle texture dalla classe Texture a Engine.createTexture, similmente a quanto avviene con il caching dei suoni.
Mi ci metto stamattina. Il lavoro è abbastanza lungo, per cui se qualcuno deve lavorare e committare me lo faccia sapere, perché non vorrei effettuare il revert dopo ore di lavoro. E' bastato che già ieri abbia costretto Riccardo a buttare via il suo...
Per il futuro, se ci sono grosse rifattorizzazioni da effettuare, forse è meglio che ognuno lo segnali agli altri nel forum prima di procedere.
Io mi metterei al lavoro, se non ci sono obiezioni.
cdimauro
09-05-2006, 07:54
Capito capo :D
P.S.: come si fa a fare un profiler??..lo faremo??
Penso che sarà difficile: abbiamo già problemi a far girare il gioco su piattaforme diverse, figuriamoci dover considerare la vastità di configurazioni hardware e di JVM esistenti...
Comunque il nostro scopo è anche quello di produrre un gioco secondo metodologie utilizzate in software house di spessore, quindi in teoria potremmo anche affrontare queste problematiche prima del rilascio... :D
Ho riportato il timer alla sua forma iniziale... Comunque ritornano i problema di sincornismo che avevo evidenziato... Che si potranno risolvere quando playFiled di fatto non dipendenrà più dal Timer, ma solo dai TimeStamp passati ai vari metodi...
Inoltre ho ho tolto l' "accelerazione" che subiva il gioco all'inizio e dovuta al ritardo di 500 ms introdotto prima dell'inizio del gioco che faceva avanzare il timer, ma non il timeStamp all'interno dei playField e di conseguenza il gioco "correva" per recuperare il tempo perduto...
I test sono i seguenti_
public void testPlayFieldTimeStampsResetInInitBeforeGameLoop()
{
gameLoop.initBeforeGameLoop();
gameLoop.getPlayFieldOne().reactToInput();
gameLoop.getPlayFieldOne().reactToInput();
assertEquals(0, gameLoop.getPlayFieldOne().wasInputServed());
gameLoop.getPlayFieldTwo().reactToInput();
gameLoop.getPlayFieldTwo().reactToInput();
assertEquals(0, gameLoop.getPlayFieldTwo().wasInputServed());
}
public void testSleepHalfASecondInInitBeforeGameLoop()
{
long timeStamp = environment.getTimer().getTime();
gameLoop.initBeforeGameLoop();
assertEquals(500, environment.getTimer().getTime() - timeStamp);
}
Direi che ora ci vorrebbe un rendering del gioco da fare prima del ciclo di input e di update...
Provo a lavorarci...
E' bastato che già ieri abbia costretto Riccardo a buttare via il suo...
Non ti rpeoccupare...tanto non funzionava tutto... L'ho fatto incrementale così sono riuscito ad isolare il problema...
Direi che ora ci vorrebbe un rendering del gioco da fare prima del ciclo di input e di update...
Riguardo a questo...come si fa a vedere che il motore di rendering abbia diesgnato qualcosa ?
Comunque il nostro scopo è anche quello di produrre un gioco secondo metodologie utilizzate in software house di spessore, quindi in teoria potremmo anche affrontare queste problematiche prima del rilascio... :D
Si', infatti affronteremo questo problema alla fine dello sviluppo con una fase di ottimizzazione, che nel nostro caso durera' molto poco. Daremo soprattutto un'occhiata al nostro uso del garbage collector e agli hot spot piu' frequenti, giusto per avere un assaggio di come si affronta questa fase.
Riguardo a questo...come si fa a vedere che il motore di rendering abbia diesgnato qualcosa ?
Intendi dire che l'engine abbia spedito gli sprite da disegnare o che la scheda grafica li abbia effettivamente disegnati?
cdimauro
09-05-2006, 09:37
Non ti rpeoccupare...tanto non funzionava tutto... L'ho fatto incrementale così sono riuscito ad isolare il problema...
Ho visto che hai modificato 3 file. Ottimo, perché sono in mezzo a una rifattorizzazione mostruosa e mi sarei sparato se avessi dovuto ricominciare da capo (a proposito: chi ha tolto di mezzo il mio codice da CrushBox, rimettendo tutta l'inizializzazione in una sezione static? Ho dovuto risistemare nuovamente il codice!!! :( ).
cdimauro
09-05-2006, 09:38
Riguardo a questo...come si fa a vedere che il motore di rendering abbia diesgnato qualcosa ?
A livello di Engine non puoi vederlo: assumi che LWJGL abbia fatto il suo lavoro.
A livello di MockEngine, ci sono degli appositi metodi (e relativi test) per simularlo.
Dovevo fare il cast al MockEngine...comuqnue non mi torna ancora...
Questo codice:
public void initBeforeGameLoop()
{
inGameLoop = true;
environment.getAudio().playMusic();
loadTextures();
restart();
render();
environment.getTimer().advance(500);
startPlayLogging();
playFieldOne.resetTimeStamps(environment.getTimer().getTime());
playFieldTwo.resetTimeStamps(environment.getTimer().getTime());
}
Non fa passare questo test:
public void testFieldDrawnInInitBeforeGameLoop()
{
gameLoop.initBeforeGameLoop();
assertTrue(((MockEngine)environment.getEngine()).getNumberOfQuadsDrawn() > 0);
}
Perchè ?
cdimauro
09-05-2006, 12:15
Finito un'altra mega rifattorizzazione a EngineInterface, Engine e Texture: d'ora in poi per creare una Texture bisognerà usare Engine.createTexture.
Questa è l'ultima rifattorizzazione che mi si rendeva necessaria per spostare il pooling della cache delle texture da Texture a Engine, e risolvere quindi il problema del test ballerino.
x cionci: francamente non so. Provo a darci un'occhiata dopo pranzo.
cdimauro
09-05-2006, 13:16
Dovevo fare il cast al MockEngine...comuqnue non mi torna ancora...
Questo codice:
public void initBeforeGameLoop()
{
inGameLoop = true;
environment.getAudio().playMusic();
loadTextures();
restart();
render();
environment.getTimer().advance(500);
startPlayLogging();
playFieldOne.resetTimeStamps(environment.getTimer().getTime());
playFieldTwo.resetTimeStamps(environment.getTimer().getTime());
}
Non fa passare questo test:
public void testFieldDrawnInInitBeforeGameLoop()
{
gameLoop.initBeforeGameLoop();
assertTrue(((MockEngine)environment.getEngine()).getNumberOfQuadsDrawn() > 0);
}
Perchè ?
Ho cercato questo test, ma non l'ho trovato, quindi suppongo che lo abbia creato tu in locale per fare delle prove.
Potresti postarmi anche il codice del setUp() ?
Mettilo in TestGameLoop... Io l'avevo messo lì...
cdimauro
09-05-2006, 13:27
Ti fallisce in entrambi i casi (sia mettendo render() che non) perché non è stato disegnato niente fino a quel momento...
cdimauro
09-05-2006, 13:41
Il motivo è da ricercare in render:
private void render()
{
long timeStamp = environment.getTimer().getTime();
if(timeStamp - lastRender >= frameRate)
{
lastRender = timeStamp;
environment.getEngine().clearDisplay();
layerManager.drawLayers(environment.getEngine());
environment.getEngine().updateDisplay();
}
}
timeStamp è pari a 0, perché "il tempo non è ancora trascorso", ossia Timer non è stato fatto avanzare. Di conseguenza l'if fallisce e il display non viene aggiornato.
Ok, allora basterebbe fare avanzare il timer...
Raga c'è qualche problema dopo l'ultimo refactoring... Fare partire da Eclipse tutti i test è praticamente impossibile... I test sono lentissimi (3-4 minuti per l'intera suite) e ci vuole oltre 1 GB di ram per farli girare tutti...
cdimauro
11-05-2006, 08:50
Ho rifattorizzatore EngineInterface & derivati, e introdotto le interfacce Displayable, AudioInterface e Playable.
Al posto di Texture adesso si usa Displayable, che viene ritornato da EngineInterface.createDisplayable.
Engine è diventato LWJGLEngine, Texture è diventato LWJGLTexture, Display è diventato DisplayInterface, DisplayImplementation è diventato LWJGLDisplay, e infine MockEngine è diventato Engine che ritorna il suo Texture.
Audio non c'è più, sostituito da AudioInterface
Il vecchio Audio è diventato OpenALAudio, Sound è diventato OpenALSound, mentre nei mock è stato introdotto Audio che ritorna il suo Sound. AudioInterface.createPlayable permette di creare un suono.
L'uso estensivo delle interfacce ha permesso di separare nettamente il "contratto" dalla sua implementazione, e come conseguenza ciò ha portato a una netta separazione fra codice di produzione e codice test. Inoltre questo renderà più facile il porting di Diamonds verso altre piattaforme (telefonini, SDL, ecc.).
Ottimo lavoro! Ora, mi cambi solo il nome di Displayable in Image?
Cionci, puoi aggiustare Timer e riportarlo al comportamento precedente oppure fare il revert prima della fine del Ciclo per favore?
E infine il Refactoring di Droppable che e' rimasto in sospeso da tre settimane.
Ragazzi, questi refactoring sono importanti, non possiamo andare avanti con alcuna nuova Storia se non li concludiamo e i prossimi cicli verranno sempre dedicati solo ai refactoring fino a che non li concludiamo. Quindi prima ce li leviamo di torno prima andiamo avanti col gioco.
cdimauro
11-05-2006, 09:29
Ottimo lavoro! Ora, mi cambi solo il nome di Displayable in Image?
Certamente (anche se avrei preferito Blittable :p).
Playable va bene, invece?
Certamente (anche se avrei preferito Blittable :p).
Playable va bene, invece?
Gia' solo per il fatto che il nome non mi chiarisce molto su che cosa sia, direi di no :p
Mancano due giorni alla fine del Ciclo e vorrei assegnare i seguenti refactoring:
1) State/Action di GridController
2) Separazione di Droppable in piu' interfacce
Se non chiudiamo questi due refactoring entro la fine del Ciclo diventeranno automaticamente una Storia del prossimo Ciclo e prenderanno il posto della normale Storia di sviluppo.
cdimauro
11-05-2006, 10:49
Gia' solo per il fatto che il nome non mi chiarisce molto su che cosa sia, direi di no :p
Era in ricordo dei vecchi tempi:
http://amiga-hardware.com/display_photos/agnus8370plcc.jpg
:p
Comunque ho cambiato Displayable & derivati in Image & co., e Playable e derivati in Sound & co., come richiesto. ;)
Mancano due giorni alla fine del Ciclo e vorrei assegnare i seguenti refactoring:
1) State/Action di GridController
2) Separazione di Droppable in piu' interfacce
Se non chiudiamo questi due refactoring entro la fine del Ciclo diventeranno automaticamente una Storia del prossimo Ciclo e prenderanno il posto della normale Storia di sviluppo.
Per il secondo ce ne occupiamo io e te Fra?
Cionci, puoi aggiustare Timer e riportarlo al comportamento precedente oppure fare il revert prima della fine del Ciclo per favore?
Già fatto da tempo...ovviamente fino a quando non si riesce a fare il refactoring di playField rendendolo indipendente dal Timer, ma dipendente solo dal timeStamp passato come paramentro i problemi che avevano portato a quelle modifiche di Timer ritornano...
Per il secondo ce ne occupiamo io e te Fra?
Se volete ci posso essere anche io ;)
cdimauro
12-05-2006, 08:14
Rifattorizzata KeyboardInterface & derivati (Keyboard è diventata LWJGLKeyboard adesso), introdotta KeyMappingsInterface e le derivate LWJGLKeymapping (ex KeyMappings) e MockKeyMappings.
Ci sono piani per dividere LogFile in due classi distinte? In questo momento la classe legge e scrive i file. Personalmente sono inorridito guardando la funzione close() e alle funzioni per createForTesting*().
ciao ;)
cdimauro
12-05-2006, 08:27
Ringrazia che ne ho già eliminate tante di "forTesting". :p
Ringrazia che ne ho già eliminate tante di "forTesting". :p
Io qui ne conto 4. Ce ne erano altre ? :eek:
ciao ;)
cdimauro
12-05-2006, 08:43
Qui ancora non c'ho messo mano. :D
Comunque ne tolte di mezzo parecchie rifattorizzando Engine, Audio, ecc.
A proposito: ho cambiato KeyMappingsInterface in KeyMappings e KeyEvent in Event.
Comunque c'è qualcosa che non mi quadra nell'attuale definizione dei vari KeyBoardInterface, Input, Event, ecc.: il design mi "puzza". Sto riflettendo su come rifattorizzare il tutto in maniera più semplice e comprensibile.
Già fatto da tempo...ovviamente fino a quando non si riesce a fare il refactoring di playField rendendolo indipendente dal Timer, ma dipendente solo dal timeStamp passato come paramentro i problemi che avevano portato a quelle modifiche di Timer ritornano...
Ok, puoi riportare il Timer a quello di prima e risolveremo il problema col TimeStamp quando hai tempo nel prossimo ciclo?
Se volete ci posso essere anche io ;)
Tu hai da occuparti degli stati :)
cdimauro
12-05-2006, 09:38
Ragazzi, io vorrei togliere di mezzo KeyMappings e modificare KeyboardInterface (e derivati) e KeyboardListener, perché ai miei c'è una complicazione inutile nel design (e codice) attuale.
La mia idea è questa: la tastiera è un dispositivo che notifica a degli ascoltatori degli eventi già "cucinati", quindi senza giocare a ping pong col listener, che poi deve far ricorso al KeyMappings per traslare il codice appena ricevuto: è una responsabilità che, IMHO, non gli compete.
In soldoni: all'update della tastiera, questa esegue la conversione dal suo codice nativo all'apposito evento (Event), che provvede poi a notificare ai lister. Un lister deve semplicemente prendere l'evento e controllare cos'è, facendo uso dei metodi "pressed" e "released" (anziché state()).
Il codice dovrebbe diventare MOLTO più semplice e comprensibile.
Che ne dite?
Eventualmente siate d'accordo vedrò quando poterci metter mano, perché c'è parecchio codice da modificare e in questo momento ho delle cose da sbrigare.
Ok, puoi riportare il Timer a quello di prima e risolveremo il problema col TimeStamp quando hai tempo nel prossimo ciclo?
Come dicevo prima...già fatto da diverso tempo :)
Che ne dite?
Ora non mi ricordo bene la struttura attuale però sarei per eliminare Input ed aggiungere responsabilità a InputReactor...
Keyboard dovrebbe essere un observer... I vari InputReactor si dovrebbero registrare... All'update della tastiera tutti i tasti vengono inviati agli inputReactor registrati già sotto forma di KeyEvent... Gli inputReactor si occupano di tradurre (con un Map) i KeyEvent in KeyEventHandler...e l'eventuale KeyEventHandler va messo nella coda degli eventi da eseguire in InputReactor.reactToInput...
Ritiro tutto...ho detto una castroneria...
IMHO la traduzione da tasto ad evento deve essere fatta dentro InputReactor con il mapping di cui parlavo prima...
Mi spiego meglio...quello che Keyboard nella mia visione dovrebbe fare è:
- se lo stato di una tasto è cambiato trasformare le costanti LWJGL in delle costanti interne al nostro gioco che identificano i tasti (questo per avere un sistema di identificazione dei tasti indipendente dal sistema da LWGJL) e non gli eventi...
org.lwjgl.input.Keyboard.KEY_A -> Key.A
org.lwjgl.input.Keyboard.KEY_W -> Key.W
In teoria il valore intero potrebbe anche essere lo stesso...si tratta di una trasformazione formale che ci potrebbe semplificare successivi porting, ad esempio su cellulari...e di fatto ci renderebbe indipendenti dalla piattaforma...
- a questo punto la traduzione da tasto a KeyEventHandler viene fatta direttamente all'interno di ogni InputReactor registrato e quindi all'interno di InputReactor ci sarà una corrispondenza Key -> KeyEventHandler...e di fatto dipenderà solo dagli eventi registrati:
InputReactor.addKeyHandler(Key key, KeyEventHandler handler);
Così ci risparmiamo i mapping con i vari metodi statici createForPlayerOne, createForPlayerTwo e createForGameLoop...e così via...
Cosi a naso mi piace più la soluzione di cesare, meno codice c'è meglio è, anche perché il porting su cellulari del gioco non è una priorità e possiamo sempre modificare il codice in un secondo momento.
ciao ;)
Cosi a naso mi piace più la soluzione di cesare, meno codice c'è meglio è, anche perché il porting su cellulari del gioco non è una priorità e possiamo sempre modificare il codice in un secondo momento.
ciao ;)
Si', sono d'accordo. Meno codice meglio e'. Il cellulare NON e' una priorita'. Ma questo refactoring ha priorita' piu' bassa del refactoring degli stati e di Droppable. Ci dobbiamo concentrare su questi due ora e lasciar perdere tutto il resto.
cdimauro
12-05-2006, 12:20
La mia visione è simile, ma più generale: la tastiera è un dispositivo observer a cui sono attaccati dei listener, che ricevono gli eventi da esso erogati (tramite notifyEvent).
Che poi sia Input o InputReactor a gestirli, è indifferente, e personalmente toglierei di mezzo ogni riferimento a Key dagli Handler, perché gli handler dovrebbero gestire qualunque tipo di evento proveniente da qualunque dispositivo.
A seguito di questa rifattorizzazione che ho in mente, ad esempio, Engine è un dispositivo che diventerà un observer e potrà eventualmente erogare eventi di tipo QUIT, a cui si aggancerà soltanto GameLoop (è l'unico a cui interessebbe gestirli).
Oppure in futuro potremmo benissimo agganciare GameLoop e Input/Reactor in maniera semplice a un dispositivo/observer di tipo Joystick, che erogherà gli oppurtuni eventi.
In generale, basterà agganciarsi all'opportuno observer per ricevere gli appositi eventi.
Questa soluzione è semplice e interessante anche per quanto concerne il porting verso i cellulari: in maniera trasparente sarà sufficiente, dentro Environment, istanziare un'ipotetica CellKeyboard anziché LWJGLKeyboard, e Diamonds funzionerà senza battere ciglio (spero :D).
cdimauro
12-05-2006, 12:23
Del porting su cellulare non interessa neppure me in questo momento, perché si otterrebbe appunto in maniera molto semplice come "effetto collaterale".
Quello che mi turba per adesso è il connubbio incestuoso che c'è fra Keyboard e KeyMappings :D, che complica abbastanza il codice e la sua lettura.
Keep it simple. Ecco perché, appena avrò un po' di tempo, vorrei mettere mano a questa rifattorizzazione.
Fermo restando che customer e coach deciderenno come sia meglio impiegare il mio tempo libero per Diamonds. ;)
Ok, ma possiamo accantonare questa discussione e riprenderla quando abbiamo concluso gli altri refactoring?
Dobbiamo stabilire delle chiare priorita', perche' tendiamo a disperdere gli sforzi in questo periodo.
Se le priorita' non sono chiare, e' colpa mia. Le chiarisco ora. Dobbiamo pensare solo ai seguenti due refactoring:
1) Droppable
2) Grid/State
Tutti gli altri refactoring sono da accontonare e le discussioni da sospendere.
Ma infatti con il mio si toglierebbe Input...dovrebbe essere meno codice... Il mapping fra tasti LWJGL e del nostro gioco basta farlo pari pari... E se si vuole si può mettere le costanti per identificarli, ma tanto non è obbligatorio...
Quindi:
class Key
{
private static final Key ESCAPE = new Key(org.lwjgl.input.Keyboard.KEY_ESCAPE);
private static final Key A = new Key(org.lwjgl.input.Keyboard.KEY_A);
private int key;
public Key(int key)
{
this.key = key;
}
public boolean equals(Key otherKey)
{
return this.key == otherKey.key;
}
}
Lo facevo per incapsulare di LWJGL e per non dipendere da quelli per le costanti...
In questo modo se si aggiunge un evento ad un InputReactor bisogna aggiungere solamente l'handler per il tasto e non bisogna aggiungere Factory Method a KeyMappings...
Inoltre quelle costanti ci serviranno solamente al momento dello sviluppo, perchè quando avremo la possibilità di configurare i tasti dal gioco si potranno semplicemente caricare gli interi dal file di configurazione....
Ragazzi, io vorrei togliere di mezzo KeyMappings e modificare KeyboardInterface (e derivati) e KeyboardListener, perché ai miei c'è una complicazione inutile nel design (e codice) attuale.
La mia idea è questa: la tastiera è un dispositivo che notifica a degli ascoltatori degli eventi già "cucinati", quindi senza giocare a ping pong col listener, che poi deve far ricorso al KeyMappings per traslare il codice appena ricevuto: è una responsabilità che, IMHO, non gli compete.
In soldoni: all'update della tastiera, questa esegue la conversione dal suo codice nativo all'apposito evento (Event), che provvede poi a notificare ai lister. Un lister deve semplicemente prendere l'evento e controllare cos'è, facendo uso dei metodi "pressed" e "released" (anziché state()).
Il codice dovrebbe diventare MOLTO più semplice e comprensibile.
Che ne dite?
Eventualmente siate d'accordo vedrò quando poterci metter mano, perché c'è parecchio codice da modificare e in questo momento ho delle cose da sbrigare.
Non sono d'accordo...e mi spiego. GIà l'aver cambiato KeyEvent in Event non mi è piaciuto ;)
Il KeyEvent è na cosa, l'Event è un altro.
Esempio pratico.
Pressione del tasto X.
Ad X è associato il KeyEvent.TASTO_FUNZIONE_A
La generazione del KeyEvent.TASTO_FUNZIONE_A genera l'evento "vsualizzazione funzione" se simao nel menù, altrimenti nel gioco genera l'evento "cambio campo"
A questi eventi applicativi si registrano tutti i listener che vogliamo.
Con questo esempio sto facendo vedre come nella mia idea c'è una forte separazione tra il tasto premuto, tasto dell'applicazione e evento dell'applicazione. Anche èerchè non è detto che gli eventi applicativi li generino solo i tasti ;)
mi sono spiegato o ho fatto solo casino :fagiano:
Ok, ma possiamo accantonare questa discussione e riprenderla quando abbiamo concluso gli altri refactoring?
Dobbiamo stabilire delle chiare priorita', perche' tendiamo a disperdere gli sforzi in questo periodo.
Se le priorita' non sono chiare, e' colpa mia. Le chiarisco ora. Dobbiamo pensare solo ai seguenti due refactoring:
1) Droppable
2) Grid/State
Tutti gli altri refactoring sono da accontonare e le discussioni da sospendere.
PERDONO :ave: :ave: :ave:
..non avevo ancora letto :(
A che punto siamo con questi refactoring? Domani e' l'ultimo giorno.
Io non ho ancora concluso niente :(
...domani sono pure tutto il giorno ad un battesimo.
In questi giorni però mi sono studiato un po' il codice.
La refatorizzazione di Action e State dipende molto dal raggiungimento di BigGem= Droppable.
In ogni caso alcune azioni usano metodi di grid che invece dovrebbero essere delle Action. Ecco il problema filosofico a cui mi sono trovato a rispondere:
Gli oggetti nella griglia devono muoversi...o li muove grid perchè sa come sono fatti, oppure gli oggetti si muovono conoscendo lo stato della griglia senza fornire alcuna informazione su come sono fatti.
Io propendo per la seconda per non rompere l'incapsulamento.
il ragionamento vale anche per tutti i metodi di "stato", come ad esempio il canMoveDown().
E' la strada giusta??
EDIT: comunque più ci penso e più mi rendo conto che ha ragione cisc...BigGem non deve essere realizzata come agglomerato di gemme, ma come oggetto unico.
Deve diventare un droppable....al punto che questo codice deve sparire:
Cell cell = gem.getCell();
if(grid.isCellInABigGem(cell.getRow(), cell.getColumn()))
{
BigGem bigGem = grid.getBigGemAt(cell.getRow(), cell.getColumn());
if(!updatedBigGems.contains(bigGem))
{
//OPERAZIONI
}
return;
}
Insomma....senza BigGem droppable si fa poco.
Propongo una storia solo per questo.
Io sono assolutamente d'accordo nel far diventare BigGem un droppable. Ne parliamo con Jocchan e vediamo se si riesce a scrivere una storia di refactoring per il prossimo ciclo.
Fra, io ci sono per il refactoring di Droppable e BigGem :)
La storia di refactoring ci vuole, possibilmente con task ben precisi, in modo da evitare confusioni.
Ecco un'altra idea di filosofia....ovvero come fare un design sensato.
Ecco la mia idea: Grid non deve più avere un elenco di celle occupate o meno, ma un elenco di Droppable.
In questo modo si itera sugli oggetti e non sulle posizioni (la gestione di una BigGem si semplifica molto).
Inoltre la posizione diventa una proprietà conosciuta dal Droppable, a cui Grid chiede tutte le informazioni e le operazioni da eseguire nella griglia (canMoveDown e moveDown).
E quando a Grid viene chiesto chi c'è in una determinata posizione, questa itera sui droppable chiedendo se ci sono loro.
Questa idea significherebbe semplicemente gestire quel campo Cell nei droppable internamente invece che esternamente. ;)
Che ne dite??
EDIT: secondo me si semplificheranno tremendamente sia i droppable( avranno più logica, ma saranno molto più uncoupled con grid) e grid...facendo un conto ad occhio, il codice di grid diminuirà di almeno il 50% :D :D
vBulletin® v3.6.4, Copyright ©2000-2025, Jelsoft Enterprises Ltd.