BGAFC Vendégkönyv

Σ: 1 post

TCH (statz)

#1, Főfasz (10443)

7416 | #2f6d | ^ | Idézet | Mon, 12 Jan 2015 01:45:42 +01

78.92.*.*

*.catv.pool.telekom.hu

Napi megabölcsesség rovat:
Ha a kóder tudja, hogy mit csinál, akkor nem számít, hogy van goto a kódjában, ha meg nem tudja, akkor meg nem számít, hogy nincs.

Hogy ezt most miért?
Itt van egy-két gyöngyszem a Dune Legacy seedgen kódjából:

if(Pos < 0)
continue;
if(Pos >= 64*64)
continue;
// tobbi kod

Biztos marha nehéz lett volna így felírni:

if (Pos < 0 && Pos >= 64*64)
continue;
// tobbi kod

Vagy még inkább:

if (Pos >= 0 && Pos <= 4095)
{
	// tobbi kod
}

Ilyenből még jópár van. De van ennél viccesebb is:

void SmoothNeighbourhood(Sint16 index, Uint32* pMapArray) {
	Sint16 TileType;
	Sint16 Xcoord;
	Sint16 Ycoord;
	Sint16 Pos;

	TileType = (Sint16) pMapArray[index];

	if(TileType == 8) {
		pMapArray[index] = 9;
		SmoothNeighbourhood(index,pMapArray);
	} else if (TileType == 9) {
		// tobbi kod

Na ez zseniális. Ha a pMapArray[index]-ból érkező TileType 8, akkor a pMapArray[index] legyen 9 és hívjuk meg újra a függvényt ugyanazokkal a paraméterekkel! Na, ezért tökmindegy egy lamernél, hogy gotozik vagy sem, mert itt tulajdonképpen ugyanazt csinálja (feleslegesen), mintha visszaugratná a programot a TileType inicializálásához, csak éppen teszi ezt rekurzív függvényhívással, hogy had fogyjon a verem, hiszen van belőle bőven, nem igaz?!
És a vicc, hogy itt semmilyen ugrás vagy rekurzív hívás nem kell, mivel 8 esetén beállítja a tömb elemét 9-re, így aztán utána (a meghívás után) totál ugyanaz fog lefutni a másik ág irányába, mert a paraméterek nem változnak, azaz legközelebb a TileType tartalma is 9 lesz. Mutatom miről zagyválok:

void SmoothNeighbourhood(Sint16 index, Uint32* pMapArray) {
	Sint16 TileType;
	Sint16 Xcoord;
	Sint16 Ycoord;
	Sint16 Pos;

	TileType = (Sint16) pMapArray[index];

	if(TileType == 8) {
		pMapArray[index] = 9;
		TileType = 9;
	}
	if (TileType == 9) {
		// tobbi kod

Ennyi. Berakjuk, hogy a TileType azonnal legyen 9 és kiszedjük az else ágból a másik irányt, hogy ha nem magától lett 9, hanem mi írtuk át, akkor is lefusson. Méghogy a goto az ördögtől való... A hülyeség és a hülye programozó van az ördögtől! Ördög alatt természetesen bilgécet kell érteni, aki ezeket a balfaszokat beengedte a szakmába...

De egyébként is vannak bajok, pl. itt a SeedRand() (a kommenteket és az üres sorokat kiszedtem belőle):

Uint16 SeedRand() {
	Uint8 a;
	Uint8 carry;
	Uint8 old_carry;
	Seed = SDL_SwapLE32(Seed);
	Uint8* pSeed = (Uint8*) &Seed;
	a = pSeed[0];
	a = a >> 1;
	carry = a & 0x01;
	a = a >> 1;
	old_carry = carry;
	carry = (pSeed[2] & 0x80) >> 7;
	pSeed[2] = pSeed[2] << 1;
	pSeed[2] = pSeed[2] | old_carry;
	old_carry = carry;
	carry = (pSeed[1] & 0x80) >> 7;
	pSeed[1] = pSeed[1] << 1;
	pSeed[1] = pSeed[1] | old_carry;
	carry = (carry == 1 ? 0 : 1);
	a = ((Uint16) a) - (((Uint16) pSeed[0]) + ((Uint16) carry));
	carry = a & 0x01;
	pSeed[0] = (pSeed[0] >> 1) | (carry << 7);
	a = pSeed[0] ^ pSeed[1];
	Seed = SDL_SwapLE32(Seed);
	return ((Uint16) a);
}

És akkor ugyanezt épeszűen:

Uint16 SeedRand() {
	Uint8 carry;
	Seed = SDL_SwapLE32(Seed);
	Uint8* pSeed = (Uint8*) &Seed;
	Uint8 a = pSeed[0] >> 1;
	carry = (pSeed[1] >> 7) ^ 1;
	pSeed[1] = (pSeed[1] << 1) | (pSeed[2] >> 7);
	pSeed[2] = (pSeed[2] << 1) | (a & 0x01);
	a = (a >> 1) - (pSeed[0] + carry);
	pSeed[0] = (pSeed[0] >> 1) | ((a & 0x01) << 7);
	a = pSeed[0] ^ pSeed[1];
	Seed = SDL_SwapLE32(Seed);
	return a;
}

Végülis nem baj, hogy a felesleges dupla hozzárendelgetések és a műveletek szarul összeállított sorrendje miatt eggyel több változó és kétszer annyi kód (és művelet) van... A carry megfordítása meg aztán külön díjat érdemel (carry = (carry == 1 ? 0 : 1);), mert az a gyöngyszem a legszebb az egészben; ha az nem is esett le neki, hogy csak az utolsó bit érintett és elég azt negálni egy xorral (carry ^= 1;), azért egy logikai tagadás (carry = !carry;) helyett ezt a szamárságot felírni azért durva. (Nem baj, hogy szétcseszi a csövet az elágazás? Bár mondjuk a fordítókat talán már felkészítették az ilyen szintű lamerségre is...) Az átírt kód egyébként nem csak kisebb, hanem laza 16%-al gyorsabb is...
Egyébként a Seed változó forgatása is felesleges ebben a függvényben, mert elég a szülő eljárásban a nyitásnál megfordítani, majd a végén vissza, nem kell a véletlenszámgenerálásnál oda-vissza forgatni. Ha meg ráadásul a program más része nem is foglalkozik ezzel a változóval, akkor a szülő eljárásban se kell visszaforgatni a végén, hiszen nem kell másutt.

Aztán itt van még ez a gyönyörűség is:

if( (randNum+4) < point.x) {
	MapArray[i] = 0x06;
} else if(point.x >= randNum) {
	MapArray[i] = 0x04;
} else if(point.x <= point.y) {
	MapArray[i] = 0x02;
} else {
	MapArray[i] = 0x00;
}

Szívem melegség járja át, valahányszor ilyen undormányt látok. Nem igaz, hogy ez ennyire nehéz, hogy felesleges elágazásokkal ne basszuk telibe a kódot. Az ilyen sima értékadós if-elseif "tornyokra" kurwa jó kis "fatáblákat" lehet felírni. Értsd: van egy tömböd, ahol az összes lehetséges kombináció fel van sorolva és a különböző feltételek a "toronyban" elfoglalt "emeletüknek" megfelelő bitet kapják az indexelő változóban.
Kicsit érthetőbben: itt három feltétel van, akkor az három bittel leírható, azaz összesen nyolc kombináció van. Ha a legtetjén csücsülő (randNum+4) < point.x teljesül, akkor mindenképpen 6 lesz a végeredmény, vagyis a tömb azon elemeiben, ahol az indexben ennek a feltételnek a bitje aktív, ott 6 lesz az érték. Ha az eggyel lejjebb lévő point.x >= randNum teljesül, akkor a végeredmény 4, kivéve, ha a felette lévő teljesül. Ez az ág kapja a 2-es bitet, ahol ez a bit aktív, de a 4-es nem, ott az elem 4. Az eggyel lejjebbi szintet is így kell felírni. Ha egyik sem teljesül, akkor 0 az eredmény. Hogy még érthetőbb legyen, itt a tábla:

(randNum+4) < point.x	point.x >= randNum	point.x <= point.y	Érték
0	0	0	0
0	0	1	2
0	1	0	4
0	1	1	4
1	0	0	6
1	0	1	6
1	1	0	6
1	1	1	6

És itt van a belőle képezhető tömb:

const Uint8 kecske[8] = {0, 2, 4, 4, 6, 6, 6, 6};

És így ez a "torony" így módosul:

MapArray[i] = kecske[(((randNum+4) < point.x) << 2) | ((point.x >= randNum) << 1) | (point.x <= point.y)];

És ennyi. Egy kurwa nagy "torony" helyett egy sornyi kód (meg egy előre deklarált konstans tömb az elején, ami nem kerül futásidőbe) és egy a csövet szétcsesző if szerkezet helyett egy sima indexelés.

Ettől a sok marhaságtól függetlenül persze a köszönet még jár a Dune Legacy fejlesztőinek, hogy legalább van publikus seedgenerátor a Dune 2-höz, de azért jópár WTF volt a kódban.