Fixing a possibly poorly initialized data member

[WaaaaW]

Hi, I'm relatively new to C++. I wrote up a short program that uses the SFML library (the problem seems to be unrelated to it, though), but it doesn't act as intended. Could you guys help me with my problem?

The issue seems to lie in my two files GameManager.h and GameManager.cpp:

This is GameManager.h:

#include <SFML/Graphics.hpp>
#include "Thing.h"

class TextureManager{
	std::map<std::string, sf::Texture*> textureManager;
public:
	TextureManager()
		:textureManager()
	{}
	sf::Texture* loadTexture(const std::string& filename);
};
class GameManager{
	static std::vector<Thing*> thingList;
	static TextureManager textureManager;
public:
	static Thing* createNewThing(const std::string& spriteFilename);
};

And GameManager.cpp:

#include <SFML/Graphics.hpp>
#include "GameManager.h"

TextureManager GameManager::textureManager = TextureManager();
std::vector<Thing*> GameManager::thingList = std::vector<Thing*>();

sf::Texture* TextureManager::loadTexture(const std::string& filename){
	std::map<std::string, sf::Texture*>::iterator result = textureManager.find(filename);
	if(result==textureManager.end()){
		//we have not created this image yet 
		sf::Texture* image = new sf::Texture();
		if(!image->loadFromFile(filename)){
			return NULL;
		}
		textureManager[filename] = image;
		return image;
	}
	else{
		//we have created this image before
		return result->second;
	}
}
Thing* GameManager::createNewThing(const std::string& spriteFilename){
	return new Thing(new sf::Sprite(*GameManager::textureManager.loadTexture(spriteFilename)),spriteFilename);
}

When the line "Thing* thing1 = GameManager::createNewThing("abc.gif");" is run through this code, the program seems to hit a bad state where TextureManager::textureManager is undefined when I enter TextureManager::loadTexture() (but GameManager::textureManager and GameManager::thingList seem to be fine). Specifically, VS2012 says that it has a very, very large size (about 10-20 million) with garbage data inside. And when I run this line of code, it also changes the filename variable and puts garbage data into it, despite the fact that (I think) it shouldn't do that. Eventually, the program hits an exception. My computer even crashed while running it, once, although I'm not sure whether that's due to this program or another program. I'm not sure what the problem could be; I thought that I've covered all my bases in terms of initialization of TextureManager. Could someone help me?

As an aside, if I'm making any egregiously bad coding faux pas, it would be nice if those could be pointed out; I'm still in the process of learning how to put things together into multiple files and with the usage of static/const in code.

[firedraco]

Is the line: Thing* thing1 = GameManager::createNewThing("abc.gif"); inside some other statically initialized data member/class/whatever? If it is, it could be run before your GameManager's static members are initialized.

[WaaaaW]

It's the first line in my main method. To be specific:

#include <SFML/Graphics.hpp>
#include "Thing.h"
#include "GameManager.h"

int main()
{
	Thing* thing1 = GameManager::createNewThing("abc.gif");
	return 0;
}

[firedraco]

I just noticed you are referring the "TextureManager" inside of a method of TextureManager. (Lines 8/9/15) Try using this-> and see what happens.

[WaaaaW]

It doesn't seem to have changed anything after recompiling. Should I perhaps have made my naming scheme different to make it clear which is which at a glance?

[WaaaaW]

After doing a bit of experimentation, I'm thoroughly confused now:

First, I ran the code written in the picture; the watch data is also shown.

http://i.imgur.com/s7LOx.png

What's strange is that even though I thought I initialized all my variables, only one seems to be properly initialized. But what was even weirder was when I ran the program again:

http://i.imgur.com/zXXtE.png

Here, x was defined even though the execution hadn't reached the point where it should have been defined!

And then I ran the program again:

http://i.imgur.com/YkoFq.png

For some reason, x has a size of 97, despite the fact that 1) it shouldn't have even been initialized yet and 2) I haven't actually put anything into it yet. What could be going on?

[cire]

What I suspect is going on is that you compiled in release mode and your code doesn't quit match what's going on in the debugger, due to optimizations. There's also the possibility you're using debug libraries with a release compile or vice versa.

I don't see anything obviously wrong. I'm not sure why "GameManager" is a class.

Dynamic allocation of the sf::Sprite in createNewThing probably isn't necessary.

[WaaaaW]

You're right! I changed the compiler option to compile in debug mode and changed out the libraries for the debug versions, and voila; everything worked as expected. No problems! I never knew that making the choice between debug and release could have made such a difference - well, now I do. Thanks!

I made GameManager a class because I would like for it to ultimately do all the managing of the game materials; ie.

int main(){
	GameManager::initialize();
	while(we aren't to close the game)
		GameManager::update();
		GameManager::draw();
	}
} 

, where GameManager contains all the necessary state variables to do those operations like a list of all things or a list of all loaded images. I'm not sure if this is exactly the way it's meant to be done for a game, but it seems to be satisfactorily simple for now. I also wasn't sure when to do dynamic/static allocation for the data members, but I thought that, if I'm going to be passing around this created Sprite a lot (as a data member in a Thing, as an object stored in a map, etc) I'd better make it a pointer so that I don't waste running time on copying it a lot. I'm not sure if this is the right line of reasoning, though...

[Disch]

Unrelated point:


Rather than make those initialize/update/draw functions static members, make them non-static and create a GameManager instance. That way you don't need global variables for anything:

int main(){
  GameManager thegame;
  thegame.initialize();  // although you could move this to the ctor instead...
  while( ... ){
     thegame.update();
     thegame.draw();
  }
}

[WaaaaW]

Hmm, I see. Thank you; I'll use that instead since it seems to be safer.

[cire]

I made GameManager a class because I would like for it to ultimately do all the managing of the game materials; ie.

If the intention is to implement all methods with free functions, you're probably better off making it a namespace and putting the implementation details in a .cpp file. There would be no reason to publicize the existence of thingList and textureManager as private members of the GameManager class. Of course, making it a full-fledged class is also a reasonable way to go.

I also wasn't sure when to do dynamic/static allocation for the data members, but I thought that, if I'm going to be passing around this created Sprite a lot (as a data member in a Thing, as an object stored in a map, etc) I'd better make it a pointer so that I don't waste running time on copying it a lot. I'm not sure if this is the right line of reasoning, though...

A sprite in SFML is a pretty lightweight construct. Most designs would call for the "thing" it was associated with to own the sprite (ie. it would usually have an sf::Sprite member as a opposed to a pointer to one.) If, from there you wanted to pass around pointers or refeferences to it, you could do that whether it was dynamically allocated or not. It's a good thing to avoid dynamically allocating memory that doesn't need to be dynamically allocated.