SDL 2.0 and String Constructor Errors

For the first question we're gonna need to see how you are creating a variable and using it... but in the second question I'm pretty sure it's crashing because (depending on how you are creating a variable of that class) SDL_CreateWindow will crash because SDL_Init hasn't been called. To be sure please show us how you are using the class and post the actual error messages please

Assuming I made a new member of type std::string called Name (which I did), why isn't it initialized properly in the initializer list?

One possibility is that you have a mismatch between the order the member variables are declared and the order used to initialise them in the initialiser list. Somewhat confusingly in C++, member variables are always initialised in the order they are declared in the class. So if you added "Name" after "win", then you would be attempting to use "Name" before it has been initialised. This is why it is a good reason to keep the order of members and the order in the initialiser list in sync.

My first suggestion in to not have complex behaviour in a constructor (in C++) as there's no way to handle fatal errors nicely. If you have complex initialisation then I'd put it in an Initialize method (though, others dislike that also so it depends on your point of view). I would consider std::string initialization 'complex' in this case as it requires a memory allocation.

Please don't pass around std::string's by value.

As to your query, it is hard to give a definite answer as you haven't included the exact code you're using (for example, you initialize a Name variable in your initializer list when no such variable is defined in your class object). So it would just be speculation.

If you make the changes I mention above, it'll probably be easier to see what is wrong.

n!

I guess I over edited my first post so Ill try to make up for it here. I originally wanted to make a SDL encapsulation where I would store my window and GL Context into one class as well as any other information a game engine might need. I ended up getting errors so I reverse engineered them with this test class which has the stuff that my previous class had, but the constructors are more basic.

This code below, will make the test constructor call its destructor right after the constructor is called even though I never call the destructor explicitly. This happens if I pass to test the way I do here a premade string or if I just pass "" without setting it to a variable. In this case, it gives a access violation error because I try to destroy a not initialized window but the question is, why does the destructor get called if I never called it explicitly?

class test
{
private:
	glm::vec2 screen;
	GLuint VBO;
	SDL_Window * win;
	SDL_GLContext g;
public:
	test (std::string Name,unsigned int screenx, unsigned int screeny);
	test::test(std::string Name);
	~test ();
};

test::test(std::string Name){}
test::~test ()
{
    SDL_DestroyWindow(win);
    win = nullptr;
}

int main (int argc, char * argv [])
	{
		// SDL Init
		if ( SDL_Init(SDL_INIT_VIDEO) < 0 )
		{
			printf( "SDL could not initialize! Error: %s\n", SDL_GetError() );
			return 0;
		}
		std::string s = "";
		test d = test(s);

On the other hand, if I change my constructor to this and I make the same call in main but without passing the string, the program runs just fine and the destructor isn't called.

test::test(){}

A more practical example is when I try to create the window, GL context, and make it a red screen with a white dot. Here's an example that works:

class test
{
private:
	glm::vec2 screen;
	GLuint VBO;
	SDL_Window * win;
	SDL_GLContext g;
public:
	test (unsigned int screenx, unsigned int screeny);
	~test ();
};

test::test (unsigned int screenx, unsigned int screeny) : 
	screen (glm::vec2(screenx, screeny)), 
	win (SDL_CreateWindow("",100,100,screen.x,screen.y,SDL_WINDOW_SHOWN | SDL_WINDOW_OPENGL)),
	g (SDL_GL_CreateContext(win))
{
	glewExperimental = GL_TRUE;
	GLenum glewError = glewInit();
	if ( glewError != GLEW_OK )
	{
		printf( "GameEngine: GLEW could not be initialized! Error: %s\n", glewGetErrorString(glewError) );
	}
	if ( win == nullptr )
	{
		return;
	}
	if ( g == NULL )
	{
		return;
	}
	SDL_GL_MakeCurrent(win, g);
	GLfloat Vertex [3];
	Vertex[0] = 0.0f;
	Vertex[1] = 0.0f;
	Vertex[2] = 0.0f;

	glGenBuffers(1, &VBO);
	glBindBuffer(GL_ARRAY_BUFFER, VBO);
	glBufferData(GL_ARRAY_BUFFER, sizeof(Vertex), Vertex, GL_STATIC_DRAW);
	glClearColor(1.0f, 0.0f, 0.0f, 0.0f);
	glClear(GL_COLOR_BUFFER_BIT);
	glEnableVertexAttribArray(0);
	glBindBuffer( GL_ARRAY_BUFFER, VBO );
	glVertexAttribPointer( 0, 3, GL_FLOAT, GL_FALSE, 0, 0 );

	glDrawArrays(GL_POINTS, 0, 1);

	glDisableVertexAttribArray(0);
	SDL_GL_SwapWindow(win);
}

test::~test ()
{
	SDL_DestroyWindow(win);
	win = nullptr;
}

	int main (int argc, char * argv [])
	{
		// SDL Init
		if ( SDL_Init(SDL_INIT_VIDEO) < 0 )
		{
			printf( "SDL could not initialize! Error: %s\n", SDL_GetError() );
			return 0;
		}
		std::string s = "";
		test i = test(100,100);
		SDL_Delay(10000);

		SDL_Quit();
		return 1;
	}

And here's one that calls the destructor right after the constructor is called. Notice the only difference is I pass a string as a parameter to the constructor but I don't actually do anything with this string. The program doesn't crash because the window is initialized but the window instantly gets destroyed instead.

class test
{
private:
	glm::vec2 screen;
	GLuint VBO;
	SDL_Window * win;
	SDL_GLContext g;
public:
	test (std::string Name,unsigned int screenx, unsigned int screeny);
	~test ();
};

test::test (std::string Name, unsigned int screenx, unsigned int screeny) : 
	screen (glm::vec2(screenx, screeny)), 
	win (SDL_CreateWindow("",100,100,screen.x,screen.y,SDL_WINDOW_SHOWN | SDL_WINDOW_OPENGL)),
	g (SDL_GL_CreateContext(win))
{
	glewExperimental = GL_TRUE;
	GLenum glewError = glewInit();
	if ( glewError != GLEW_OK )
	{
		printf( "GameEngine: GLEW could not be initialized! Error: %s\n", glewGetErrorString(glewError) );
	}
	if ( win == nullptr )
	{
		return;
	}
	if ( g == NULL )
	{
		return;
	}
	SDL_GL_MakeCurrent(win, g);
	GLfloat Vertex [3];
	Vertex[0] = 0.0f;
	Vertex[1] = 0.0f;
	Vertex[2] = 0.0f;

	glGenBuffers(1, &VBO);
	glBindBuffer(GL_ARRAY_BUFFER, VBO);
	glBufferData(GL_ARRAY_BUFFER, sizeof(Vertex), Vertex, GL_STATIC_DRAW);
	glClearColor(1.0f, 0.0f, 0.0f, 0.0f);
	glClear(GL_COLOR_BUFFER_BIT);
	glEnableVertexAttribArray(0);
	glBindBuffer( GL_ARRAY_BUFFER, VBO );
	glVertexAttribPointer( 0, 3, GL_FLOAT, GL_FALSE, 0, 0 );

	glDrawArrays(GL_POINTS, 0, 1);

	glDisableVertexAttribArray(0);
	SDL_GL_SwapWindow(win);
}

test::~test ()
{
	SDL_DestroyWindow(win);
	win = nullptr;
}

	int main (int argc, char * argv [])
	{
		// SDL Init
		if ( SDL_Init(SDL_INIT_VIDEO) < 0 )
		{
			printf( "SDL could not initialize! Error: %s\n", SDL_GetError() );
			return 0;
		}
		std::string s = "";
		test d = test(s, 100, 100);
		SDL_Delay(10000);

		SDL_Quit();
		return 1;
	}

All of these problems are solved if the string gets passed by reference but my question is, why is it that just by passing a string by value does my destructor automatically get called right after my constructor is called?

My second question is one that I sort of figured out why it crashes but not why it should crash. So here is an example that doesn't crash (assume that I call the constructor in main just as I have for the rest of my above code given the parameters and if I pass a string, its an empty string).

class test
{
private:
    glm::vec2 screen;
    GLuint VBO;
    SDL_Window * win;
    SDL_GLContext g;
    std::string Name; // Added
public:
    test (std::string _Name, unsigned int screenx, unsigned int screeny);
    ~test ();
};

test::test (std::string _Name, unsigned int screenx, unsigned int screeny) : 
	Name (_Name),
	screen (glm::vec2(screenx, screeny)), 
	win (SDL_CreateWindow("",100,100,screen.x,screen.y,SDL_WINDOW_SHOWN | SDL_WINDOW_OPENGL)),
	g (SDL_GL_CreateContext(win))
{
    // The exact same code as the above constructor, make a red window with a white dot
}

This however, crashes giving access violation error at 0xcccccccc.

test::test (std::string _Name, unsigned int screenx, unsigned int screeny) :
    Name (_Name),
    screen (glm::vec2(screenx, screeny)),
    win (SDL_CreateWindow(Name.c_str(),100,100,screen.x,screen.y,SDL_WINDOW_SHOWN | SDL_WINDOW_OPENGL)),
    g (SDL_GL_CreateContext(win))
{
....
}

The only difference between this and the top is this part of the code:

win (SDL_CreateWindow(Name.c_str(), ...

In debug, I also get that Name is a and I kinda figured out why. When I call Name (_Name) in the initialzier list, apparently it doesn't do the copying properly so instead, its actually a undefined variable. Then in SDL_CreateWindow, I'm trying to create a const char pointer to this memory which doesn't exist, therefore generating an exception. My question for this is, why is it that calling Name(_Name) doesn't actually copy over the strings value but instead, leaves it at undefined? As you can see as well, I'm initializing in the order of use so by the time win gets initialized, Name should be initialized as well. This works normally for g which creates a valid GL Context so I have to be going in the right order from top to bottom.

As a small note, I realize that passing a string by value isn't the best way to get things done but I'm interested in to knowing why these errors happen when I do so. I'm also doing this in the constructor because I feel that having an init method is pretty redundant if I just call it in the constructor anyways.

I don't see anything obvious, but try changing

test i = test(100,100); // creates temporary test object and assigns the temporary object to the object i

to

test i(100, 100); // this creates test object i, and calls constructor of i

The option you gave doesn't have issues in the first place, I only get errors when a string is involved. Nether less, I tried what you said but with a string and it actually works now. That's weird, so creating a temporary object and assigning it gives errors but just creating the object and constructing it doesn't. But the temporary object isn't a pointer, is it because of the copy operator?

EDIT: Actually wait, if its a copy operator issue, why doesn't the window just close when the string isn't involved since it is a pointer too. Something has to be invoked because of the string being there.

The syntax you're using for creating your variable means that you want to create an anonymous temporary object, and then copy construct your named variable from that. Of course, any such temporary needs to be destroyed too, so that is what you are seeing - the creation and destruction of the temporary.

What you are not seeing is the calling of the (default generated) copy constructor. As your class does not obey the rule of three, this causes undefined behaviour. You'd get the same behaviour if you accidentally invoked the default generated assignment operator. In C++, it is generally a good idea to think about the allowed copying behaviour. Unless your type is a simple value object such as a mathematical vector, a good rule of thumb is to disable copying unless you can think of sensible copying semantics. Disabling can be done like so:

// C++ 11
class Example {
public:
    Example(/* some constructor */);
    ~Example();

    // Non-copyable
    Example(const Example &) = delete;
    Example &operator=(const Example &) = delete;
    
    // ...
};

// Pre C++ 11
class Example {
public:
    Example(/* some constructor */);
    ~Example();

    // ...

private:
    // Non-copyable, deliberately private and unimplemented
    Example(const Example &);
    Example &operator=(const Example &);
};

This way you'll get a compiler (or linker) error if your code accidentally tries to copy or assign a complex object. It is possible to factor the pre C++ 11 version into a helper class that is privately inherited from, I've not used C++11 much so while I suspect that would work too you should test this out. An example of such as helper is boost::noncopyable.

The compiler is allowed elide unnecessary copies in some circumstances, for example when returning a variable by value from a function (RVO or NRVO) - this may explain some of the variation you're seeing. The compiler might be choosing to elide the copy based on whether you pass a temporary string or not, perhaps because the order of destroying the temporary arguments relative to invoking the copy constructor is important. I've not been actively developing C++ for some time, so you'll need to consult the standard or a suitably up to speed language lawyer to give you more information about the exact rules in play there.

The syntax ultramailman suggests is what you want, but one gotcha to look out for is trying to use this approach for a default constructor:

test i();

Due to C++'s parsing rules, this is actually parsed as a function declaration (yes, you can declare the existence of functions inside other functions) of a function named "i" taking no arguments and returning a "test" object. So when you try to use this "variable", you'll get some odd errors.

Wow okay that makes a lot more sense. I always assumed writing test i = test() initialized the i value instead of copying over a value. Okay so I redid my class a bit, since I'm using VS 2010, I can't do it the C++11 way with delete methods since it isn't supported on that compiler. I instead did it your way, and I sort of got a Game engine class up and running using SDL. Doing so got rid of both errors, so both of my questions have sort of been answered. I guess the string gave the errors because of how it was being copied when i did test(...).

I rewrote some of my code to use unique_ptr for the window. Its my first time using unique_ptr so I'm not completely sure how it works but I get a rough idea. Only weird thing being how I initialize it, I can't just make it equal to nullptr anymore, I instead have to give it a function pointer to DestroyWindow I think so that it knows how to delete the variable (or something like that). I also set the pointer to null first in the initializer list and then make a window out of it later since by default pointers aren't nullptr's so I guess this is safer. Here's the code:

	class GameEngine
	{
	private:

		GLuint VBO;
		glm::vec2 Screen;
		SDL_GLContext GLContext;
		std::unique_ptr <SDL_Window, void (*)(SDL_Window*)> Window;
		std::string Name;

		// Copy Constructor, non copyable
		GameEngine (const GameEngine & other);
		// Copy Assignment Operator, non copyable
		GameEngine& operator= (const GameEngine &);

		void CreateVertexBuffer ();
		void Render ();

	public:
		// Constructor
		GameEngine (std::string _Name, unsigned int ScreenX, unsigned int ScreenY);
		// Destructor
		~GameEngine ();

		void MainLoop ();
	};

	GameEngine::GameEngine (std::string _Name, unsigned int ScreenX, unsigned int ScreenY) : 
		Name (_Name),
		Screen (glm::vec2(ScreenX, ScreenY)),
		Window (std::unique_ptr<SDL_Window, void (*)(SDL_Window*)>(nullptr, SDL_DestroyWindow))
	{
		// Check if Window is valid
		Window.reset(SDL_CreateWindow(Name.c_str(), 100, 100, Screen.x, Screen.y, SDL_WINDOW_OPENGL | SDL_WINDOW_SHOWN));
		if ( Window == nullptr )
		{
			printf( "GameEngine: Window could not be created! Error: %s\n", SDL_GetError() );
			return;
		}
		// Check if OpenGL context
		GLContext = SDL_GL_CreateContext(Window.get());
		if ( GLContext == NULL )
		{
			printf( "GameEngine: OpenGL context could not be created! Error: %s\n", SDL_GetError() );
			return;
		}
		// Use OpenGL 3.1 core
		SDL_GL_SetAttribute( SDL_GL_CONTEXT_MAJOR_VERSION, 3 );
		SDL_GL_SetAttribute( SDL_GL_CONTEXT_MINOR_VERSION, 2 );
		SDL_GL_SetAttribute( SDL_GL_CONTEXT_PROFILE_MASK, SDL_GL_CONTEXT_PROFILE_CORE );
		// Initialize GLEW
		glewExperimental = GL_TRUE;
		GLenum glewError = glewInit();
		if ( glewError != GLEW_OK )
		{
			printf( "GameEngine: GLEW could not be initialized! Error: %s\n", glewGetErrorString(glewError) );
		}
	}

	GameEngine::~GameEngine ()
	{
		SDL_GL_DeleteContext(GLContext);
		SDL_DestroyWindow(Window.get());
		Window.release();
		Window = nullptr;
	}

	void GameEngine::CreateVertexBuffer ()
	{
		GLfloat Vertex [3];
		Vertex[0] = 0.0f;
		Vertex[1] = 0.0f;
		Vertex[2] = 0.0f;

		glGenBuffers(1, &VBO);
		glBindBuffer(GL_ARRAY_BUFFER, VBO);
		glBufferData(GL_ARRAY_BUFFER, sizeof(Vertex), Vertex, GL_STATIC_DRAW);
	}

	void GameEngine::MainLoop ()
	{
		glClear( GL_COLOR_BUFFER_BIT );

		this->Render();

		SDL_GL_SwapWindow( Window.get() );
	}

	void GameEngine::Render ()
	{
		CreateVertexBuffer();
		glEnableVertexAttribArray(0);
		glBindBuffer( GL_ARRAY_BUFFER, VBO );
		glVertexAttribPointer( 0, 3, GL_FLOAT, GL_FALSE, 0, 0 );
		glDrawArrays(GL_POINTS, 0, 1);
		glDisableVertexAttribArray(0);
	}

Thanks for all the help guys and if there's still some weird bug causing thing I'm doing, let me know :D.

You use of std::unique_ptr is odd. You're losing the benefits by calling SDL_DestroyWindow and then cleaning up the object manually. You should be able to safely remove those three lines of code.

Other than that, your class has complex resource handling requirements because it is doing many different things. Classes should strive to have a single responsibility. An alternative might be to have a Window and/or Renderer class, a VertexBuffer class and then create a GameObject class that uses these.

Right now, I would guess your game would crash if an error occurs creating the Window or initialising the OpenGL context, and there is no guarantee that your print statements will have been flushed and will be visible in any logs. Consider using exceptions to signal failure to construct an object, or having a status member to record if an error occurs that must be tested by the client code, or having a separate initialisation function that can return an error if necessary.

Oh true, I should fix that, thanks :D

Well the VBO being there is only because I needed it for testing. Eventually, Id have some kind of resource managing class.

As for single priority, at some level you might want an encapsulation of your engine code and that's what this class will be. Its either this or having everything done in main with globals or something which I would much rather not do.

Why isn't it guaranteed the prints will run? I know some people say some prints might break during certain situations but I don't really understand why. If the if statement is true, then the program has to run the print before it can return right?

Why isn't it guaranteed the prints will run? I know some people say some prints might break during certain situations but I don't really understand why. If the if statement is true, then the program has to run the print before it can return right?

The print function run, it's just that they might not actually print to the screen until you flush the buffer or the buffer gets full.

Oh okay, thanks a lot!