std::vector problems

[ArBuZ]

Hi!

I have some problems with using vectors. And for showing my problem, Ive made some simple example.

 

I have my own class "myClass" This class has Sound attribute as protected member:

 

class myClass
{
protected: 
Sound sound;
public:

myClass(std::string soundPath);
myClass();
~myClass(void);

Sound GetSound();
};

myClass::myClass(std::string soundPath)
{
sound.Load(soundPath);
}

myClass::~myClass(void)
{
sound.Free();
}

Sound myClass::GetSound()
{
return sound;
}

 

In main loop I try to collect "myClass" objects to a vector and access the sound for creating source via the vector.

Here is the code:

 

int WINAPI WinMain( HINSTANCE hInstance,
				HINSTANCE hPrevInstance,
				LPSTR lpCmdLine,
				int nShowCmd ) 
{
Engine engine( "VectorTest", 800, 600 ) ;
Engine::SetAbstractPath( "C:/Leadwerks Engine SDK" ) ;
Engine::SetFilters() ;

World world( CREATENOW ) ;
if( !world.IsValid() ) 
{
	MessageBoxA(0,"Error","Failed to create world.",MB_OK);
	return engine.Free() ;
}

Buffer gbuffer( Engine::GetWidth(), Engine::GetHeight() );

Camera camera( CREATENOW );
camera.SetPosition( 0, 0, -2 );

Material material("abstract::cobblestones.mat") ;
Cube	 mesh( CREATENOW ) ;
mesh.Paint( material ) ;

Cube ground( CREATENOW ) ;
ground.SetScale( 10, 1, 10 );
ground.SetPosition( 0, -2, 0 );
ground.Paint( material ) ;

DirectionalLight light( CREATENOW ) ;
light.SetRotation( 45 , 45, 45 );

Source source;

myClass myC("abstract::gunshot.ogg");
myClass myC2("abstract::reload.ogg");

std::vector <myClass> vec;
vec.push_back(myC);
vec.push_back(myC2);

// Game loop
while( !Keyboard::I****() && !Engine::IsTerminated() )
{
	if( !Engine::IsSuspended() ) // We are not in focus!
	{
		if (Keyboard::I****(KEY_SPACE))
		{
			if (source.IsValid())
				source.Free();
			source.Create(vec[0].GetSound(),EAX);
			source.Play();
		}
		// Rotate cube
		mesh.Turn( Vec3( 0.5f*AppSpeed() ) ) ;

		// Update timing and world
		Engine::UpdateTime();
		World::Update( Engine::GetSpeed()) ;

		// Render
		gbuffer.Set();
		World::Render() ;
		Engine::GetBackBuffer().Set() ;
		World::RenderLights(gbuffer);

		// Send to screen
		Engine::Flip(0) ;
	}
}

// Done
return engine.Free() ;
}

 

This code gives me error on leFreeSound() command that I don't call.

If I don't use vectors, and just get sound of any object it works fine.

 

Please help me with this issue. Thanks in advanced.

 

PS I know that Lumooja is a fan of vectors So please, I need your help

[Canardia]

The problem is your MyClass destructor, which calls FreeSound(). Don't put any code into constructors and destructors, since they will be called at unwanted times.

[ArBuZ]

Nope. Even if comment out sound.Free() in the destructor, I get the same error

[Canardia]

Well, I tried it, and the destructor is called once after the 2nd push_back, which makes no sense, so destructors have completely illogical behaviour.

 

And then you have another mistake, which also called FreeSound():

Sound myClass::GetSound()
{
       return sound;
}

Here you return a copy of the Sound class, so the copy is destroyed after the function returns, thus it calls also the destructor of the Sound class. To fix this, always return references to attributes:

Sound& myClass::GetSound(void) const
{
       return sound;
}

[ArBuZ]

First thanks for your time.

 

Well, if I change it to Sound& myClass::GetSound(void) const then nothing plays at all.

 

But!!! If I use TSound instead of LEO::Sound then everything works very fine!

 

So I guess its LEO issue, isn't it?

[Canardia]

LEO's Sound class is doing FreeSound() in its destructor, which is normally not wrong, but when using it with a vector, it seems the temporary object's (which is used to feed to vector) destructor is called for no apparent reason. C++ should not call any destructors before the object is released from memory.

[ArBuZ]

I thought isReference is used to prevent this.

[Canardia]

Yes, but isReference is only true when Sound is initialized with an entity in its constructor, or assigned an entity with the = operator. But I don't think the problem is with the Sound class, but with C++'s random destructors. I have to find a hack how to prevent C++ from calling destructors at random times. The easiest would be of course to use always .Create() and .Free() manually, but some people don't like that either.

 

Here's a demo to demonstrate the C++ bug:

#include <stdio.h>
#include <vector>
#include <string>
using namespace std;

class myClass
{
public:
string name;
inline myClass(){name="fail";printf("constructor\n");}
virtual ~myClass(){printf("destructor for %s\n",name.c_str());}
virtual void Load(const string& s){name=s;printf("loading sound %s\n",name.c_str());}
};

int main()
{
myClass myC;

printf("creating vec\n");
vector<myClass> vec;

printf("pushing 1\n");
myC.Load("abstract::gunshot.ogg");
vec.push_back(myC);

printf("pushing 2\n");
myC.Load("abstract::reload.ogg");
printf("C++ BUG: there should be no destructor called here,\
besides there are not even 4 objects, but only 3!\n");
vec.push_back(myC);

printf("array has %d elements\n",vec.size());
for(int i=0;i<vec.size();i++)printf("%d: %s\n",i,vec.at(i).name.c_str());

printf("end of prog, all 3 destructors should come next:\n");
}

[Rick]

I have to find a hack how to prevent C++ from calling destructors at random times.

 

?? I'm guessing it's not calling it at random times.

 

I didn't test this but try storing pointers to your class in the vector. My guess would be that when adding via push_back() it's creating a copy of the class instead of the pointer to the class. I think it's kind of like passing any non pointer/reference to a function. Copies are created of the variable. If you use pointers I'm thinking it'll work.

 

 

[EDIT]

Yes, this seems to be the case. push_back() is creating a copy of your object, so once push_back() is finished that copy goes out of scope and so the destructor gets called, which makes sense. You can use pointers to solve this. That way push_back() is taking a pointer to your class, and no temp object will be created which means no destructor gets called.

[Canardia]

I tried also with pointers, but then no destructors are called at all, and I need manually empty the vector. The code gets also unreadable with those -> instead of ..

I think the only viable solution is to use the infamous .Create() and .Free() methods, and not let C++ decide when a constructor and destructor should be called. That way I have also more control over the performance, since I don't want any additional methods being called in time critical loops.

[Rick]

I tried also with pointers, but then no destructors are called at all, and I need manually empty the vector.

 

Yeah, that's the way it works with pointers.

 

The viable solution is to use pointers. If you do that you are not letting C++ determine when it gets called either. You control it with new and delete.

 

Pointers solve your problem Lumooja. You should just accept them. They give you the control you are asking for

 

 

The code gets also unreadable with those -> instead of .

 

Come on Lumooja, that is just insane. It doesn't become unreadable just because you have to use ->

[Roland]

Your are all missing the problem with the example.

 

In this case the vector is destroyed when it goes out of scope.

This is when we reach the end of main.

 

The problem here is that the engine has been terminated at this

moment "return engine.Free()" so what happens it that those myClass destructor's

are consequentially call AFTER the engine has be closed. What then happens

in the myClass destructor are freeing an object that does not exist any more

as it (and all other Entity's used) has been destroyed in engine.Free already.

 

This can be avoided in various way.

Here I show some of them (I only show the interesting parts of the code)

 

Alt 1

int WINAPI WinMain( HINSTANCE hInstance,
                                       HINSTANCE hPrevInstance,
                                       LPSTR lpCmdLine,
                                       int nShowCmd ) 
{
       Engine engine( "VectorTest", 800, 600 ) ;

       ...........

       myClass myC("abstract::gunshot.ogg");
       myClass myC2("abstract::reload.ogg");

       std::vector <myClass>* vec = new std::vector<myClass>() ;
       vec->push_back(myC);
       vec->push_back(myC2);

       // Game loop
       .......

       // Done

       delete vec ; // all myClass'es are deleted BEFORE engine
                     // as the vector is deleted :
       return engine.Free() ;
}

 

 

Alt 2

int WINAPI WinMain( HINSTANCE hInstance,
                                       HINSTANCE hPrevInstance,
                                       LPSTR lpCmdLine,
                                       int nShowCmd ) 
{
       Engine engine( "VectorTest", 800, 600 ) ;

       ...........

       myClass* myC = new myClass("abstract::gunshot.ogg");
       myClass* myC2 = new myClass("abstract::reload.ogg");

       std::vector <myClass*> vec ;
       vec.push_back( myC );
       vec.push_back( myC2 );

       // Game loop

       // Done

       // we delete all myClass'es (BEFORE engine is terminated )
       for( std::vector<myClass*>::iterator it = vec.begin();
            it ! vec.end(); it++ )
       {
            delete *it ;
       }
       return engine.Free() ;
}

 

Alt 3

int WINAPI WinMain( HINSTANCE hInstance,
                                       HINSTANCE hPrevInstance,
                                       LPSTR lpCmdLine,
                                       int nShowCmd ) 
{
       Engine engine( "VectorTest", 800, 600 ) ;

       ...........

       { // create a new scope
           myClass myC("abstract::gunshot.ogg");
           myClass myC2("abstract::reload.ogg");

           std::vector<myClass> vec ;
           vec.push_back( myC );
           vec.push_back( myC2 );

           // Game loop

           // Done

           // vec goes out of scope and  so does myC, myC2
           // destructors are called BEFORE engine
       }
       return engine.Free() ;
}

[Canardia]

Scopes! Of course, that's what I have been telling too, just forgot it myself now

[ArBuZ]

Roland the error occurs in the main loop and not in the end of program. But I had such problems as you described and solved it as you pointed. Thanks.

 

Rick, you are absolutly right about push_back command. Pointers solved my problem.

 

Lumooja I like to use pointers but need to be more cearefull. And I dont see -> sign is so ugly I can live with it.

[Roland]

Roland the error occurs in the main loop and not in the end of program. But I had such problems as you described and solved it as you pointed. Thanks.

Aha.. Sorry. I misunderstood you problem then.

 

Lumooja I like to use pointers but need to be more cearefull. And I dont see -> sign is so ugly I can live with it.

Yes you are correct. Pointers , new and delete are your best friends in C++.

They are a part of the language and there is absolutely no reason to avoid them.

[Pixel Perfect]

I love pointers and have never had a problem with them ever .... just thought I'd say that in their defence

[Canardia]

Well, in case I have to use pointers some day, I can do it like this:

#define dot ->

int main()
{
MyClass* myC = new MyClass();
vector<MyClass*> vec;
myC dot Load("abstract::gunshot.ogg");
vec.push_back(myC);
}

[Rick]

Oh for the love of god