[OpenLayer] Bitmaps and Iterators
Matthew Dalrymple

I'm trying to get this to work... here is the pastebin if you'd rather look at it through that http://pastebin.bafserv.com/699

Anyway, when using an iterator to call the Draw() method nothing actually draws. Pressing space in the example will draw them as Draw() is being called directly.

1/*
2 * Author: Matthew Dalrymple
3 * Revision: One zero one zero zero zero zero one zero
4 * Title: Testing OpenLayer's Bitmap's and vectors/iterators
5*/
6 
7// Include the OpenLayer graphical library
8#include <OpenLayer.hpp>
9#include <vector>
10 
11// Use the OpenLayer namespace
12using namespace ol;
13using namespace std;
14 
15class Test
16{
17public:
18 Test();
19 ~Test();
20 void Draw();
21private:
22 int x;
23 int y;
24 Bitmap theimage;
25};
26 
27Test::Test()
28{
29 this->x = 5;
30 this->y = 10;
31 this->theimage.Load("gnolam.png");
32 if(!theimage.IsValid())
33 allegro_message("ZOMG ITS NOT VALID");
34}
35 
36Test::~Test()
37{
38 this->theimage.Destroy();
39}
40 
41void Test::Draw()
42{
43 this->theimage.Blit(x, y);
44}
45 
46 
47 
48int main()
49{
50 // Sets up the devices needed
51 // keyboard, mouse, timer
52 Setup::SetupProgram(true, false, true);
53
54 // Sets up the screen dimensions
55 // Screen Width, Screen Height, WINDOWED OR FULLSCREEN
56 Setup::SetupScreen(800, 600, WINDOWED);
57
58 Bitmap TheScreen(800, 600, Rgba::WHITE);
59 Bitmap Logo("logo.png");
60
61 Test a, b, c, d;
62
63 vector< Test > TheList;
64 TheList.push_back(a);
65 TheList.push_back(b);
66 TheList.push_back(c);
67 TheList.push_back(d);
68
69 vector< Test >::iterator iter;
70 
71 /*
72 * This loop is a simple demonstration of continuing until
73 * a key is pressed, it is highly inefficient. It is strongly
74 * recommended that you read through the Allegro handbook for
75 * handling keyboard input.
76 */
77 float deltaTime = 0;
78 FpsCounter::Start( 60.0 );
79 while(!key[KEY_ESC])
80 {
81 FpsCounter::NewFrameStarted();
82 deltaTime = FpsCounter::GetDeltaTime();
83
84
85 TheScreen.Blit(0, 0);
86 Logo.Blit(250, 250);
87 for(iter = TheList.begin(); iter != TheList.end(); iter++)
88 iter->Draw();
89
90 // press space and see that Draw works properly
91 if(key[KEY_SPACE])
92 {
93 a.Draw();
94 b.Draw();
95 c.Draw();
96 d.Draw();
97 }
98 
99 Canvas::Refresh();
100 rest(1);
101 }
102
103 TheScreen.Destroy();
104 Logo.Destroy();
105 
106 return 0;
107}END_OF_MAIN() // Required by Allegro & OpenLayer, YOU MUST HAVE THIS

What am I doing wrong/how can I correct this?

Hehe don't ask why I used gnolam's avatar.

EDIT:

Here is the fix for this.. is there anyway to do this with the above example?

1/*
2 * Author: Matthew Dalrymple
3 * Revision: One zero one zero zero zero zero one zero
4 * Title: Testing OpenLayer's Bitmap's and vectors/iterators
5*/
6 
7// Include the OpenLayer graphical library
8#include <OpenLayer.hpp>
9#include <vector>
10 
11// Use the OpenLayer namespace
12using namespace ol;
13using namespace std;
14 
15class Test
16{
17public:
18 Test();
19 ~Test();
20 void Draw();
21private:
22 int x;
23 int y;
24 Bitmap theimage;
25};
26 
27Test::Test()
28{
29 this->x = 5;
30 this->y = 10;
31 this->theimage.Load("gnolam.png");
32 if(!theimage.IsValid())
33 allegro_message("ZOMG ITS NOT VALID");
34}
35 
36Test::~Test()
37{
38 this->theimage.Destroy();
39}
40 
41void Test::Draw()
42{
43 this->theimage.Blit(x, y);
44}
45 
46 
47 
48int main()
49{
50 // Sets up the devices needed
51 // keyboard, mouse, timer
52 Setup::SetupProgram(true, false, true);
53
54 // Sets up the screen dimensions
55 // Screen Width, Screen Height, WINDOWED OR FULLSCREEN
56 Setup::SetupScreen(800, 600, WINDOWED);
57
58 Bitmap TheScreen(800, 600, Rgba::WHITE);
59 Bitmap Logo("logo.png");
60
61 Test a, b, c, d;
62
63 vector< Test* > TheList;
64 TheList.push_back(&a);
65 TheList.push_back(&b);
66 TheList.push_back(&c);
67 TheList.push_back(&d);
68
69 vector< Test* >::iterator iter;
70 
71 /*
72 * This loop is a simple demonstration of continuing until
73 * a key is pressed, it is highly inefficient. It is strongly
74 * recommended that you read through the Allegro handbook for
75 * handling keyboard input.
76 */
77 float deltaTime = 0;
78 FpsCounter::Start( 60.0 );
79 while(!key[KEY_ESC])
80 {
81 FpsCounter::NewFrameStarted();
82 deltaTime = FpsCounter::GetDeltaTime();
83
84
85 TheScreen.Blit(0, 0);
86 Logo.Blit(250, 250);
87 for(iter = TheList.begin(); iter != TheList.end(); iter++)
88 (*iter)->Draw();
89
90 // press space and see that Draw works properly
91 if(key[KEY_SPACE])
92 {
93 a.Draw();
94 b.Draw();
95 c.Draw();
96 d.Draw();
97 }
98 
99 Canvas::Refresh();
100 rest(1);
101 }
102
103 TheScreen.Destroy();
104 Logo.Destroy();
105 
106 return 0;
107}END_OF_MAIN() // Required by Allegro & OpenLayer, YOU MUST HAVE THIS

CGamesPlay

If Bitmap doesn't have a proper copy constructor, it won't be copied, and it won't be displayed. Maybe checking IsValid inside the Draw method will tell you something?

When you push the a, b, c, and d objects into the vector, you push the actual objects, so you actually make a copy of them. When this happens, OpenLayer has to send a copy of the Bitmap to the graphics card. I don't know if it handles this properly. One possible fix is to use a Bitmap* inside the class, and continue to use a vector of objects, but the pointer method is superior for most cases in terms of performance.

Matthew Dalrymple

FYI: CGamesPlay was a major contributor to this being fixed :-D.

Another error that I'm having that CGamesPlay said was the right syntax and I thought so myself is this line:

...
vector < Avatars* > CurrentList, AvatarList;
// Avatar List gets filled so don't worry there is stuff in it
...

CurrentList.reserve(MAX_AVATARS_ON_SCREEN);
for(int i = 0; i < MAX_AVATARS_ON_SCREEN; i++)
  CurrentList.push_back(AvatarList.at(rand() % AvatarList.size()));

*.at() should return a pointer and so it compiles correctly. The problem is that it stops things from being drawn and causes my CPU to be 100% and lags like crazy. I comment that line out and "bam" it's fixed. Is their a correct way to do this or is their a workaround like maybe setting a temp Avatar instance setting it equal to *.at(...) then pushing it to the back of CurrentList? I would rather have a one line simple thing.

CGamesPlay

What is the value of MAX_AVATARS_ON_SCREEN?

Like I said: that line of code is fine. Show the part where you draw, for instance.

Matthew Dalrymple

MAX_AVATARS_ON_SCREEN is set to 15 for now.

This is how they're drawn: 

bool Game::DrawAvatars()
{
  vector< Avatar* >::iterator curAvatar;
  for(curAvatar = CurrentList.begin(); curAvatar != CurrentList.end(); curAvatar++)
    (*curAvatar)->Draw();
  //Log::log("CurrentList.size() = %i", CurrentList.size());
  return true;
}

This is the Avatar's Draw method: 

bool Avatar::Draw(void)
{
  this->m_Avatar.Blit(10, 10);
  return true;
}

EDIT: I hate having difficult questions that no one knows the exact answer to or just ignore me :P:P:P

CGamesPlay

I wonder if you've found the answer yet?

ImLeftFooted

Donno why its hanging there, likely a copy paste error or something. 

CurrentList.reserve(MAX_AVATARS_ON_SCREEN);
for(int i = 0; i < MAX_AVATARS_ON_SCREEN; i++)
  CurrentList.push_back(AvatarList.at(rand() % AvatarList.size()));

Should be: 

#include <algorithm>

CurrentList.resize(AvatarList.size());
random_sample(AvatarList.begin(), AvatarList.end(), CurrentList.begin(), CurrentList.end());

Quote:

Here is the fix for this.. is there anyway to do this with the above example?

Its not pretty:

1class Test
2{
3public:
4 Test();
5 ~Test();
6 void Draw();
7
8 Test(const Test &t)
9 : x(t.x), y(t.y), theimage(t.theimage)
10 {
11 ((Test*)&t).theimage = 0;
12 }
13
14private:
15 int x;
16 int y;
17 Bitmap *theimage;
18};
19 
20Test::Test()
21: theimage(new Bitmap)
22{
23 this->x = 5;
24 this->y = 10;
25 this->theimage->Load("gnolam.png");
26 if(!theimage->IsValid())
27 allegro_message("ZOMG ITS NOT VALID");
28}
29 
30Test::~Test()
31{
32 if(theimage)
33 this->theimage->Destroy();
34 delete theimage;
35}
36 
37.. rest is same (except using -> instead of .)

Matthew Dalrymple
Quote:

I wonder if you've found the answer yet?

Haha nope, well now that DD has posted I might have.

To DD (Or anyone else):

I'm reading about random_sample() from cppreference.com and it seems that it confuses me haha.
AvatarList holds 1 copy of each avatar, now CurrentList is supposed to be holding the actual avatars you'll see and fight. When one dies a random avatar from AvatarList will be popped onto CurrentList. Now will random_sample() work for what I'm doing (just for the initialization though)? Does it randomly put them into CurrentList depending on CurrentList's size? The way it looks it seems like CurrentList needs to already be full :-/.

CGamesPlay

random_sample(AvatarList.begin(), AvatarList.end(), CurrentList.begin() + dead_avatar_index, CurrentList.begin() + dead_avatar_index + 1);That will make it replace 1. I have no idea why you wouldn't just use your code.

Matthew Dalrymple

I would use my code because it doesn't work. It may be logically correct but its causing things to not work, I mean I can rar it up again and show you how it doesn't work :D

Edit: I keep getting the error: random_sample().. first use of this function. And I've included algorithm >.< Rebuilt all... Baaaah.
Eww totally had to include <algo.h> to get it to work :-/ I dunno why.

Jakub Wasilewski
Quote:

Edit: I keep getting the error: random_sample().. first use of this function. And I've included algorithm >.< Rebuilt all... Baaaah.
Eww totally had to include <algo.h> to get it to work :-/ I dunno why.

Err, maybe std::random_sample or "using namespace std;" would help? These seem like errors you get when you forget about a namespace :). This and the fact that <algorithm> probably only wraps <algo.h> in a namespace ;).

ImLeftFooted
Quote:

When one dies a random avatar from AvatarList will be popped onto CurrentList.

[edit]Ah I see. Yes your code is better suited for this task. Just don't use reserve. Its generally useless unless you really know what you're doing. And even when you do, it pretty much will only apply to one compiler.

Matthew Dalrymple
Jakub Wasilewski said:

Err, maybe std::random_sample or "using namespace std;" would help?

Tried that and I am already using the standard namespace.

Dustin Dettmer said:

Just don't use reserve.

Then how do I set the size that CurrentList is going to be filled up to?

CGamesPlay

vec.resize(newsize);:)

Matthew Dalrymple

Bah I never saw resize() on cppreference.com, I always overlook things. That's totally what I wanted to use in my other projects too not reserve(). Umm where can I get an update for all the standard libraries because no matter what I try I can't get random_sample() to work with <algorithm>.

ImLeftFooted

What compiler are you using?

Matthew Dalrymple

MinGW (the version that came with Dev-C++ beta 5 [4.9.9.2 or whatever])

ImLeftFooted

Should support it just fine. Paste some code.

Matthew Dalrymple
Quote:

Paste some code. 

#define MAX_AVATARS_ON_SCREEN 15
...
#include <algorithm>
...
CurrentList.resize(MAX_AVATARS_ON_SCREEN);
random_sample(AvatarList.begin(), AvatarList.end(), CurrentList.begin(), CurrentList.end());

When I use #include <algo.h>
I get errors when trying to draw my CurrentList using (*iter)->Draw() My logfile is full of

Quote:

 is not valid

for all of my Avatars in CurrentList. I changed CurrentList and AvatarList from vector < Avatar* > to vector < Avatar > and I got their names instead of that weird A but they were still not valid >.<. I'm thinking about trying this with an array instead of a vector. Or maybe try and STL::List (but lists and vectors are so similar I wouldn't see how that would effect things.

CGamesPlay

Could you show the code that wrote the "Â is not valid", as well as the full code for the Avatar class?

Matthew Dalrymple

Avatar.hpp

1#ifndef __AVATARS_HPP__
2#define __AVATARS_HPP__
3 
4#include "stdafx.hpp"
5 
6#define TOPLEFT 0
7#define TOPRIGHT 1
8#define BOTLEFT 2
9#define BOTRIGHT 3
10 
11 
12class Avatar
13{
14public:
15 Avatar();
16 Avatar(std::string fileName);
17 ~Avatar();
18 bool Load(std::string fileName);
19 void Draw(void);
20 void Update(void);
21
22 std::string GetName(void);
23 void SetName(std::string name);
24 
25 /**
26 * Position the image in one of the four corners of the screen
27 * Paramater takes: TOPLEFT | TOPRIGHT | BOTLEFT | BOTRIGHT
28 **/
29 void Reposition(int corner);
30private:
31 Bitmap m_Avatar;
32 std::string m_Name;
33 int x;
34 int y;
35 int m_Corner;
36};
37 
38#endif

Avatar.cpp

1#include "../include/stdafx.hpp"
2 
3Avatar::Avatar()
4{
5 // Starting the image at one of the four courners of the map
6 Reposition(rand() % 3);
7}
8Avatar::Avatar(std::string fileName)
9{
10 Load(fileName);
11}
12Avatar::~Avatar()
13{
14 this->m_Avatar.Destroy();
15}
16bool Avatar::Load(std::string fileName)
17{
18 this->m_Avatar.Load(fileName.c_str());
19 Log::verify(this->m_Avatar.IsValid(), "%s did not load correctly", fileName.c_str());
20 if(!this->m_Avatar.IsValid())
21 return false;
22 return true;
23}
24void Avatar::Draw(void)
25{
26 Log::verify(m_Avatar.IsValid(), "%s is not valid", m_Name.c_str());
27 this->m_Avatar.Blit(10, 10);
28}
29 
30void Avatar::Update(void)
31{
32}
33 
34/* DONT MIND THIS FUNCTION YET :D */
35void Avatar::Reposition(int corner)
36{
37 m_Corner = corner;
38 
39 this->x = rand() % SCREEN_W;
40 this->y = rand() % SCREEN_H;
41}
42 
43std::string Avatar::GetName(void)
44{
45 return this->m_Name;
46}
47void Avatar::SetName(std::string name)
48{
49 this->m_Name = name;
50}

Ok to get the log file to produce this:

1Á is not valid
2Á is not valid
3Á is not valid
4Á is not valid
5Á is not valid
6Á is not valid
7Á is not valid
8Á is not valid
9Á is not valid
10Á is not valid
11Á is not valid
12Á is not valid
13Á is not valid
14Á is not valid
15Á is not valid
16(null) is not valid
17(null) is not valid
18(null) is not valid
19(null) is not valid
20(null) is not valid
21(null) is not valid
22(null) is not valid
23(null) is not valid
24(null) is not valid
25(null) is not valid
26(null) is not valid
27(null) is not valid
28(null) is not valid
29(null) is not valid
30(null) is not valid

This is what I did
Game.cpp area:

1bool Game::LoadAvatars()
2{
3...
4 CurrentList.resize(MAX_AVATARS_ON_SCREEN);
5 random_sample(AvatarList.begin(), AvatarList.end(), CurrentList.begin(), CurrentList.end());
6...
7}
8/**
9* Draw the current list of avatars to the screen at their correct coordinates
10**/
11bool Game::DrawAvatars()
12{
13 vector< Avatar* >::iterator curAvatar;
14 for(curAvatar = CurrentList.begin(); curAvatar != CurrentList.end(); curAvatar++)
15 (*curAvatar)->Draw();
16 //Log::log("CurrentList.size() = %i", CurrentList.size());
17 return true;
18}
19 
20/**
21* Update the Avatar sprites to their new locations
22**/
23bool Game::UpdateAvatars()
24{
25 vector< Avatar* >::iterator curAvatar;
26 for(curAvatar = CurrentList.begin(); curAvatar != CurrentList.end(); curAvatar++)
27 (*curAvatar)->Update();
28 return true;
29}

CGamesPlay

Add a copy constructor to your object:

Avatar::Avatar(const Avatar& other)
{
    Log::error("Object not allowed to be copied");
}

Matthew Dalrymple

Same log file. The object isn't being copied. Is that good or bad?

CGamesPlay

Good :)

Unfortunately, I'm at a loss. Start dumping your objects at very frequent intervals and see what causes them to screw up. Make sure they are loaded correctly.

Matthew Dalrymple

Baaaaah, this is what always defers me away from programming sometimes. Everything always looks right, seems like it should work... BAM! RUN TIME ERRORS that are hard as hell to solve.

ImLeftFooted

Which is why strict typing is a wonderful language feature.

Matthew Dalrymple
DD said:

Which is why strict typing is a wonderful language feature.

Can you point me in a direction so I can learn more about this? I Google'd and Wikipedia'd this subject with no specific results. I don't see how this subject directly effects my current project, are you referring to the use of templates in the STL?

EDIT:

I think I might have narrowed down the problem. I am now trying to use an iterator just to draw AvatarList to the screen but that isn't working. So my problem is probably how I'm loading the avatars. Here's my loading code. Is the tempAvatar being erased after it leaves the scope of the do-while loop? Should AvatarList maybe be a vector of Avatar objects and not a vector of pointers to Avatar objects?

Anyway I'll be trying this out while I post the code here:

1bool Game::LoadAvatars()
2{
3 Log::log("[LIST OF AVATARS]");
4 string AvatarDirectory("images/avatars/");
5
6 // The allegro file info structure
7 al_ffblk info;
8
9 // Checking to see if there are PNG files in there
10 if(al_findfirst("images/avatars/*.png", &info, FA_ALL) != 0)
11 {
12 Log::warning("No avatars were found");
13 return false;
14 }
15 // While there are PNG files, lets grab them and load them
16 do
17 {
18 Log::log("%s", info.name);
19 string path = AvatarDirectory + info.name;
20 
21 /* PROBABLE PROBLEM AREA */
22 
23 Avatar tempAvatar(path);
24 tempAvatar.SetName(info.name);
25
26 this->AvatarList.push_back(&tempAvatar);
27 }
28 while (al_findnext(&info) == 0);
29
30 // Close the file search
31 al_findclose(&info);
32
33 Log::log("[/LIST OF AVATARS]");
34 
35 return true;
36}

ImLeftFooted
  do
  {
    Log::log("%s", info.name);
    string path = AvatarDirectory + info.name;

    /* PROBABLE PROBLEM AREA */

    Avatar tempAvatar(path);
    tempAvatar.SetName(info.name);
    
    this->AvatarList.push_back(&tempAvatar);
  } // tempAvater's destructor is called here

Corrected (don't forget to call delete later!): 

  do
  {
    Log::log("%s", info.name);
    string path = AvatarDirectory + info.name;

    /* PROBABLE PROBLEM AREA */

    Avatar *tempAvatar = new Avatar(path);
    tempAvatar->SetName(info.name);
    
    this->AvatarList.push_back(tempAvatar);
  }

Matthew Dalrymple

By calling delete later you mean after I push it onto AvatarList correct? Bah things still aren't drawing and the program is crashing when I try and end it by pressing escape.

ImLeftFooted

No when you're done with this->AvatarList. For example in the destructor. But do it wherever is appropriate for you.

When you give up ownership of your memory is up to you.

Matthew Dalrymple

Yes but tempAvatar is only in the scope of that do-while loop. Maybe I'm missing something here.

Oh and the crashing error I was having was caused by a new not fully implemented feature of my logging system.

Ok well things are totally drawing now, as I'm writing this I just removed the delete tempAvatar line and yeah.. shh <sarcasm> there probably isn't a memory leak now </sarcasm>

Now I'll go around deleting their allocated memory in the game's de-constructor. I'll get back in a little bit with an update.

ImLeftFooted

I believe your ownership of tempAvatar extends beyond the do loop. Do you ever access the pointer again?

Matthew Dalrymple

I don't ever access tempAvatar but I do access AvatarList.

CGamesPlay

Don't forget how pointers work! When you copy a pointer, you don't copy the data it points to, you copy the directions for how to get to the data. Since you made the data yourself (using new), you need to get rid of it yourself, using delete. When do you do that? When you don't need the data any more. Whenever you would erase the avatar from the avatar list, instead of just deleting it, delete the data, then remove it from the list.

Matthew Dalrymple

Well what I was getting at was specifically having to delete tempAvatar, as tempAvatar was only used to load objects into AvatarList. This was done in the constructor of my Game class... So in the de-constructor I did this:

vector< Avatar* >::iterator curAvatar;
for(curAvatar = CurrentList.begin(); curAvatar != CurrentList.end(); curAvatar++)
  delete *curAvatar;

I believe that is correct. What I was trying to get at with my questions was more specifically was if you guys were telling me to delete tempAvatar, which I thought you were which was confusing me. I keep poking at this because I'm a bit on the unsure side and I hate being unsure and still delving into things.

Things are working now and everything is drawing and logging correctly. But because I am ME, this has shown me new errors. Haha now the images aren't randomly drawing to the screen... they are drawing at the same random spots every time I run the program. I'm not even sure all 15 are being drawn.

I might also be having a PWD problem to, as when I compile + run in Dev-C++ I think all the *.png avatars show up... but when I double click on the executable only two show up.

I'll try and solve those problems on my own first of course as programming is about solving problems :P. I might just add on to this thread if I need more help as I don't like to create lots of threads.

EDIT: Haha I'm totally dumb. I forgot I was using the constructor that takes a path to load a file. So when I'm logging stuff in the other constructors I was freaking out on why they weren't getting called... That fixed the X and Y random coord thing I was working on.

EDIT 2: Well imma just give props as the problem is fixed and now I can focus on the logic of my game and have a early version of it up in a day or two on the depot.

Thread #589963. Printed from Allegro.cc