View Full Version : Refactoring di Actions
http://fcarucci.homeip.net:8080/cruisecontrol/artifacts/diamonds-nightly/20080204010005/cobertura/frame-summary-it.diamonds.grid.action.html
Abbiamo due problemi:
- CrushByFlashAction ha una complessita' di 3.2 contro una complessita' media di 1.5. Tutto cio' che sta sopra 2.0 merita la nostra attenzione. Mi serve un volontario che si occupi di semplificare questa action per poi eliminarla.
- RemoveDroppableAction: c'e' un metodo non testato. Se quel metodo non e' testato ed e' vuoto, perche' sta li'? Va eliminato o portato in una classe base.
Lo scopo ultimo sara' la rimozione delle Action, perche' hanno un'interazione davvero oscura col resto del codice. Ma prima di tutto vanno semplificate, per facilitarne la rimozione.
In futuro faremo la stessa cosa con un bel Command Pattern (http://en.wikipedia.org/wiki/Command_pattern), magari semplificato.
Mi servono due volontari.
Col command dopo ci viene il log gratis. ;)
Volontario per la CrushByFlashAction presente.
O almeno, provo a lavorarci su e vi dico :D
EDIT: ma sbaglio o questa classe non è per nulla testata? E se è così, perchè il Coverage Report indica una copertura del 95%? Che devo fare? I test ci sono ma da un'altra parte? Li scrivo? Lascio perdere tanto poi verrà eliminata? Ditemi un po' voi :)
Col command dopo ci viene il log gratis. ;)
dejavu
ma le action non erano state fatte proprio in ottica di implementare un command pattern?
cos'hanno attualmente di diverso?(apparte il fatto che non sono gli unici modi in cui la griglia viene modificata..)
dejavu
ma le action non erano state fatte proprio in ottica di implementare un command pattern?
cos'hanno attualmente di diverso?(apparte il fatto che non sono gli unici modi in cui la griglia viene modificata..)
Si', quella era l'idea, ma l'esecuzione non fu delle piu' felici :D
Baol, applica l'antica tecnica di refactoring delle sette stelle di hokuto dell'assimilazione anale. Eliminala quell'action.
Porto tutto in Grid? Non dovrebbe essere complicato
EDIT: ma se ci sta lavorando già 71104 a sto punto io mi fermo...
No, per favore, il codice deve essere portato VIA da Grid allo stato attuale delle cose.
Porto tutto in Grid? Non dovrebbe essere complicato
EDIT: ma se ci sta lavorando già 71104 a sto punto io mi fermo...
Continua da dove ha lasciato 71104 oggi e porta via il codice da Grid.
Continua da dove ha lasciato 71104 oggi e porta via il codice da Grid.
E' che stavo facendo esattamente quello che ha fatto 71104, comunque ok, reverto tutto e vedo quello che posso fare (non sono così veloce come altri, purtroppo :) ).
jappilas
05-02-2008, 13:10
<..> l'antica tecnica di refactoring delle sette stelle di hokuto dell' assimilazione anale. :mbe:
Io e Fede ci stiamo facendo prendere la mano da Ken Shiro :D
E' che stavo facendo esattamente quello che ha fatto 71104, comunque ok, reverto tutto e vedo quello che posso fare (non sono così veloce come altri, purtroppo :) ).
Va benissimo cosi', non devi essere veloce, devi fare le cose bene ;)
Cosi' tutti sono piu' veloci.
Inoltre il codice e' di tutti, come avrai notato non c'e' qualcuno che ha la responsabilita' di un qualche pezzo di codice o sistema, ma tutti interveniamo ovunque. Quindi e' ottimo per te ripartire dal lavoro fatto da Alberto e proseguire in quella direzione.
Mi raccomando: fai le cose semplici. Non devi fare il commit per forza. Se quello che hai scritto non e' semplice, chiedi consigli qui su come semplificarlo, oppure fai il revert e ricomincia. La seconda volta di solito funziona tutto meglio e si fa molto piu' in fretta.
Comunque penso che Ken Shiro abbia segnato profondamente la mia esistenza :|
Ho pure la OST originale
Stanno per suonare le campane per la CrushByFlashAction.
E' tutto a posto, ho solo una domanda: possono esserci più FlashingGem contemporaneamente? E quindi la crush deve tenere conto che possano crushare più d'una? Perchè modificando il codice l'ho involontariamente portato a considerare una sola flashing (modifico in un attimo nel caso, basta aggiungere un ciclo) ma nessun test è fallito per questo...
Questo vuol dire che devo aggiungerlo vero? :D
Ho modificato per permettere più flashingGems, i test che coprono ci sono già.
Al momento ho lasciato in Grid, semplicemente perchè l'unica cosa che ho aggiunto è un'iterazione sulla griglia, e per portarla fuori ci serve l'Iterator no?
(avevo anche pensato a un'eventuale classe accessoria "GridGemFinder" in cui spostare tutti i metodi che cerchino delle gemme nella lista, però non mi convinceva).
Ora committo, qui di seguito il codice:
public void updateCrushes()
{
crushByFlash();
crushByChestAction.reset();
doIteration(crushByChestAction);
crushedGemsCounter += crushByChestAction.getCrushedGemsCounter();
if(crushByChestAction.isCrushed())
{
chainCounter++;
}
}
private void crushByFlash()
{
DroppableList flashingGems = findFlashingGems();
for(Droppable flashingGem : flashingGems)
{
flashingGem.getCrusherObject().makeCrush(this);
}
}
private DroppableList findFlashingGems()
{
DroppableList listOfFlashingGems = new DroppableList();
for(Droppable droppable : droppableList)
{
if(droppable.getGridObject().getType().isFlashingGem()
&& !droppable.getFallingObject().isFalling())
{
listOfFlashingGems.add(droppable);
}
}
return listOfFlashingGems;
}
e CrushByFlashAction è eliminata :sofico:
Stanno per suonare le campane per la CrushByFlashAction.
E' tutto a posto, ho solo una domanda: possono esserci più FlashingGem contemporaneamente? E quindi la crush deve tenere conto che possano crushare più d'una? Perchè modificando il codice l'ho involontariamente portato a considerare una sola flashing (modifico in un attimo nel caso, basta aggiungere un ciclo) ma nessun test è fallito per questo...
Questo vuol dire che devo aggiungerlo vero? :D
Si', devi aggiungerlo tu. Domanda a Jocchan se devi considerare o no questo caso prima.
flashingGem.getCrusherObject().makeCrush(this);
Mi togli quel getCrusherObject() per favore?
if(droppable.getGridObject().getType().isFlashingGem()
&& !droppable.getFallingObject().isFalling())
{
listOfFlashingGems.add(droppable);
}
E poi cerca di eliminare la dipendenza da isFlashingGem() e, possibilmente, anche getFallingObject().
Ottimo lavoro fino ad ora :)
E' molto difficile che ci sia più di una flashing gem, ma non è impossibile. Questo perchè, pur avendo la limitazione (voluta) di non avere mai due flashing nella stessa coppia, è possibile che si verifichi questa situazione nella griglia:
R - gemma rossa
B - gemma blu
F - flashing
| |
| |
| |
|RRRR RRR|
|RRRR RRR|
|BB BB|
|BB F BB|
------------
...ovvero si lascia cadere una flashing in un punto in cui non è a contatto con nulla (per comodità nell'esempio ho usato solo big gem).
Con il codice attuale cosa succederebbe in questa situazione? E se un'altra flashing venisse poggiata su quella già presente?
Quello che deve succedere è che la flashing rimanga lì, sola soletta in fondo alla griglia, fino a quando non verrà in contatto con qualcosa. A quel punto, sempre seguendo l'ordine di priorità che ha adesso, dovrà azionarsi.
Non ricordo però se era stato previsto il caso di due flashing a contatto tra loro.
Per logica direi che se due flashing vengono a contatto allora tutte le flashing devono essere eliminate... Dopotutto e` un colore NO_COLOR no?
Quello che deve succedere è che la flashing rimanga lì, sola soletta in fondo alla griglia, fino a quando non verrà in contatto con qualcosa. A quel punto, sempre seguendo l'ordine di priorità che ha adesso, dovrà azionarsi.
Non ricordo però se era stato previsto il caso di due flashing a contatto tra loro.
In SPF2T la flashing gem che tocca il fondo esplode.
jappilas
05-02-2008, 16:20
In SPF2T la flashing gem che tocca il fondo esplode.anch' io sarei stato per far svanire la flashing gem al termine della caduta, che ci fossero gemme da crushare o meno :O
ma discutendo con Jocchan in pvt proprio di come fosse questo (e altri) dettagli nel gioco originale, si è arrivati alla conclusione che sottili differenze presenti a livello di gameplay (*) si potrebbero anche mantenere...
(*) per dire, notavo (su wikipedia) che in PF le Counter gems ( le nostre Stones) possono essere eliminate se vicine a una Crash Gem ( il nostro baule) che a sua volta elimina almeno una gemma normale - in Diamonds pare (dettaglio a cui ho fatto caso adesso) le Stones vengano eliminate se vicine direttamente a una gemma coinvolta in una Crush
Va benissimo allora. Dobbiamo supportare piu' flashing gem.
Per logica direi che se due flashing vengono a contatto allora tutte le flashing devono essere eliminate... Dopotutto e` un colore NO_COLOR no?
Sì, l'idea è questa. Ho chiesto quale fosse il comportamento attuale perchè non ho trovato una descrizione di questo caso particolare da nessuna parte, e non ricordo se c'erano state discussioni in merito.
Su suggerimento di Fek eliminata l'interfaccia CrusherObject, aggiunto un TestCase "TestDroppableMakeCrush" per coprire le modifiche.
Le altre modifiche le faccio domani, fino a sera sarò impegnato :(
Su suggerimento di Fek eliminata l'interfaccia CrusherObject, aggiunto un TestCase "TestDroppableMakeCrush" per coprire le modifiche.
Le altre modifiche le faccio domani, fino a sera sarò impegnato :(
Ma sto creando un'orda assassina di extreme programmer :D
"Come, my minions... come here"
Perdonate il ripensamento, ma sono rimasto fino ad ora a valutare la situazione.
Oltre ad una miriade di casi critici in cui possiamo avere potenziali inconsistenze, avere fino a n flashing su schermo rischia di facilitare un pò troppo la vita al giocatore (abbiamo 5 colori diversi, farne fuori due in un colpo significa un 40% statisticamente variabile di gemme su schermo in meno).
Quindi eliminiamo il problema alla radice: se una flashing droppa e non tocca nulla si cancella da sola.
Quindi eliminiamo il problema alla radice: se una flashing droppa e non tocca nulla si cancella da sola.
Per chiarire ulteriormente: se tocca il fondo esplode. Chi se ne occupa?
Per chiarire ulteriormente: se tocca il fondo esplode. Chi se ne occupa?
Siccome e' una cosa "nuova", non possimao aspettare i i cicli e le storie per introdurre questa cosa??
A proposito, ma l'idea di cilci e storie e' SCRUMM based?? :D
Non capisco.. Se tocca il fondo ma a fianco ha una gemma deve esplodere comunque?
Devo rigiocare SPF2.
Comunque non possiamo tenercelo per quando entriamo in produzione? Per come la vedo io dovrebbe essere un task vero e proprio..
Non capisco.. Se tocca il fondo ma a fianco ha una gemma deve esplodere comunque?
Devo rigiocare SPF2.
Comunque non possiamo tenercelo per quando entriamo in produzione? Per come la vedo io dovrebbe essere un task vero e proprio..
Ci leggiamo nel pensiero :D :D
Il fatto e' questo
| rf |
| |
|dd |
|dd |
|e |
| |
| r |
|dd |
|dd |
|e f |
si ma fra ha detto "se tocca il fondo"
Siccome e' una cosa "nuova", non possimao aspettare i i cicli e le storie per introdurre questa cosa??
Decisione di Jocchan.
A proposito, ma l'idea di cilci e storie e' SCRUMM based?? :D
Si' :)
si ma fra ha detto "se tocca il fondo"
Mmmhhh...
forse non ho capito cosa hai chiesto?
Se arriva in fondo e nessuna delle 4 condizioni e' verficata.... ovvero non ha di finaco un bel niente di niente, esplode.
jappilas
05-02-2008, 20:19
http://www.hwupgrade.it/forum/showpost.php?p=20939974&postcount=116
AnonimoVeneziano
05-02-2008, 20:28
http://www.hwupgrade.it/forum/showpost.php?p=20939974&postcount=116
:doh:
Come hai fatto?
Poi ci sarebbe anche l'altro bug che facendo una 2x3 mettendone due di fianco uguali in un colpo solo crusha
Sono proprio toste queste BugGem
jappilas
05-02-2008, 20:48
è successo in seguito alla caduta di una flashing gem, che ha crushato tutte le gemme blu, di cui alcune, su più righe, in corrispondenza della big Gem verde di destra
questa però se non ho visto male si è formata con la caduta di alcuni smeraldi , dev' essere per quello che il controllo di compenetrazione con l' altra bigGEm e con il chest non è stato fatto... :wtf:
Comunque non possiamo tenercelo per quando entriamo in produzione? Per come la vedo io dovrebbe essere un task vero e proprio..
In realtà sarebbe un elemento che non avevamo considerato in una feature già presente (e - ai nostri occhi - completata) nella first playable, il che lo rende a tutti gli effetti un bug.
Se arriva in fondo e nessuna delle 4 condizioni e' verficata.... ovvero non ha di finaco un bel niente di niente, esplode.
Esatto. Il punto non è farla esplodere se tocca il fondo, ma farla esplodere se tocca il fondo e non ha nulla intorno. Ovvero: una flashing gem, comunque vada, non deve restare nella griglia.
:doh:
Come hai fatto?
Poi ci sarebbe anche l'altro bug che facendo una 2x3 mettendone due di fianco uguali in un colpo solo crusha
Sono proprio toste queste BugGem
Succede ancora? Credevo fosse stato risolto (oggi ci ho provato e il bug non si presentava).
Per favore leggete:
ho creato una nuova interfaccia per gestire le iterazioni in grid.
l'interfaccia la trovate in it.diamonds.grid.iteration.DroppableIteration
L'interfaccia e` semplice e siete pregati di non cambiarla.
Piano piano tutte le iterazioni presenti sulla griglia (esempio: TurnStonesIntoGemsAction) andranno spostate verso la nuova interfaccia.
Il metodo pubblico doIteration in Grid deve sparire il prima possibile poiche ora esiste runIteration(DroppableIteration iteration).
Per favore cercate di testare il tutto il meglio possibile. Sto inoltre cercando di testare GridIteration senza usare grid nei test. Guardate i miei test attuali in TestRetrieveGemsByColor
Succede ancora? Credevo fosse stato risolto (oggi ci ho provato e il bug non si presentava).
Non succede più, il bug, per come è definito, è risolto. Se ce n'è un altro si verifica in altre condizioni, ma non credo parlasse di altri :D
ho creato una nuova interfaccia per gestire le iterazioni in grid.
l'interfaccia la trovate in it.diamonds.grid.iteration.DroppableIteration
L'interfaccia e` semplice e siete pregati di non cambiarla.
Piano piano tutte le iterazioni presenti sulla griglia (esempio: TurnStonesIntoGemsAction) andranno spostate verso la nuova interfaccia.
Il metodo pubblico doIteration in Grid deve sparire il prima possibile poiche ora esiste runIteration(DroppableIteration iteration).
Ottimo! :mano:
Ho creato una classe DroppableIterationTestCase per testare le varie iteration. Evitate di usare Grid nei test perche` si finisce sempre con test di 50 righe che chiamano 8 diversi update in grid e non si capisce piu` niente.
La MockDroppableIteration lasciatemela li` non c'e` motivo di metterla nel package mock generico se non per creare casino.
Qualcuno saprebbe mica spiegarmi questo codice in in UpdateFallsAction per favore?
protected void applyOn(Droppable droppable)
{
if(pair != null)
{
if(droppable == pair.getPivot() || droppable == pair.getSlave())
{
return;
}
}
droppable.getMovingDownObject().moveDown(getGrid());
}
Perche` controlla che non siano gemme della pair? Esistono casi in cui i fall sono eseguiti mentre la pair sta ancora cadendo?
1) togli tutto puoi guarda se i test passano.
- se non passano i test dovrebbero spiegarti il perche'.
- se passano, lanci il gioco
- se il gioco scoppia, vuol dire che i test sono buggosi.
(So che sono cose che tu sai gia', ma mi piace ripeterle :D :D )
2) Ma domani sei in ferie? :asd: :asd:
AnonimoVeneziano
06-02-2008, 01:40
Qualcuno saprebbe mica spiegarmi questo codice in in UpdateFallsAction per favore?
protected void applyOn(Droppable droppable)
{
if(pair != null)
{
if(droppable == pair.getPivot() || droppable == pair.getSlave())
{
return;
}
}
droppable.getMovingDownObject().moveDown(getGrid());
}
Perche` controlla che non siano gemme della pair? Esistono casi in cui i fall sono eseguiti mentre la pair sta ancora cadendo?
Credo che la UpdateFallsAction lavori sulle gemme che cadono per motivi diversi dalla normale discesa a coppia. Ad esempio se una gemma sotto un'altra gemma viene crushata allora la gemma sopra che cade verso il basso viene gestita da questa classe.
Molto ambiguo. Altro esempio di quanto le action necessitino di essere rifattorizzate/morire :D
Credo che la UpdateFallsAction lavori sulle gemme che cadono per motivi diversi dalla normale discesa a coppia. Ad esempio se una gemma sotto un'altra gemma viene crushata allora la gemma sopra che cade verso il basso viene gestita da questa classe.
Molto ambiguo. Altro esempio di quanto le action necessitino di essere rifattorizzate/morire :D
Si ma durante una crush la gemspair non dovrebbe esistere no?
Si ma durante una crush la gemspair non dovrebbe esistere no?
Esatto :)
E' possibile che si tratti di qualche "residuo" dell'advanced mode, dove introdurre la dinamite aveva creato una catena di conseguenze?
Esatto :)
E' possibile che si tratti di qualche "residuo" dell'advanced mode, dove introdurre la dinamite aveva creato una catena di conseguenze?
mi sa che hai fatto bingo :)
jappilas
06-02-2008, 11:39
E' possibile che si tratti di qualche "residuo" dell'advanced mode, dove introdurre la dinamite aveva creato una catena di conseguenze?non è possibile, è così al 99%... ricordo bene che all' epoca si era optato per consentire al giocatore di far esplodere la dinamite in arrivo con una nuova gempair, anche durante la caduta di questa
Bene allora stasera dinamito quel codice. :D
jappilas
06-02-2008, 12:08
vai con la tecnica segreta dei "mille pugni nella nebbia di londra" delle sette stelle di Hokuto su quel codice :O
Antares88
06-02-2008, 14:05
vai con la tecnica segreta dei "mille pugni nella nebbia di londra" delle sette stelle di Hokuto su quel codice :O
dato che ormai è diventato la nostra mascotte ufficiale, e dato che mi trovavo a cazzeggiare col photoshop:
http://img137.imageshack.us/img137/6697/kenpq9.jpg
ok torno a studiare java che è più utile :D
Mi fai una versione al 50%? :)
Antares88
06-02-2008, 14:56
Mi fai una versione al 50%? :)
dici a me fek ? :D
Ho rifattorizzato CrushByChestAction nel modo seguente, dovrebbe essere più semplice ora spezzarla e farla scomparire. In particolare io personalmente metterei FindAdjacentDroppables in grid e farei diventare il resto Iterations.
Non ho ancora committato perchè c'è ancora qualche "if" di troppo :D
La posto giusto per sapere cosa ne pensate.
Prima:
private boolean updateCrushesOnChest(Droppable gem)
{
DroppableList gemList = new DroppableList();
getAdjacentCrushableGems(gem, gemList);
if (gemList.size() == 0)
{
return false;
}
getGrid().crushDroppablesWithScore(gemList);
return true;
}
private void getAdjacentCrushableGems(Droppable crushSourceDroppable, DroppableList adjacentCrushableGems)
{
getLeftOrRightAdjacentCrushableGems(crushSourceDroppable, adjacentCrushableGems, GO_LEFT);
getLeftOrRightAdjacentCrushableGems(crushSourceDroppable, adjacentCrushableGems, GO_RIGHT);
getUpOrDownAdjacentCrushableGems(crushSourceDroppable, adjacentCrushableGems, GO_UP);
getUpOrDownAdjacentCrushableGems(crushSourceDroppable, adjacentCrushableGems, GO_DOWN);
}
private void getUpOrDownAdjacentCrushableGems(Droppable source, DroppableList adjacentCrushableGems, Direction direction)
{
int row = 0;
if (direction == GO_UP)
{
row = source.getRegion().getTopRow();
}
if (direction == GO_DOWN)
{
row = source.getRegion().getBottomRow();
}
row += direction.getRowDelta();
if (row < 0 || row > getGrid().getNumberOfRows() - 1)
{
return;
}
for (int column = source.getRegion().getLeftColumn(); column <= source.getRegion().getRightColumn(); column++)
{
Droppable toTest = getGrid().getDroppableAt(new Cell(row, column));
tryToAddGemToCrushableGems(toTest, source, adjacentCrushableGems);
}
}
private void getLeftOrRightAdjacentCrushableGems(Droppable source, DroppableList adiacentCrushableGems, Direction direction)
{
int column = 0;
if (direction == GO_LEFT)
{
column = source.getRegion().getLeftColumn();
}
if (direction == GO_RIGHT)
{
column = source.getRegion().getRightColumn();
}
column += direction.getColumnDelta();
if (column < 0 || column > getGrid().getNumberOfColumns() - 1)
{
return;
}
for (int row = source.getRegion().getTopRow(); row <= source.getRegion().getBottomRow(); row++)
{
Droppable toTest = getGrid().getDroppableAt(new Cell(row, column));
tryToAddGemToCrushableGems(toTest, source, adiacentCrushableGems);
}
}
private void tryToAddGemToCrushableGems(Droppable droppableToTest, Droppable adjacentDroppable, DroppableList adjacentCrushableDroppables)
{
if (droppableToTest == null)
{
return;
}
if (droppableToTest.getFallingObject() != null
&& droppableToTest.getFallingObject().isFalling())
{
return;
}
if (droppableToTest.getGridObject().getType().isStone())
{
if (!adjacentDroppable.getGridObject().getType().isChest())
{
adjacentCrushableDroppables.add(droppableToTest);
}
return;
}
if (droppableToTest.getGridObject().getColor() == adjacentDroppable.getGridObject().getColor()
&& !adjacentCrushableDroppables.contains(droppableToTest))
{
adjacentCrushableDroppables.add(droppableToTest);
getAdjacentCrushableGems(droppableToTest, adjacentCrushableDroppables);
}
}
Dopo:
private boolean updateCrushesOnChest(Droppable chest)
{
DroppableList gemList = new DroppableList();
DroppableList droppableTocCheck = new DroppableList();
droppableTocCheck.add(chest);
iterateFindingAdjacentCrushable(gemList, droppableTocCheck);
if (gemList.size() == 1)
{
return false;
}
getGrid().crushDroppablesWithScore(gemList);
return true;
}
private void iterateFindingAdjacentCrushable(DroppableList droppableChecked, DroppableList droppableToCheck)
{
DroppableList tempList = new DroppableList();
for (Droppable droppable : droppableToCheck)
{
findAdjacentCrushable(droppable, droppableChecked, tempList);
droppableChecked.add(droppable);
}
if (tempList.size() != 0)
{
iterateFindingAdjacentCrushable(droppableChecked, tempList);
}
}
private void findAdjacentCrushable(Droppable source, DroppableList droppableChecked, DroppableList tempList)
{
DroppableList adjacentList;
adjacentList = findAdjacentDroppables(source);
for (Droppable droppableToTest : adjacentList)
{
if (droppableToTest.getGridObject().getType().isStone())
{
if (!source.getGridObject().getType().isChest())
{
droppableChecked.add(droppableToTest);
}
else
{
continue;
}
}
if (isDroppableCrushable(droppableToTest, source)
&& !tempList.contains(droppableToTest)
&& !droppableChecked.contains(droppableToTest))
{
tempList.add(droppableToTest);
}
}
}
private boolean isDroppableCrushable(Droppable droppableToTest, Droppable source)
{
if (droppableToTest.getFallingObject() != null
&& droppableToTest.getFallingObject().isFalling())
{
return false;
}
if (droppableToTest.getGridObject().getColor() != source.getGridObject().getColor())
{
return false;
}
return true;
}
private DroppableList findAdjacentDroppables(Droppable droppable)
{
DroppableList adjacentDroppables = new DroppableList();
Region droppableRegion = droppable.getRegion();
for (int column = droppableRegion.getLeftColumn(); column <= droppableRegion.getRightColumn(); column++)
{
tryToAddAdjacentAtCell(adjacentDroppables, droppableRegion.getTopRow() - 1, column);
tryToAddAdjacentAtCell(adjacentDroppables, droppableRegion.getBottomRow() + 1, column);
}
for (int row = droppableRegion.getTopRow(); row <= droppableRegion.getBottomRow(); row++)
{
tryToAddAdjacentAtCell(adjacentDroppables, row, droppableRegion.getLeftColumn() - 1);
tryToAddAdjacentAtCell(adjacentDroppables, row, droppableRegion.getRightColumn() + 1);
}
return adjacentDroppables;
}
private void tryToAddAdjacentAtCell(DroppableList adjacentDroppables, int row, int column)
{
if (!getGrid().isValidCell(row, column))
{
return;
}
Droppable found = getGrid().getDroppableAt(new Cell(row, column));
if (found != null && !adjacentDroppables.contains(found))
{
adjacentDroppables.add(found);
}
Hmmm preferisco riscriverla in termini di DroppableIteration. Ci do un'occhiata stasera :)
Comunque davvero ottimo il tuo pugno carpiato all'indietro del drago nascente argentato dell'hokuto d'oriente portato a segno sul codice di scoring.
Ottimo lavoro :)
tolto il parametro alla closeChain in ScoreCalculator.
public void closeChain()
{
score += tempScore * chainCounter;
// chainCounter = 0;
tempScore = 0;
}
public void incrementChainCounter()
{
chainCounter++;
}
public int getChainCounter()
{
return chainCounter;
}
public void resetChainCounter()
{
chainCounter = 0;
}
i metodi di grid che prima facevano questo lavoro ora fanno forward su scoreCalculator.
cè un ulteriore cosa che forse si può fare, cioè resettare la catena quando di chiude la chain. Ora le 2 cose sono separate ma forse si possono unire(devo fare qualche prova per vedere quali test falliscono e perchè)
edit: non si possono accorpare, perchè l'informazione della chain serve per il crushBox
ho trovato che Grid.getScoreCalculator() viene usato solo nei test, per avere lo score avendo grid.
Propongo di mettere un metodo getScore() in grid ed eliminare getScoreCalculator()
Hai rotto la build. Fai il revert. Fissa il problema e rifai il commit.
Hai rotto la build. Fai il revert. Fissa il problema e rifai il commit.
non so per quale motivo non mi ha committato la classe di test, e ora ho perso pure i test che avevo aggiunto :( spero di recuperarli con l'history...
edit:recuperati e committati anche loro
La CreateNewBigGemsAction ha cambiato volto !!! :sofico:
Noto ora che l'interfaccia GridObject e' molto (troppo) simile ad un DroppableDescriptor. Che si fa? :D
Come disse Highlander: Ne restera` soltanto uno!
Noto ora che l'interfaccia GridObject e' molto (troppo) simile ad un DroppableDescriptor. Che si fa? :D
Abbiamo un volontario per eliminare GridObject ;)
Ok!! :D
Vado col colpo della roccia di hokuto.
Per conferma: sostituisco al getGridObject -> GetDroppableDescription. Giusto?
vBulletin® v3.6.4, Copyright ©2000-2025, Jelsoft Enterprises Ltd.