Quote:Original post by d h k
The game uses 2D graphics and simple isometric tiles. My tiles use the following index-system on the map:
As you can see, tiles always have either both coordinates even or both odd. However, this way of numbering (while being kinda a pain to set up in the first place) feels much more natural to me than most other common ways to number.
OK, you'll waste every other tile in your array, but ok.
Quote:
This is how tiles are initialized when the game starts:
You're making things much too complicated.
- Objects shouldn't know about the container they're in, or duplicate that information. Thus, tiles shouldn't have 'i' and 'j' members - that information is implicit by their location in the array.
- You don't need to handle odd and even rows separately with the row-index like that (and even then, why not use a boolean like "current_row_tabbed" or something?); you already check if the row value is even in order to see if the tile is inited at all. Further, the way your numbering works actually takes care of the "tabbing" *automatically*; try using an image editor to get the image coordinates of the center of each diamond in your picture, and comparing to the drawn "tile coordinates", if you don't believe me.
In fact, this strange complication of your logic was the problem: you were using x values 0, 1, 2, 3, 4, ... to TILES_X / 2, and then mapping each of those to only *one* of 'x * 2' or 'x * 2 + 1' according to the row_index. To make the existing approach work would have required looping over both values of row_index for each x value.
- Don't make comments that say exactly what the function name does.
- This is C++; use a real string type (std::string), and save yourself from worries elsewhere in the code.
- If you're writing stuff with leading underscores to keep variables separate, you really should be thinking more about the names. But as it turns out, we don't need the '_x' and '_y' at all...
- Simplify boolean logic: what you're interested in is if the *parity of the two values is equal*.
We should be looking at something more like this:
void init_tiles (const std::string& filename) { tiles_drawn = 0; tileset.init(filename, TILE_WIDTH, TILE_HEIGHT, 10 ); for (int j = 0; j < TILES_Y; ++j) { for (int i = 0; i < TILES_X; ++i) { if (is_even(x) == is_even(y)) { tile[x][y].init((x - 1) * (TILE_WIDTH / 2), (y - 1) * (TILE_HEIGHT / 2), grass); } } }}
Yes, it's really that simple.
Quote:And this is how I draw the tiles repeatedly:
And similarly:
- The tile should take care of the calculations involving its own coordinates. Have draw() accept the bounds of the rectangle, and let the tile decide whether or not it appears inside, and its relative position.
- Try using bail-out logic to avoid those
ugly "arrows" while also keeping the conditionals simple.
bool Tile::draw(int left, int top, int width, int height) { if (x + TILE_WIDTH < left) return false; if (x > left + width) return false; if (y + TILE_HEIGHT < top) return false; if (y > top + height) return false; // Again, this is C++; avoid C-style casts doOldDrawing(static_cast<float>(x - left), static_cast<float>(y - top)); return true;}// And then we draw everything in a loop:// See, 'tile[x][y].i' is simply x, according to our init logic, and similarly// for j. So it's redundant, and we're jumping through hoops to get information// that's right in front of us...if (is_even(x) == is_even(y) && tile[x][y].draw(camera.x, camera.y, SCREEN_WIDTH, SCREEN_HEIGHT)) { tiles_drawn++;}
Quote:For confirmation, this is "is_even ()":
AARGH AARGH AARGH.
bool is_even ( int value ) { return value % 2 == 0;}
All you need or want. The comparison is already a boolean, and it's already exactly the boolean you want to return. Don't split things up into cases. It suggests a complexity that
isn't there.
Simplify.