|
Code Review |
Charles256
Member #8,370
February 2007
|
Warning: If you do not want to read code for the sake of review purposes run away! only reason I ask is for some review is because I'm new to the allegro library and not a huge fan of game programming all in one's style so let me know what you think of mine and what I could possibly improve or a feature of allegro I'm misusing, whatever! Let me know. :-D main.cpp
buffering.cpp
mouse_function.cpp
main_menu.cpp
|
miran
Member #2,407
June 2002
|
I only skimmed over the first two code boxes. 1. You should not include .cpp files. Include .h instead. Others will no doubt point out more problems. -- |
orz
Member #565
August 2000
|
Your program is not fullscreen, but uses 16 bpp regardless of desktop depth. You might consider matching the desktop depth or using 32 bpp or something. draw_sprite the buffer to the screen? draw_sprite skips mask colored pixels ie two level transparency. You probably want blit there. You also rest for 60 milliseconds after each frame is drawn. That means even if everything else is infinitely fast, your framerate is capped at 16-17 fps. Using rest() to set the rate of game play is generally frowned upon - for most things using a timer or something is better. You do a linear search through your set of bitmap pointers. Might as well make it a set instead. Why is drawing them to the buffer linked to marking them for deletion? They should be marked for deletion on load, not on render. You do not remove bitmaps from the vector when you destroy them. Which is probably fine, assumeing you only call the destroy() method at the end, but still, it's bad form. In mouse_hovering you use x_end and y_end as the width and height of the region... to me the parameter names imply that they are absolute coordinates. Of course, STL already uses misleading names, so why shouldn't everyone. Your indentation is inconsistent. It may appear consistent to you depending upon your editor settings, but you use 4 spaces in some places and 1 tab in other places. You have two bufferings named game_screen. The contents of one are copied over the other. This is strange, and probably not what you want to be doing. I would suggest that you make main_menu's buffering be a reference. Buffering apparently has some methods not shown. Perhaps they're in the header file? |
Charles256
Member #8,370
February 2007
|
I'll reply to and change some of those things later ( bout to head to class) but I just had to ask before I run out, what do you mean buffer has more methods not shown? Pretty sure I showed them all. :-D Either way, yes. I'll reply to the rest later. Man, y'all giving some tough love. :-D I'm not new to c++ just new to allegro. I'm pretty sure I was passing stuff around by pointer. Hence why me passing game_screen still allowed game_Screen.render() in main to work because it had been passed by pointer instead of by reference because by reference would cause it not to show anything when it tried to render... Of course you could be referring to something else, I'll check back in later. By the way, when you give criticism on my allegro technique (or c++ technique, but hopefully that'll be less frequent ) please tell me what I should be doing instead because I'm not exactly an allegro expert: ) Edit: I agreed with your marking them for deletion on load, mainly because it's less work the program has to do hence faster, so I changed my code for main_menu to the following
and the function mark_bitmap_for_deletion resides in buffering and looks like so void mark_bitmap_for_deletion(BITMAP *picture) { bitmaps.push_back(picture); } changed delete_bitmaps to a destructor, after i removed allegro_exit call so it'd still work. Only issue, I wanted to do delete bitmaps; but my compiler got pretty upset about that. ~buffering() { for(int i=0;i<bitmaps.size();i++) { destroy_bitmap(bitmaps<i>); } bitmaps.empty(); } As for the mouse hovering opinion you're absolutely right, I meant to rename that before I posted. Miran, I'll reply to you in a bit but if I don't get going I swear I'll be killed by my teacher. Thanks for the comments, keep them coming! New code looks as follows! main.cpp
buffering.h
mouse_function.h
main_menu.h
|
|