sfml framework

The Partridge Family were neither partridges nor a family. Discuss.
xu2201g
Posts: 33
Joined: April 19th, 2017, 12:49 pm
Location: Germany

sfml framework

Post by xu2201g » May 20th, 2017, 10:06 pm

Hi there,

i randomly stumbled upon chilis tutorials and couldnt stop watching them. You re doing a great job there. I have already some knowledge about c++, but i still learned alot in these tutorials and i want to learn much more. Currently iam working on a sfml framework with great help from a book "SFML Game Development by Example" to learn more about game development, like resource management, states, programming patterns, networking etc. and want to share my progress within this thread.

When ive made it through the book the framework should support a little 2d mmorpg like game. At the moment there is a snake game implemented and in the next chapters there ll follow a sidescroller. If u re interested in giving me some feedback here is the github link: https://github.com/xu2201g/framework
I am not that familiar with github yet. If the solution does not work you may make a new project add the files and setup sfml and shlwapi.lib as an additional dependency in the linker input.

Most stuff i completly copied from the sourcecode of the book itself so far, but i tried to use smartpointers to manage resources instead of raw pointers + delete allocated stuff and modified the actual game a bit like chili mentioned in the snek game tutorial.

The game uses an eventmanager, wich handles key and mouse events and loads the bindings from a config file.
It also supports states in which the behaviour of the game is defined, like the intro state, mainmenu state, gamestate etc.

If u ve any suggestions to improve my code or anything else feel free to give me feedback.

---
Ill stick the latest version of the different branches of the framework ive worked on so far on this post:

Snake:
snake.rar
(904.4 KiB) Downloaded 143 times
Spoiler:
Image
Platformer:
platformer.rar
(1.03 MiB) Downloaded 153 times
Spoiler:
Image
Spaceshooter:
spaceshooter.rar
(962.3 KiB) Downloaded 145 times
Spoiler:
Image
Last edited by xu2201g on June 19th, 2017, 7:35 pm, edited 1 time in total.

albinopapa
Posts: 4373
Joined: February 28th, 2013, 3:23 am
Location: Oklahoma, United States

Re: sfml framework

Post by albinopapa » May 21st, 2017, 10:10 am

For me, the one thing I don't really care for is the use of std::pair. Mostly just because of the (first, second) members. Later on in the code it is harder to reason about what they mean. I would suggest making a struct with the pairs of objects instead of using std::pair just for readability.

For example:

Code: Select all

using Events = std::vector<std::pair<EventType, EventInfo>>;
would be

Code: Select all

struct Event
{
     EventType type;
     EventInfo info;
};

using Events = std::vector<Event>;
I know it's extra work, but instead of events[0].first and events[0].second, you'd get readability with events[0].type and events[0].info.

Another thing, though I don't know much about unordered_map, would be to change the 'key' from std::string to an enum. Here's my thinking about it. One, comparing strings requires more processing time than comparing ints. I know unordered_map might use a hash table or binary tree for lookup and is quite fast, but it might help. Two, the more important one in my opinion is, using strings in general can possibly lead to bugs through mistyping the key. I believe when a key isn't found it creates a new entry, instead of telling you it's not there. At least there will be less of a chance of mistyping an enum as the compiler would tell you that the enumeration itself doesn't exist. You could still put the wrong enum and have the same problem as the mistyped string, but I think it would be easier to find. Just my $0.02.

I suppose the beauty in using strings though would be the flexibility of not having types and different unordered_maps for each enum class type.
If you think paging some data from disk into RAM is slow, try paging it into a simian cerebrum over a pair of optical nerves. - gameprogrammingpatterns.com

xu2201g
Posts: 33
Joined: April 19th, 2017, 12:49 pm
Location: Germany

Re: sfml framework

Post by xu2201g » May 21st, 2017, 11:09 am

Hi albinopapa,

thanks for reviewing my code and for the quick response, when ive finished the next chapter ill replace the std::pair stuff with a struct for better readability. In performance there shouldnt be any noticable change from what ive read about it.
The std::unordered_map is a hashtable - pre c++11 there were no standard hashtable implementations, so peeps made their own hashtables and it has a different name to avoid collisions ive read.
Performance wise its probably better to not use strings, but i load those key value pairs from a file and the string fits there good enough for now i think.
The unordered map has a find function which returns an iterator. If it finds the key the iterator points to the element and if not it points to .end() to check if it was found or not and it works so far with strings like:

.

Code: Select all

bool EventManager::AddBinding(std::unique_ptr<Binding> binding)
{
	//there is already a binding with the string key registered
	if (m_bindings.find(binding->m_name) != m_bindings.end())
	{
		return false;
	}

	//add the new binding
	return m_bindings.emplace(binding->m_name, std::move(binding)).second;
}
Its the first time that i use smartpointers and i wonder if it is the right way of using them. The sourcecode compiles and it seems to work.

I try to understand most of the code ive monkey see, monkey do like copied from the book. The code is probably hard to read and i should comment the code more. If u ve more ideas how to improve my style in any way dont hesitate to tell me.

User avatar
chili
Site Admin
Posts: 3948
Joined: December 31st, 2011, 4:53 pm
Location: Japan
Contact:

Re: sfml framework

Post by chili » May 21st, 2017, 2:14 pm

It really depends on how many times per second/frame you're gonna be doing a lookup in the hash table. Hashing a string is certainly slower; the algorithm will generally have to touch all the bytes of the string. But if you're not doing a ton of lookups per frame, it doesn't really matter so just do what is the easier / what make your code more maintainable.
Chili

User avatar
chili
Site Admin
Posts: 3948
Joined: December 31st, 2011, 4:53 pm
Location: Japan
Contact:

Re: sfml framework

Post by chili » May 21st, 2017, 2:17 pm

Also, there is a better way to insert into an unordered map. Look for the overload that returns a pair and use that. Calling find and then emplace is kind of a waste.
Chili

albinopapa
Posts: 4373
Joined: February 28th, 2013, 3:23 am
Location: Oklahoma, United States

Re: sfml framework

Post by albinopapa » May 21st, 2017, 7:21 pm

Didn't make it all the way through first time round, just wanted to add some insight to the unique_ptr.

Code: Select all

	void RegisterState(const StateType& type)
	{
		m_stateFactory[type] = [this] () -> std::unique_ptr<BaseState>
		{
			return std::unique_ptr<T>(new T(this));
		};
	}


// The preferred way to return a unique_ptr would be
	void RegisterState(const StateType& type)
	{
		m_stateFactory[type] = [this] () -> std::unique_ptr<BaseState>
		{
			return std::make_unique<T>( this );
		};
	}

The reason has something to do with exception safety. In this specific case, I don't think it matters, but there might be other instances where it does and using std::make_unique out of habit would avoid those instances.
If you think paging some data from disk into RAM is slow, try paging it into a simian cerebrum over a pair of optical nerves. - gameprogrammingpatterns.com

xu2201g
Posts: 33
Joined: April 19th, 2017, 12:49 pm
Location: Germany

Re: sfml framework

Post by xu2201g » May 22nd, 2017, 9:25 am

Hi chili,
usually i try to use these lookups only to setup stuff when changing the game state, like key bindings, textures etc.
Setting up a texture means to require the texture from the texture manager, which loads the texture from a cfg file, and then get a reference to where the texture lies.

like its done here:

Code: Select all


...
if (!m_pTextureManager->RequireResource(texture)) //setup texture
{
	std::cerr << "! Could not setup the texture: " << texture << std::endl;
	continue;
}

m_texture = texture;
m_sprite.setTexture(*m_pTextureManager->GetResource(m_texture)); 
...
its working quite similar to your
C++ Tutorial Intermediate Recipes (SFML) [Shared Resource Management] tutorial
https://www.youtube.com/watch?v=IdY6pvVY0C0

I guess ill make use of shared_pointers too in the future.

I try to pay attention to limit the map lookups if they re not only used to setup stuff or use enums if i cant avoid them, thanks.

About the find and then emplace into a map, ye its kinda waste to call find and check wether the iterator points to .end(). From what ive read the emplace function iam already using checks if the key is already in the map and returns a pair (itr, bool). The second part of its returning pair holds a bool about the success of the insertion.
Thats what u meant by
Look for the overload that returns a pair and use that
?

So i should remove the find part like that:

Code: Select all

bool EventManager::AddBinding(std::unique_ptr<Binding> binding)
{
   ////there is already a binding with the string key registered
   //if (m_bindings.find(binding->m_name) != m_bindings.end())
   //{
   //   return false;
   //}

   //add the new binding
   return m_bindings.emplace(binding->m_name, std::move(binding)).second;
}
I finished the first part about spritesheet and simple animations so far and implemented a quick and dirty snake game which uses sprites instead of shapes looks like that atm:

Image

and replaced the std::pair stuff with structs for better readability and used std::make_unique.

Thanks for the feedback.
Last edited by xu2201g on May 25th, 2017, 8:18 pm, edited 1 time in total.

xu2201g
Posts: 33
Joined: April 19th, 2017, 12:49 pm
Location: Germany

Re: sfml framework

Post by xu2201g » May 25th, 2017, 8:17 pm

Hi,
i reviewed and tested the stuff i implemented so far and got a bug (probably i did something wrong cause of misunderstanding).

When i close the snake game while its in the game state i get a exception "vector subscript out of range". So i started to debug to find the reason why this is happening.

See the next post for up to date informations.

It starts whithin the spritesheet destructor. (deprecated)

Usually when the game is closed in the gamestate, the game_state destructor is invoked, which invokes the member destructors and so on. Please correct me if iam wrong.

Its working like this:

- closing the window within the game_state
...
~State_Game(); is invoked
...
~Snake(); is invoked
...
~SpriteSheet is invoked which invokes ReleaseSheet

Image

Image

Image

so that u can see m_texture is readable before ReleaseSheet is executed:

Image

The problem here is that m_texture (std::string member of spritesheet) cant be read and iam wondering why. Do i ve no access to members within class functions anymore when the destructor is executed?

The sourcode is stored in github: https://github.com/xu2201g/framework

The spritesheet loads the texture name into m_texture from file together with other informations for animation etc. So the texturemanager(the texturemanager loads a file wich handles the resource loading with this key like:

Code: Select all

boolSpriteSheet::LoadSheet(const std::string& file)
{
	std::ifstream sheet;
	sheet.open(Utils::GetWorkingDirectory() + file);

	if (sheet.is_open())
	{
		ReleaseSheet(); //release current sheet resources

		std::string line;
		while (std::getline(sheet, line))
		{
			if (line[0] == '|') { continue; } //'|' indicates comments or not processed lines like // in c++

			std::stringstream keystream(line);
			std::string type;

			keystream >> type;

			if (type == "Texture")
			{
				if (m_texture != "") //already holding a texture
				{
					std::cerr << "! Duplicate texture entries in: " << file << std::endl;
					continue;
				}

				std::string texture;
				keystream >> texture;

				if (!m_pTextureManager->RequireResource(texture)) //setup texture
				{
					std::cerr << "! Could not setup the texture: " << texture << std::endl;
					continue;
				}

				m_texture = texture;
				m_sprite.setTexture(*m_pTextureManager->GetResource(m_texture));
			}

...
 
Last edited by xu2201g on May 25th, 2017, 11:47 pm, edited 3 times in total.

xu2201g
Posts: 33
Joined: April 19th, 2017, 12:49 pm
Location: Germany

Re: sfml framework

Post by xu2201g » May 25th, 2017, 11:42 pm

Well, i guess i know what fcked me up there.
First of all there are several mistakes in the previews post. While debugging the value of m_texture is not readable cause the line was not executed yet :roll: . And if its executed the right value ll be shown.

Image

So lets see a bit deeper what code is getting executed.

Code: Select all

SpriteSheet::~SpriteSheet()
{
	ReleaseSheet();//<---
}

void SpriteSheet::ReleaseSheet()
{
	std::string texture = m_texture;

	m_pTextureManager->ReleaseResource(texture); //<---
	m_pAnimationCurrent = nullptr;

	while (m_animations.begin() != m_animations.end())
	{
		m_animations.erase(m_animations.begin());
	}
}

bool ReleaseResource(const std::string& id)
{
        auto resource = Find(id);//<---
	//resource not found
	if (!resource)
	{
		return false;
	}

	//decrement required counter
	--resource->second;

	//if the required counter is <= 0 unload the resource
	if (!resource->second)
	{
			Unload(id);
	}
	return true;
}

std::pair<std::unique_ptr<T>, unsigned int>* Find(const std::string& id)
{
	auto itr = m_resources.find(id);//<---
	return itr != m_resources.end() ? &itr->second : nullptr;
}

When i hit the auto itr = m_resources.find(id); line with the debugger i got:

Image

Looked fine to me, well there is no resource(in this case a texture) stored, but the find function should just not find it and return .end() to the iterator.
This line throws the "vector subscript out of range" exception. The problem is that the resource/texturemanager was already destroyed, but std::unordered_map members dont disappear and look relatively fine, even when they re "destroyed".

So i checked the destroy order and the texturemanager is getting destroyed almost at the first.
Usually i expect the members destroyed or holding some garbage values.
For some reason the unordered maps re not going to look destroyed, just empty. I added an int member to the texturemanager/resourcemanager just to see wether this member is destroyed or not and it is really destroyed and disappears.

The problem i wasnt aware of is the order members of a class re destroyed. I thought i put the texturemanager at last so it gets destroyed after the other members and that was a false assumption.
Destructors for nonstatic member objects are called in the reverse order in which they appear in the class declaration.
source: https://msdn.microsoft.com/en-us/library/6t4fe76c.aspx

Well swaping the members and everything seems to be fine now... fml.
Its maybe a bit too detailed, but i want to share that with u guys, so u dont run into similar issues.

User avatar
chili
Site Admin
Posts: 3948
Joined: December 31st, 2011, 4:53 pm
Location: Japan
Contact:

Re: sfml framework

Post by chili » May 26th, 2017, 1:21 am

Yeah, the constructor/destructor have a very logical (and I would say also elegant) symmetry. The destructor is the reverse of the constructor--a true reverse--and so it does everything in reverse order. I will talk about this in a future video (probably going to be I6).

Good on your for using the debugger to find your problem, these are the kind of essential skills that get developed when you actually take the time to make some shit, instead of just blazing through a book / tutorial series.
Chili

Post Reply