Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add debug overlay grid, togglable with F4 #138

Merged
merged 1 commit into from Nov 9, 2014
Merged

add debug overlay grid, togglable with F4 #138

merged 1 commit into from Nov 9, 2014

Conversation

franciscod
Copy link
Contributor

Inspired on #135.

This simply draws 2d lines over the screen: it has no fancy shaders or anything. It won't work once we have terrain altitude. But it has helped me to spot what I think it's a bug on the coordinate conversion code :D also, it's useful for fixing #57.


coord::camgame t = coord::tile{0, 0}.to_tile3().to_phys3().to_camgame();

int dx = ((t.x + 96) % 96);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use cpp/util/misc.h:mod() functions to avoid negative number fuckups.

@franciscod
Copy link
Contributor Author

v2, please review :D

@franciscod
Copy link
Contributor Author

should this be on the engine instead of game_main.cpp?

@mic-e
Copy link
Member

mic-e commented Nov 7, 2014

the Engine and GameMain classes will soon be merged to a Game class anyway, so... it doesn't really matter.

@@ -589,6 +594,8 @@ bool GameMain::on_draw() {
// draw terrain
terrain->draw(&engine);

if (this->debug_grid_active) { this->draw_debug_grid(); }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think somewhere in our style guide it says to always put code blocks in the next line, even if it's just one statement, i.e.

if (condition) {
        statement()
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems reasonable!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mic-e
Copy link
Member

mic-e commented Nov 8, 2014

Apart from that missing newline it all looks good now; squash and it's ready to merge.

(C programmers are notorious for using extremely short identifiers because back in the days when it was invented, before video terminals existed, the code was in fact printed by the 300-baud ASR33 teltypes those UNIX gods were using for coding. I don't even have a printer :P)

@franciscod
Copy link
Contributor Author

lol hahahaha

squashed and rebased

coord::camgame camera = coord::tile{0, 0}.to_tile3().to_phys3().to_camgame();

int cam_offset_x = util::mod(camera.x, 96);
int cam_offset_y = util::mod(camera.y, 48);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missed this one

@franciscod
Copy link
Contributor Author

fixed a magic number, now it's fine

@franciscod
Copy link
Contributor Author

(closes #135 on merge?)


glBegin(GL_LINES);
glVertex3f(i * e.tile_halfsize.x * 2 + cam_offset_x - line_half_width + e.tile_halfsize.y * 2, cam_offset_y - line_half_height - 1, 0);
glVertex3f(i * e.tile_halfsize.x * 2 + cam_offset_x + line_half_width + e.tile_halfsize.y * 2, cam_offset_y + line_half_height - 1, 0);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

although the compiler will most likely optimize it, you could extract the redundant multiplications to a common variable.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that common variable would be Engine::tile_size. Seriously, tile_halfsize? What retard had that idea? looks at git blame uhm nvm

@TheJJ TheJJ added lang: c++ Done in C++ code area: renderer Concerns our graphics renderer nice new thing ☺ A new feature that was not there before labels Nov 8, 2014
@franciscod
Copy link
Contributor Author

done! ready for more fixes :D

glVertex3f(i * tilesize_x + x1, y1 - 1, 0);
}

glEnd();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one finaly style thing: this is the preferred style for glBegin blocks (should add that somewhere to the codestyle doc).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okaaay

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't sound all too convinced :)

Had you not used two indentations for some reason, I wouldn't even have noticed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@mic-e
Copy link
Member

mic-e commented Nov 9, 2014

👍

@TheJJ
Copy link
Member

TheJJ commented Nov 9, 2014

👍 merged the change is, my padawan

TheJJ added a commit that referenced this pull request Nov 9, 2014
game: add debug overlay grid, togglable with F4
@TheJJ TheJJ merged commit 0524a6f into SFTtech:master Nov 9, 2014
@franciscod
Copy link
Contributor Author

awwwwwwwww yissssssss

@franciscod franciscod deleted the debug_overlay_grid branch November 9, 2014 23:31
inakoll added a commit to inakoll/openage that referenced this pull request Nov 24, 2014
game: add debug overlay grid, togglable with F4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: renderer Concerns our graphics renderer lang: c++ Done in C++ code nice new thing ☺ A new feature that was not there before
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants