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
1 | #include <allegro.h> |
2 | #include <string> |
3 | #include <vector> |
4 | #include "buffering.cpp" |
5 | #include "mouse_function.cpp" |
6 | #include "main_menu.cpp" |
7 | |
8 | using namespace std; |
9 | |
10 | void setup() |
11 | { |
12 | allegro_init(); |
13 | set_color_depth(16); |
14 | install_keyboard(); |
15 | install_mouse(); |
16 | install_timer(); /*don't know if you need this*/ |
17 | if (set_gfx_mode(GFX_AUTODETECT_WINDOWED,1024,768,0,0)!=0) |
18 | { |
19 | set_gfx_mode(GFX_TEXT,0,0,0,0); |
20 | allegro_message(allegro_error); |
21 | exit(1); |
22 | } |
23 | show_mouse(screen); |
24 | } |
25 | |
26 | int main(void) |
27 | { |
28 | setup(); // sets up the screen |
29 | buffering game_screen; // creates an object where our screen will be held, handles |
30 | // double buffering for us. |
31 | main_menu my_main_menu(game_screen); // we want to set up the main menu so we pass |
32 | // it a reference to game screen so it knows where to set up stuff. kind of important :-D |
33 | |
34 | while(!key[KEY_ESC]) |
35 | { |
36 | my_main_menu.render_menu();// puts the main menu onto the buffer |
37 | game_screen.render(); // puts the buffer onto the viewable screen removing the flickering |
38 | } |
39 | game_screen.destroy_bitmaps();//destroys all the bitmaps on the screen to free up memory. |
40 | allegro_exit(); |
41 | return 0; |
42 | } |
43 | END_OF_MAIN() |
buffering.cpp
1 | #include <allegro.h> |
2 | #include <string> |
3 | #include <vector> |
4 | |
5 | using namespace std; |
6 | #ifndef BUFFERING |
7 | #define BUFFERING |
8 | class buffering |
9 | { |
10 | private: |
11 | // we'l need a buffer and a vector to hold reference to location of bitmaps. |
12 | BITMAP * buffer; |
13 | vector<BITMAP *> bitmaps; |
14 | |
15 | public: |
16 | buffering() |
17 | { |
18 | // the buffer needs to be the entire screen |
19 | // so we use allegro defined variables to make it so. |
20 | buffer=create_bitmap(SCREEN_W,SCREEN_H); |
21 | } |
22 | |
23 | // this is just a wrapper for load_bitmap so we can add their memory locations |
24 | // into a vector for automatic deletion later on. |
25 | void add_bitmap(BITMAP *picture,int x=0,int y=0) |
26 | { |
27 | draw_sprite(buffer,picture,x,y); |
28 | bool exists=false; |
29 | for(int i=0;i<bitmaps.size();i++) |
30 | { |
31 | if(picture==bitmaps<i>) |
32 | { |
33 | exists=true; |
34 | } |
35 | } |
36 | if(!exists) |
37 | { |
38 | bitmaps.push_back(picture); |
39 | } |
40 | } |
41 | |
42 | // same thign as above |
43 | void add_stretched_bitmap(BITMAP * picture,int x=0, int y=0,int width=SCREEN_W, int height=SCREEN_H) |
44 | { |
45 | stretch_sprite(buffer,picture,x,y,width,height); |
46 | bool exists=false; |
47 | for(int i=0;i<bitmaps.size();i++) |
48 | { |
49 | if(picture==bitmaps<i>) |
50 | { |
51 | exists=true; |
52 | } |
53 | } |
54 | if(!exists) |
55 | { |
56 | bitmaps.push_back(picture); |
57 | } |
58 | } |
59 | |
60 | // draw the buffer to the screen |
61 | void render() |
62 | { |
63 | vsync(); |
64 | draw_sprite(screen,buffer,0,0); |
65 | rest(60); |
66 | } |
67 | |
68 | //dynamically destroy all the bitmaps. good stuff. :-D |
69 | // beats the hell out of 20 million calls to destroy_bitmap |
70 | void destroy_bitmaps() |
71 | { |
72 | for(int i=0;i<bitmaps.size();i++) |
73 | { |
74 | destroy_bitmap(bitmaps<i>); |
75 | } |
76 | } |
77 | }; |
78 | |
79 | #endif |
mouse_function.cpp
1 | #include <allegro.h> |
2 | #include <string> |
3 | #include <vector> |
4 | #ifndef MOUSE_FUNCTION |
5 | #define MOUSE_FUNCTION |
6 | class mouse_function |
7 | { |
8 | public: |
9 | mouse_function() |
10 | { |
11 | //something, right now this class is just to see if mouse is hovering over a position |
12 | // later on it will expand to do other things |
13 | } |
14 | bool mouse_hovering(int x,int y, int end_x,int end_y) |
15 | { |
16 | if(mouse_x>=x && mouse_y>=y && mouse_x<=(x+end_x) && mouse_y<=(y+end_y)) |
17 | { |
18 | return true; |
19 | } |
20 | else |
21 | { |
22 | return false; |
23 | } |
24 | } |
25 | }; |
26 | |
27 | #endif |
main_menu.cpp
1 | #include <allegro.h> |
2 | #include <string> |
3 | #include <vector> |
4 | #include "buffering.cpp" |
5 | #include "mouse_function.cpp" |
6 | #ifndef MAIN_MENU |
7 | #define MAIN_MENU |
8 | class main_menu |
9 | { |
10 | private: |
11 | BITMAP *title,*new_game,*new_game_hover,*load_game,*load_game_hover,*delete_game, |
12 | *delete_game_hover,*quit,*quit_hover; |
13 | buffering game_screen; |
14 | mouse_function my_mouse; |
15 | public: |
16 | main_menu(buffering the_screen) |
17 | { |
18 | // make our game_screen point to same game_Screen as main function |
19 | // and then load up all our bitmaps. |
20 | this->game_screen=the_screen; |
21 | title = load_bitmap("images/title.bmp", NULL); |
22 | new_game = load_bitmap("images/new_game.bmp", NULL); |
23 | new_game_hover = load_bitmap("images/new_game_hover.bmp", NULL); |
24 | load_game = load_bitmap("images/load_game.bmp", NULL); |
25 | load_game_hover = load_bitmap("images/load_game_hover.bmp", NULL); |
26 | delete_game = load_bitmap("images/delete_game.bmp", NULL); |
27 | delete_game_hover = load_bitmap("images/delete_game_hover.bmp", NULL); |
28 | quit = load_bitmap("images/quit_game.bmp", NULL); |
29 | quit_hover = load_bitmap("images/quit_hover.bmp", NULL); |
30 | } |
31 | void render_menu() |
32 | { |
33 | // here we add all our bitmaps to the menu based on whether ro not |
34 | // they're being hovered over. |
35 | |
36 | game_screen.add_stretched_bitmap(title,0,0); |
37 | if(my_mouse.mouse_hovering(400,100,new_game->w,new_game->h)) |
38 | { |
39 | game_screen.add_bitmap(new_game_hover,400,100); |
40 | } |
41 | else |
42 | { |
43 | game_screen.add_bitmap(new_game,400,100); |
44 | } |
45 | if(my_mouse.mouse_hovering(400,150,load_game->w,load_game->h)) |
46 | { |
47 | game_screen.add_bitmap(load_game_hover,400,150); |
48 | } |
49 | else |
50 | { |
51 | game_screen.add_bitmap(load_game,400,150); |
52 | } |
53 | if(my_mouse.mouse_hovering(400,200,delete_game->w,delete_game->h)) |
54 | { |
55 | game_screen.add_bitmap(delete_game_hover,400,200); |
56 | } |
57 | else |
58 | { |
59 | game_screen.add_bitmap(delete_game,400,200); |
60 | } |
61 | if(my_mouse.mouse_hovering(400,250,quit->w,quit->h)) |
62 | { |
63 | game_screen.add_bitmap(quit_hover,400,250); |
64 | } |
65 | else |
66 | { |
67 | game_screen.add_bitmap(quit,400,250); |
68 | } |
69 | } |
70 | }; |
71 | |
72 | #endif; |
I only skimmed over the first two code boxes.
1. You should not include .cpp files. Include .h instead.
2. You do not need to call allegro_exit.
3. I don't follow the logic in the buffering class. What's the purpose of adding bitmaps? At which point do you clear the buffer or part of it in order to redraw something that has changed? EDIT: Aaah, I get it now. Yeah, should work even if it's not how most people would do it.
4. When do you destroy the buffer? As I see it you have large memory leaks especially when you pass stuff around by value.
5. Don't use rest for timing purposes.
Others will no doubt point out more problems.
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?
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:
changed draw_sprite to blit, function call looks like this'
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
1 | main_menu(buffering the_screen) |
2 | { |
3 | // make our game_screen point to same game_Screen as main function |
4 | // and then load up all our bitmaps. |
5 | this->game_screen=the_screen; |
6 | title = load_bitmap("images/title.bmp", NULL); |
7 | new_game = load_bitmap("images/new_game.bmp", NULL); |
8 | new_game_hover = load_bitmap("images/new_game_hover.bmp", NULL); |
9 | load_game = load_bitmap("images/load_game.bmp", NULL); |
10 | load_game_hover = load_bitmap("images/load_game_hover.bmp", NULL); |
11 | delete_game = load_bitmap("images/delete_game.bmp", NULL); |
12 | delete_game_hover = load_bitmap("images/delete_game_hover.bmp", NULL); |
13 | quit = load_bitmap("images/quit_game.bmp", NULL); |
14 | quit_hover = load_bitmap("images/quit_hover.bmp", NULL); |
15 | game_screen.mark_bitmap_for_deletion(title); |
16 | game_screen.mark_bitmap_for_deletion(new_game); |
17 | game_screen.mark_bitmap_for_deletion(new_game_hover); |
18 | game_screen.mark_bitmap_for_deletion(load_game); |
19 | game_screen.mark_bitmap_for_deletion(load_game_hover); |
20 | game_screen.mark_bitmap_for_deletion(delete_game); |
21 | game_screen.mark_bitmap_for_deletion(delete_game_hover); |
22 | game_screen.mark_bitmap_for_deletion(quit); |
23 | game_screen.mark_bitmap_for_deletion(quit_hover); |
24 | } |
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
1 | #include <allegro.h> |
2 | #include <string> |
3 | #include <vector> |
4 | #include "buffering.h" |
5 | #include "mouse_function.h" |
6 | #include "main_menu.h" |
7 | |
8 | using namespace std; |
9 | |
10 | void setup() |
11 | { |
12 | allegro_init(); |
13 | set_color_depth(32); |
14 | install_keyboard(); |
15 | install_mouse(); |
16 | install_timer(); /*don't know if you need this*/ |
17 | if (set_gfx_mode(GFX_AUTODETECT_WINDOWED,1024,768,0,0)!=0) |
18 | { |
19 | set_gfx_mode(GFX_TEXT,0,0,0,0); |
20 | allegro_message(allegro_error); |
21 | exit(1); |
22 | } |
23 | show_mouse(screen); |
24 | } |
25 | |
26 | int main(void) |
27 | { |
28 | setup(); // sets up the screen |
29 | buffering game_screen; // creates an object where our screen will be held, handles |
30 | // double buffering for us. |
31 | main_menu my_main_menu(game_screen); // we want to set up the main menu so we pass |
32 | // it a reference to game screen so it knows where to set up stuff. kind of important :-D |
33 | |
34 | while(!key[KEY_ESC]) |
35 | { |
36 | my_main_menu.render_menu();// puts the main menu onto the buffer |
37 | game_screen.render(); // puts the buffer onto the viewable screen removing the flickering |
38 | } |
39 | return 0; |
40 | } |
41 | END_OF_MAIN() |
buffering.h
1 | #include <allegro.h> |
2 | #include <string> |
3 | #include <vector> |
4 | |
5 | using namespace std; |
6 | #ifndef BUFFERING |
7 | #define BUFFERING |
8 | class buffering |
9 | { |
10 | private: |
11 | // we'l need a buffer and a vector to hold reference to location of bitmaps. |
12 | BITMAP * buffer; |
13 | vector<BITMAP *> bitmaps; |
14 | |
15 | public: |
16 | buffering() |
17 | { |
18 | // the buffer needs to be the entire screen |
19 | // so we use allegro defined variables to make it so. |
20 | buffer=create_bitmap(SCREEN_W,SCREEN_H); |
21 | } |
22 | void mark_bitmap_for_deletion(BITMAP *picture) |
23 | { |
24 | bitmaps.push_back(picture); |
25 | } |
26 | |
27 | // this is just a wrapper for load_bitmap so we can add their memory locations |
28 | // into a vector for automatic deletion later on. |
29 | void add_bitmap(BITMAP *picture,int x=0,int y=0) |
30 | { |
31 | draw_sprite(buffer,picture,x,y); |
32 | } |
33 | |
34 | // same thign as above |
35 | void add_stretched_bitmap(BITMAP * picture,int x=0, int y=0,int width=SCREEN_W, int height=SCREEN_H) |
36 | { |
37 | stretch_sprite(buffer,picture,x,y,width,height); |
38 | } |
39 | |
40 | // draw the buffer to the screen |
41 | void render() |
42 | { |
43 | vsync(); |
44 | blit(buffer,screen,0,0,0,0,SCREEN_W,SCREEN_H); |
45 | rest(5); |
46 | } |
47 | |
48 | //dynamically destroy all the bitmaps. good stuff. :-D |
49 | // beats the hell out of 20 million calls to destroy_bitmap |
50 | ~buffering() |
51 | { |
52 | for(int i=0;i<bitmaps.size();i++) |
53 | { |
54 | destroy_bitmap(bitmaps<i>); |
55 | } |
56 | bitmaps.empty(); |
57 | } |
58 | }; |
59 | |
60 | #endif |
mouse_function.h
1 | #include <allegro.h> |
2 | #include <string> |
3 | #include <vector> |
4 | #ifndef MOUSE_FUNCTION |
5 | #define MOUSE_FUNCTION |
6 | class mouse_function |
7 | { |
8 | public: |
9 | mouse_function() |
10 | { |
11 | //something, right now this class is just to see if mouse is hovering over a position |
12 | // later on it will expand to do other things |
13 | } |
14 | bool mouse_hovering(int x,int y, int width,int height) |
15 | { |
16 | if(mouse_x>=x && mouse_y>=y && mouse_x<=(x+width) && mouse_y<=(y+height)) |
17 | { |
18 | return true; |
19 | } |
20 | else |
21 | { |
22 | return false; |
23 | } |
24 | } |
25 | }; |
26 | |
27 | #endif |
main_menu.h
1 | #include <allegro.h> |
2 | #include <string> |
3 | #include <vector> |
4 | #include "buffering.h" |
5 | #include "mouse_function.h" |
6 | #ifndef MAIN_MENU |
7 | #define MAIN_MENU |
8 | class main_menu |
9 | { |
10 | private: |
11 | BITMAP *title,*new_game,*new_game_hover,*load_game,*load_game_hover,*delete_game, |
12 | *delete_game_hover,*quit,*quit_hover; |
13 | buffering game_screen; |
14 | mouse_function my_mouse; |
15 | public: |
16 | main_menu(buffering the_screen) |
17 | { |
18 | // make our game_screen point to same game_Screen as main function |
19 | // and then load up all our bitmaps. |
20 | this->game_screen=the_screen; |
21 | title = load_bitmap("images/title.bmp", NULL); |
22 | new_game = load_bitmap("images/new_game.bmp", NULL); |
23 | new_game_hover = load_bitmap("images/new_game_hover.bmp", NULL); |
24 | load_game = load_bitmap("images/load_game.bmp", NULL); |
25 | load_game_hover = load_bitmap("images/load_game_hover.bmp", NULL); |
26 | delete_game = load_bitmap("images/delete_game.bmp", NULL); |
27 | delete_game_hover = load_bitmap("images/delete_game_hover.bmp", NULL); |
28 | quit = load_bitmap("images/quit_game.bmp", NULL); |
29 | quit_hover = load_bitmap("images/quit_hover.bmp", NULL); |
30 | game_screen.mark_bitmap_for_deletion(title); |
31 | game_screen.mark_bitmap_for_deletion(new_game); |
32 | game_screen.mark_bitmap_for_deletion(new_game_hover); |
33 | game_screen.mark_bitmap_for_deletion(load_game); |
34 | game_screen.mark_bitmap_for_deletion(load_game_hover); |
35 | game_screen.mark_bitmap_for_deletion(delete_game); |
36 | game_screen.mark_bitmap_for_deletion(delete_game_hover); |
37 | game_screen.mark_bitmap_for_deletion(quit); |
38 | game_screen.mark_bitmap_for_deletion(quit_hover); |
39 | } |
40 | void render_menu() |
41 | { |
42 | // here we add all our bitmaps to the menu based on whether ro not |
43 | // they're being hovered over. |
44 | |
45 | game_screen.add_stretched_bitmap(title,0,0); |
46 | if(my_mouse.mouse_hovering(400,100,new_game->w,new_game->h)) |
47 | { |
48 | game_screen.add_bitmap(new_game_hover,400,100); |
49 | } |
50 | else |
51 | { |
52 | game_screen.add_bitmap(new_game,400,100); |
53 | } |
54 | if(my_mouse.mouse_hovering(400,150,load_game->w,load_game->h)) |
55 | { |
56 | game_screen.add_bitmap(load_game_hover,400,150); |
57 | } |
58 | else |
59 | { |
60 | game_screen.add_bitmap(load_game,400,150); |
61 | } |
62 | if(my_mouse.mouse_hovering(400,200,delete_game->w,delete_game->h)) |
63 | { |
64 | game_screen.add_bitmap(delete_game_hover,400,200); |
65 | } |
66 | else |
67 | { |
68 | game_screen.add_bitmap(delete_game,400,200); |
69 | } |
70 | if(my_mouse.mouse_hovering(400,250,quit->w,quit->h)) |
71 | { |
72 | game_screen.add_bitmap(quit_hover,400,250); |
73 | } |
74 | else |
75 | { |
76 | game_screen.add_bitmap(quit,400,250); |
77 | } |
78 | } |
79 | }; |
80 | |
81 | #endif; |