Tetris - kritika – JavaScript, AJAX, jQuery – Fórum – Programujte.com

Dobrý den,
projekt považuji za téměř hotový, avšak než půjdu na další části - co říkáte na "muri.moxo.cz"? Ještě si hraji s layoutem - zkouším nejrůznější rozložení. Především uvítám výhrady vůči standardnímu chování hry Tetris atp. Ovšem rád bych se i ujistil v použitých postupech.
Děkuji.

Zdravím, 
Vaši hru jsem právě vyzkoušel a osobně si myslím, že se povedla. Nějak jsem nepostřehl, jak je udělané to bodování, ale to mi osobně nevadí. Nějaké skóre jsem si nahrál a to mi bohatě stačí. :)

#2 Marek Šrám
Je to přeci podle pravidel Tetrisu... (Nejlepší je shazovat řady jen po 4.)

http://cs.wikipedia.org/wiki/Tetris

Dobře. Jak už jsem psal, osobně si myslím, že se Vám hra povedla :)

ak kocka este nieje v poli tak je mozne ist za okraje ... ked kocka klesne tak sa zasekne o okraj a je koniec

#5 Flowy
Vyřešeno - ? :D

#6 Matěj Andrle
Docela v pohode :)

Naozaj neviem ako to v original tetrise funguje, ale podla mňa, ak nejde otočiť pretože tak by to bolo vnorené v iných kockách, tak by sa to malo posunúť.

#7 lukas.balaz
Posouvá se přeci v jasném intervalu - to je blbost...

#1 Matěj Andrle
Chceš kritiku, máš ji mít  No a jelikož jsme na programátorském fóru, tak se koukneme na kód. Zdroják je docela čitelný a hra bez problémů funguje. Následující výtky proto ber spíše jako rady do budoucna.

Trochu mi v kódu vadí spousta magických čísel, např.:

...
case 19:
case 80:
    pause();
...

Je jasné, že jde o kódy kláves 'p' a 'pause', ale vždycky je lepší si to hodit jako konstanty. Trochu horší už je to tady:

...
for(var row = 3; row < 22; row++)
if(testRow(row))
{
    removeRow(row);
...

Na první pohled není úplně jasné, proč procházíš zrovna řádky 3 - 22. Stejně tak není jasné, proč má být řádek smazán, protože testRow nic moc neříká o tom, co se ve funkci děje. Jestliže do této funkce nahlédnu...

function testRow(row)
{
    var cells = canvas.rows[row].cells;
    return cells[0].style.backgroundPosition && cells[1].style.backgroundPosition &&
    cells[2].style.backgroundPosition && cells[3].style.backgroundPosition &&
    cells[4].style.backgroundPosition && cells[5].style.backgroundPosition &&
    cells[6].style.backgroundPosition && cells[7].style.backgroundPosition &&
    cells[8].style.backgroundPosition && cells[9].style.backgroundPosition;
}

... tak pokud bych JS (případně CSS) moc neovládal, tak stejně nebudu vědět, proč v ní testuješ backgroundPosition na true. A i když JS ovládám (a vím, že převádí neprázdný řetězec na true), tak musím projít celý zdroják, abych pochopil, o co jde. A upřímně si stejně nejsem zcela jistý jestli rozumím správně tomu, že testuješ jestli je v každé buňce řádku kostka. Takže vhodnější název funkce a obalení toho checkování backgroundPosition do samostatné funkce (třeba s názvem isCellOccupied()) by rozhodně přispělo k větší čitelnosti.
V té funkci je ještě jedna věc, kterou bych vytkl - proč v ní nepoužít for cyklus? Je tam 10x to samé, jen s jiným indexem. Stejné je to třeba tady: 

...
previewElement.rows[0].cells[0].style.backgroundPosition = tmpTetromino[0];
previewElement.rows[0].cells[1].style.backgroundPosition = tmpTetromino[1];
previewElement.rows[0].cells[2].style.backgroundPosition = tmpTetromino[2];
previewElement.rows[0].cells[3].style.backgroundPosition = tmpTetromino[3];
previewElement.rows[1].cells[0].style.backgroundPosition = tmpTetromino[4];
previewElement.rows[1].cells[1].style.backgroundPosition = tmpTetromino[5];
previewElement.rows[1].cells[2].style.backgroundPosition = tmpTetromino[6];
previewElement.rows[1].cells[3].style.backgroundPosition = tmpTetromino[7];
...

Je to jen část, celkem tam máš 16 takovýchto řádků, ve kterých se děje to stejné.

Ať ale jen nekritizuju, tak musím uznat, že použití tabulky na vykreslení hrací plochy je docela chytré. Věřím, že by spousta lidí použila canvas, protože ten je dnes v módě...

* Tak ty silene radky, co vypisuje LukoSS, bych resil cyklem a podobne to preview.
var bool = true; for (i=0;i<10;i++) {bool = bool && cells[i].style.backgroundPosition;} return bool;
var bool = false; for (i=0;i<10;i++) {bool = bool || cells[i].style.backgroundPosition;} return bool;
* Nahore to pole tetromino, neslo by to resit jako splitovany text a nasobky velikosti kostky? Kdyz bych chtel treba zvetdeni o 1.5, tak bych vsechny ty hodnoty musel rucne prepocitat.
* validInput lze take resit cyklem nebo
validInput = "0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29" . split(", ");
* To remove by mohlo kontrolovat jen radky, kde dopadla kostka, neni treba vsechny, protoze nenechavas padat uvolnene rady.
* Schazi mi popisek osvetlujici, co znamena Pause, pause break. Chvili mi trvalo, nez mi doslo, ze je to napoveda k ovladacim klavesam.

#10 peter
Vy byste vše řešili jen cykly. Já nechci plácat výpočetním výkonem - když předem znám počet a ten počet není zase tak velký. Kdybych měl ještě testovat kam dopadla kostka - tak by to byl další cyklus a podmínka. Vůbec nechápu jak mi můžete radit větvení a cykly, když to jsou největší neřádi. Předem nevím, kde v tom poli o 16 elementech kostka přesně je - musel bych zkrátka najít kraje. Když mám možnost se cyklu a větvení vyhnout, udělám to. Pokud se neznalý Tetrisu snaží pochopit, proč jsou v cyklu přeskočeny 3 řádky, tak se není čemu divit. Znalý totiž ví, že Tetris má 10x22 - kde první 2 řádky nejsou vidět a první viditelný se testuje pro konec hry...

S cykly nejsou problemy, kdyz se spravne napisi. Hlavne je to prehlednejsi nez 8x pod sebe napsat dlouhy radek.

#12 peter
Nemohu za to, že JS nemá makra. Ale kvůli tomu nebudu přemisťovat náročnost programu do CPU... Přijde mi to neuvěřitelně nelogické. Proč dělat cyklus na kód, který bude neustále stejný? (Být to cyklus v makru, tak to vygeneruje právě tento kód a splňovalo by to oba požadavky - čitelnost a přenesení náročnosti pryč z CPU...)

To mas treba, jak mam tady "var sousede..." http://mujweb.cz/…ry/ff/ff.htm
A tady uz to mam predelane na funkci, ktera to generuje z hraci plochy. http://mlich.zam.slu.cz/js-ff/ff4b.htm (kdyz bych chtel ted misto sestiuhelniku treba hvezdu)

Ja ti nerikam, ze to mas delat, jen ti vysvetluju, proc si myslim, ze cyklus by byl lepsi. Muj nazor. Chtel jsi vedet nazory ostatnich, ne?

#15 peter
Však já se z tebe snažím dostat vysvětlení, proč cyklit v místě, kde to vůbec není potřeba. Prostě jen něco více, než že se mi tím zkrátí kód. To není kdo ví jaký důvod - v editoru mohu funkce sbalit, mohu použít nějaký preprocesor - jak jsem psal, mít to makra... Viděl jsem dost interpretů a přijde mi pořád lepší, když se na začátku expandují makra - kód se spustí jen jednou a pak se již pouští vygenerovaný kód... A smazání jsem dle mého také dobře obhájil - stálo by mne to další cyklus s podmínkou v těle... Nechápej mě špatně - já neodmítám kritiku o kterou jsem žádal - snažím se jen trochu probrat takovouto dle mého zásadní změnu... (Jak jsem psal - je to úplně něco jiného, když má program neustále zjišťovat rozměry, kteréžto však může zjistit právě jednou, oproti přímé deklaraci kódu...)

#11 Matěj Andrle
Vy byste vše řešili jen cykly. Já nechci plácat výpočetním výkonem - když předem znám počet a ten počet není zase tak velký.

No jestli ti je pár ušetřených nanosekund přednější než čitelnější kód, tak si asi budeš práci hledat těžko. Ne nadarmo se předčasná optimalizace uvádí jako jedna z nejčastějších začátečnických chyb. Optimalizace má smysl jen v případě, kdy některá část kódu výrazně brzdí zbytek programu.
Navíc si trochu protiřečíš, protože v kódu máš mimo jiné třeba tohle: 

for(var width = 0; width < 10; width++)
    cells[width].style.backgroundPosition = "";

Není podle tvé logiky lepší tenhle cyklus smazat a dát tam 10x stejný řádek?