2017-09-06, 20:39
  #1
Medlem
kakelpannas avatar
Jag har gjort tetris i C++ och undrar vad du tycker sticker ut och som kan göras bättre. Jag avser inte använda namespace eller dela upp det i flera filer i det här fallet.

Tack på förhand.

Kod:
#include <ncurses.h>
#include <array>
#include <cstdlib>

// The tetris board
const int board_size_x = 15;
const int board_size_y = 20;

// In-game stats
int block_x = 0; // Position of the moving block inside the board
int block_y = 0;
int tick_force_down; // Ticks down. Force down block when < 0. Resets to 'level' every time the block moves down.
int level; 	// Max ticks before block is forced down. Decreases during the game as score increases.
int score; 	// Current score (number of lines taken)


// Where to print stuff
const int board_x = 10; // Tetris board
const int board_y = 3;
const int score_y = 1; // Scoreboard
const int score_x = 5;
const int next_block_x = 30; // Next block
const int next_block_y = 5;

const int blocksize = 3;
using block = std::array<std::array<int, blocksize>, blocksize>;

// Tetris board. 0 = empty
std::array<std::array<int, board_size_x>, board_size_y> board {};
block current_block {};
block next_block{};


// Draw a colored square
void drawsquare(int y, int x, int color){
	move(y,x);
	attron(COLOR_PAIR(color));
	addch(' ');
	attroff(COLOR_PAIR(color));
}

// Draw block
void drawblock(int row, int col, block & b){
	for(int y=0; y < blocksize; ++y)
		for(int x=0; x < blocksize; ++x)
			if(b[y][x])
				drawsquare(row + y, col + x, b[y][x]);
}

// Draw moving block
void drawmoving(){
	drawblock(board_y+1+block_y, board_x+1+block_x, current_block);
}


// Draw next block
void drawnext(){
	mvprintw(next_block_y, next_block_x, "Next: ");
	drawblock(next_block_y+1, next_block_x+1, next_block);
}


// Randomize next block
void newnext(){
	int c = 1 + rand()%7;	// Color. 1-7 as initialized for ncurses.
	switch(rand()%7){
		case 0:
			next_block = {0,c,0,  0,c,0,  c,c,c};
			break;
		case 1:
			next_block = {c,c,c,  c,c,c,  c,c,c};
			break;
		case 2:
			next_block = {c,c,0,  0,c,c,  0,0,0};
			break;
		case 3:
			next_block = {0,c,0,  0,c,0,  0,c,c};
			break;
		case 4:
			next_block = {0,c,0,  0,c,0,  0,c,0};
			break;
		case 5:
			next_block = {0,c,c,  c,c,0,  0,0,0};
			break;
		case 6:
			next_block = {0,c,0,  0,c,0,  c,c,0};			
			break;
	}

}


// Crystalizes moving block into the tetris board
void raster(){
	for(int y=0; y < blocksize; ++y)
	for(int x=0; x < blocksize; ++x){
		if(!  current_block[y][x])
			continue;
		board[block_y+y][block_x+x] = current_block[y][x];
	}	
}

// block is inside another rasterized block or outside the board?
bool collide(int row, int col, const block & b){
	for(int y=0; y < blocksize; ++y)
	for(int x=0; x < blocksize; ++x){
		if(! b[y][x] )
			continue;

		int y_on_board = row + y;
		int x_on_board = col + x;

		if(x_on_board < 0 || x_on_board >= board_size_x || y_on_board >= board_size_y)
			return true;

		if(board[y_on_board][x_on_board])
			return true;
	}

	return false;
}

// Drops the next block, makes a new next. False on collide.
bool drop(){
	block_y = 1;
	block_x = board_size_x/2 - 1;
	current_block = next_block;
	newnext();

	return !collide(block_y, block_x, current_block);
}

// Rotated right if possible
void rotright(){
	block rot;
	for(int ny=0; ny < blocksize; ++ny)
		for(int nx=0; nx < blocksize; ++nx)
			rot[ny][nx] = current_block[blocksize-1-nx][ny];
	if(collide(block_y, block_x, rot))
		return;
	current_block = rot;
}

// Rotated left if possible
void rotleft(){
	block rot;
	for(int ny=0; ny < blocksize; ++ny)
		for(int nx=0; nx < blocksize; ++nx)
			rot[ny][nx] = current_block[nx][blocksize-1-ny];
	if(collide(block_y, block_x, rot))
		return;
	current_block = rot;
}


// false and refuse on collide
bool movedown(){
	if(collide(block_y+1, block_x, current_block))
		return false;
	++block_y;
	return true;
}

void moveleft(){
	if(collide(block_y, block_x-1, current_block))
		return;
	--block_x;	
}

void moveright(){
	if(collide(block_y, block_x+1, current_block))
		return;
	++block_x;
}



void textout(int y, int x, const char* str){
	mvprintw(y, x, str);
}

// Returns number of cleared lines
int clearlines(){
	int cleared = 0;
	for(int y=0; y < board_size_y; ++ y){
		int squares = 0;
		for(int x=0; x < board_size_x; ++ x){
			if(board[y][x])
				++squares;
		}


		// Drop down all the above lines		
		if(squares == board_size_x){
			++cleared;
			for(int xc=0; xc < board_size_x; ++xc) // Clear line. Important for row 0.
				board[y][xc]=0;

			for(int y2 = y; y2 > 0; --y2) // The line we're moving to
			for(int x2 = 0; x2 < board_size_x; ++x2)
				board[y2][x2] = board[y2-1][x2]; // Move above line to this line
		}
		
		
	}
	return cleared;
}

void drawboard(){
	// Draw a box around the tetris board
	mvaddch(board_y, board_x, ACS_ULCORNER);
	mvaddch(board_y, board_x + board_size_x + 1, ACS_URCORNER);
	mvaddch(board_y + board_size_y + 1, board_x, ACS_LLCORNER);
	mvaddch(board_y + board_size_y + 1, board_x + board_size_x + 1, ACS_LRCORNER);

	for(int i = 1; i <= board_size_x; ++i){
		mvaddch(board_y, board_x + i , ACS_HLINE);
		mvaddch(board_y + board_size_y + 1, board_x + i, ACS_HLINE);
	}

	for(int i = 1; i <= board_size_y; ++i){
		mvaddch(board_y + i, board_x, ACS_VLINE);
		mvaddch(board_y + i, board_x + board_size_x + 1, ACS_VLINE);
	}

	// Draw the filled board squares
	for(int y=0; y < board_size_y; ++y)
		for(int x=0; x < board_size_x; ++x)
			drawsquare(board_y + y + 1, board_x + x + 1, board[y][x]);
}

// Init a new game
void newgame(){
	newnext();
	drop();
	level = 300;
	tick_force_down = level;
	score = 0;
}

bool lost = false;
bool ingame_loop(){
	int c=getch();

	if(c == 'q' || c == 'Q')
		return false;

	if(lost){
		mvprintw(0,0,"You lost. Press q to quit.");
		refresh();
		return true;
	}

	bool down = false;

	if(--tick_force_down < 0){
		tick_force_down = level;
		down = true;
	}

	switch(c){
		case ' ':
			down = true;
			while(movedown())
				;
			break;
		case 'z':
		case 'Z':
			rotleft();
			break;

		case 'x':
		case 'X':
		case KEY_UP:
			rotright();
			break;

		case KEY_LEFT:
			moveleft();
			break;

		case KEY_RIGHT:
			moveright();
			break;

		case KEY_DOWN:
			down = true;
			break;
	}

	if(down){
		tick_force_down = level;
		if(!movedown()){
			raster();
			if(!drop())
				lost = true;
			else{
				int lines = clearlines();
				level -= lines;
				score += lines;
			}
		}
	}

	// Update the screen
	clear();
	drawboard();
	drawmoving();
	drawnext();
	mvprintw(score_y, score_x, "Score: %d", score);
	refresh();

	return true;
}

int main()
{	
	// Init ncurses
	initscr();
	start_color();
	curs_set(0);
	cbreak();
	noecho();
	keypad(stdscr,TRUE);
	for(int i=1; i <= 7; ++i) // man init_pair
		init_pair(i, COLOR_BLACK, i);
	timeout(1);

	newgame();
	while(ingame_loop())
		;

	endwin();
	return 0;
}
Citera
2017-09-07, 02:52
  #2
Medlem
kakelpannas avatar
Jag får en jävla massa "klagomål" på att det är C-kod och inte C++. Man syftar huvudsakligen på kodstilen. Svårt att följa huvudsakligen och bör delas upp.

Det jag inte håller med om är att vissa 'practices' tjänar ett syfte här. Jag är extremt målmedvetet envis om att det ska vara en terminal-tetris som är färdig när den är färdig. Det gör att vissa practices blir over-engineering här. Jag har t.ex. noll nytta av namespaces.
Citera
2017-09-07, 08:57
  #3
Medlem
RostigHinks avatar
Citat:
Ursprungligen postat av kakelpanna
Jag får en jävla massa "klagomål" på att det är C-kod och inte C++. Man syftar huvudsakligen på kodstilen. Svårt att följa huvudsakligen och bör delas upp.

Det jag inte håller med om är att vissa 'practices' tjänar ett syfte här. Jag är extremt målmedvetet envis om att det ska vara en terminal-tetris som är färdig när den är färdig. Det gör att vissa practices blir over-engineering här. Jag har t.ex. noll nytta av namespaces.
Du har ju använt std::array i alla fall men resten är i stort ren C-kod.

Vad du kan göra är att deklarera alla const int som constexpr int i stället så blir konstanterna rvalues och tar inget minne.

Sedan skulle jag göra tetrisbitarna som instanser av en klass.
Citera
2017-09-07, 10:39
  #4
Medlem
Protons avatar
Citat:
Ursprungligen postat av RostigHink
Sedan skulle jag göra tetrisbitarna som instanser av en klass.
Ska man göra det med Cpp är ju förslaget att använda lite klasser, spelplanen kan ju till exempel vara en (som får ansvar för att tillse att bitarna inte faller "out-of-bounds" samt innehålla kollisionsdetektering och initiering av nästa bit, samt radering av en komplett rad).

Bitarna i sig kan man ju börja med att skapa nån slags basklass som får ansvar för att flytta bitarna neråt, samt nån slags interface för att rotera bitarna. Interface för att roteringen kommer göras olika beroende på formen på biten, men den kommer ändå behöva göras av subklasserna.

Nån sån struktur kanske hade varit att föredra?
Citera
2017-09-07, 15:35
  #5
Medlem
kakelpannas avatar
Citat:
Ursprungligen postat av RostigHink
Du har ju använt std::array i alla fall men resten är i stort ren C-kod.

Vad du kan göra är att deklarera alla const int som constexpr int i stället så blir konstanterna rvalues och tar inget minne.

Sedan skulle jag göra tetrisbitarna som instanser av en klass.

Citat:
Ursprungligen postat av Proton
Ska man göra det med Cpp är ju förslaget att använda lite klasser, spelplanen kan ju till exempel vara en (som får ansvar för att tillse att bitarna inte faller "out-of-bounds" samt innehålla kollisionsdetektering och initiering av nästa bit, samt radering av en komplett rad).

Bitarna i sig kan man ju börja med att skapa nån slags basklass som får ansvar för att flytta bitarna neråt, samt nån slags interface för att rotera bitarna. Interface för att roteringen kommer göras olika beroende på formen på biten, men den kommer ändå behöva göras av subklasserna.

Nån sån struktur kanske hade varit att föredra?


Jag kan göra det här med modern C++-idiom, men jag ser inte poängen. Det blir dogmatiskt. Det är som att rosta bröd med kylaren på en CPU när det finns en brödrost. Ett sådant här litet program uttrycks bäst med ett par funktioner och variabler, man behöver inte krångla till det. Men bara för att man kan göra det tycker jag inte att man ska använda sig utav C, varför skulle man?

Jag vill jämföra det med att överdesigna ett hello-world program.

Jag vet också att jag inte ska utveckla programmet, någonsin, för då börjar jag om från början och använder en annan design om det behövs.
Citera
2017-09-07, 17:57
  #6
Medlem
Citat:
Ursprungligen postat av kakelpanna
Jag kan göra det här med modern C++-idiom, men jag ser inte poängen. Det blir dogmatiskt. Det är som att rosta bröd med kylaren på en CPU när det finns en brödrost. Ett sådant här litet program uttrycks bäst med ett par funktioner och variabler, man behöver inte krångla till det. Men bara för att man kan göra det tycker jag inte att man ska använda sig utav C, varför skulle man?

Jag vill jämföra det med att överdesigna ett hello-world program.

Jag vet också att jag inte ska utveckla programmet, någonsin, för då börjar jag om från början och använder en annan design om det behövs.

Nu är inte jag programmerare, utan kan verkligen bara väldigt grundläggande saker med C och Java. Men jag tänker, att även om du gör mindre program, så kan det vara bra att få in en goda vanor redan från början. Men det är bara vad jag tänker...och jag kan som sagt nätt och jämnt programmera.
Citera
2017-09-07, 20:17
  #7
Medlem
kakelpannas avatar
Citat:
Ursprungligen postat av RostigHink
Vad du kan göra är att deklarera alla const int som constexpr int i stället så blir konstanterna rvalues och tar inget minne.
const int på det här viset (i samma fil) optimeras till detsamma som constexpr.


Citat:
Sedan skulle jag göra tetrisbitarna som instanser av en klass.
varför?
Citera
2017-09-07, 20:36
  #8
Medlem
Citat:
Ursprungligen postat av kakelpanna
const int på det här viset (i samma fil) optimeras till detsamma som constexpr.



varför?

Varför ber du om kritik, när du tycker all kritik som folk ger dig är fel och det du gjort är rätt?
Citera
2017-09-07, 21:46
  #9
Bannlyst
Citat:
Ursprungligen postat av kakelpanna
Kod:
	switch(rand()%7){
		case 0:
			next_block = {0,c,0,  0,c,0,  c,c,c};
			break;
		case 1:
			next_block = {c,c,c,  c,c,c,  c,c,c};
			break;
		case 2:
			next_block = {c,c,0,  0,c,c,  0,0,0};
			break;
		case 3:
			next_block = {0,c,0,  0,c,0,  0,c,c};
			break;
		case 4:
			next_block = {0,c,0,  0,c,0,  0,c,0};
			break;
		case 5:
			next_block = {0,c,c,  c,c,0,  0,0,0};
			break;
		case 6:
			next_block = {0,c,0,  0,c,0,  c,c,0};			
			break;
	}

Det räcker med två statements: en definition i början av filen och en tilldelning till next_block i koden. Samla konstanter för sig så blir koden kortare/lättare att läsa och man kan ändra dem på ett ställe.

Behöver man inte använda srand() i C++, eller har du glömt det?

Är du säker på att getch() inte busy-väntar när man har satt en delay med timeout()? Jag har ingen koll på det men tycker att napms() verkar vara ett säkrare alternativ.
__________________
Senast redigerad av SuperSizeMe 2017-09-07 kl. 21:58.
Citera
2017-09-07, 23:19
  #10
Medlem
kakelpannas avatar
Citat:
Ursprungligen postat av yokisuci
Varför ber du om kritik, när du tycker all kritik som folk ger dig är fel och det du gjort är rätt?

Varför skulle vi inte diskutera saken menar du? Det är ju det som är poängen med ett diskussionsforum.
Citera
2017-09-07, 23:35
  #11
Medlem
kakelpannas avatar
Citat:
Ursprungligen postat av SuperSizeMe
Behöver man inte använda srand() i C++, eller har du glömt det?

Är du säker på att getch() inte busy-väntar när man har satt en delay med timeout()? Jag har ingen koll på det men tycker att napms() verkar vara ett säkrare alternativ.


Skulle nog inte ens använda rand() normalt i C++, finns bättre RND i C++. srand(time(0)); annars, men det seedar bara randomfunktionen. Det var inte så viktigt för mig att få bra slumpmässiga värden, var mer intresserad av att bara få tetriset att fungera.


getch() tajmar out efter ca 10ms i curses enligt timeout(10) och returnerar ERR. Inte busy-väntar (aktiv pollande CPU-ätande loop), utan block-väntar och gör annat i andra processer medans. Så ungefär var 10e millisekund eller vid knapptryckning switchar operativsystemet in tetrisspelet igen. timeout(1) ger nog närmare 10ms ändå eftersom att systemklockan ofta är lite trubbig på den fronten.
Citera
2017-09-08, 09:54
  #12
Medlem
RostigHinks avatar
Citat:
Ursprungligen postat av kakelpanna
const int på det här viset (i samma fil) optimeras till detsamma som constexpr.



varför?
... och den dagen du har ett projekt som växer så att du vill dela upp kod i headers och moduler får du gå in och byta till constexpr. Varför inte använda det från början?

Varför? För att du vill använda C++ och det vore väl bra om du använde språkets styrka. Även om man inte måste använda OOP i C++ så finns det fördelar i framförallt läsbarhet.

Men visst, för småprojekt som ryms på ett par skärmsidor är det strunt samma vilken kodningsstil man använder.

Vad jag ställer mig frågande till är vad du egentligen vill diskutera. Är det språket, kodningsmönster eller algoritmerna?
Citera
  • 1
  • 2

Skapa ett konto eller logga in för att kommentera

Du måste vara medlem för att kunna kommentera

Skapa ett konto

Det är enkelt att registrera ett nytt konto

Bli medlem

Logga in

Har du redan ett konto? Logga in här

Logga in