Climbing out of Pointer Hell

The Partridge Family were neither partridges nor a family. Discuss.
albinopapa
Posts: 4373
Joined: February 28th, 2013, 3:23 am
Location: Oklahoma, United States

Re: Climbing out of Pointer Hell

Post by albinopapa » July 26th, 2017, 10:39 pm

Oh, btw, instead of que[que.size()-1], just do que.back() which will return the last element.
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

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

Re: Climbing out of Pointer Hell

Post by chili » July 27th, 2017, 1:11 am

Great thread here. Smart pointers generally make things easier, cleaner, and safer, but if you don't get a proper introduction to them and start using them on shaky foundations, things can get ugly. Glad you worked through it though. This info is good for me to see how people mess up with smart pointers. I will take it into consideration when I teach that topic.
GracefulComet wrote: i hate that vector.size() counts from 1 and everything else counts from 0. very easy to forget, has happened before too.
This seems to be a bit of a misconception on your part. I know of no container that 'counts' from 0, in the sense that you are representing the quantity of elements in the container. In terms of quantity, 0 means 'no things'. You are mixing up indexing (the assignment of identifying numbers to things) with counting (the quantifying of things). In terms of indexing, 0 represents the 1st thing.
Chili

GracefulComet
Posts: 12
Joined: July 16th, 2017, 9:49 pm

Re: Climbing out of Pointer Hell

Post by GracefulComet » July 28th, 2017, 12:58 pm

okay i figured it out...

Sprite owns a Heap allocated MSGreceiver that a Copy Assignment is deleteing. but that reference is handed out as a pointer to something else that dereference.

simply replacing a heap allocated MSGreciever for a Stack allocated one fixed it.

and now no more heap Corruption. and no more nullptr dereference.

ill commit the latest working version. and now try replacing most of these pointers with smart ones now.

pretty happy that i finally got it. but a lot more work needs to be done.

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

Re: Climbing out of Pointer Hell

Post by albinopapa » July 28th, 2017, 4:59 pm

Well, I see why the old version you posted here works then, Sprite had a MSGreciever object instead of a MSGreciever *.
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

GracefulComet
Posts: 12
Joined: July 16th, 2017, 9:49 pm

Re: Climbing out of Pointer Hell

Post by GracefulComet » July 29th, 2017, 2:36 pm

Damn it. i hate my dyslexia. i meant the vice versa of what i said. owned a MSGreciever instead of a MSGreciever*. i mis-match the heap and the stack all the time. it will happen again and it will confuse me and everyone as well.
im sorry.

i will just call it dynamic and non dynamic to keep my intentions clear.

its bad the other way because Sprite gives out a MSGreciever* via getlistener function.
during Sprites Copy construction everything is deleted and reallocated at a new address. so now their is a dead/dangling ptr to non dynamic memory given out to another obj.

if i make it a MSGreciever* instead from the start and allocate it dynamically it works perfectly. for now im keeping it a shallow copy.
because if i try and think ahead. if an object later receives an assignment operation and it does a deep copy, all of the old ptr's that i have in my GOfactory vectors would then point to garbage.

im still not done with memory leaks yet. but when i opened this thread, the game was hemorrhaging memory worse that a severed artery lol. so its an improvement at the least


i have alot i intend to work on this but i have family over so i probably wont get any shit done this weekend.

@albinopapa
in the version you have, in Game.cpp, in Game::Game() constructor at the end of the function you should see 2

fact.addplayer(...);
but one of them is commented out. if you uncomment the second one and run does it crash?
it should to my knowledge

if we change the following

changing MSGreciever to MSGreciever*,

adding a

m_messenger = new MSGreciever(ID(id,OBJTYPE::Sprite));

into the constructor with 4 arguments

and removing the extra "&" from inside of the getlistener() function it will run with both objects animating and be moveable with the arrow keys.


technically now the MSGreciever* never gets deleted . so that leaks out... and i think now i can utilize smart pointers to stop that from leaking too.

i will try to work on the code some more. but i have family staying over and will probably not get to do anything i want for a while.

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

Re: Climbing out of Pointer Hell

Post by albinopapa » July 30th, 2017, 6:17 am

Ok, a couple of different issues. One you spotted with the MSGreciever object in Sprite. Since you create a temp copy it over to the array of Gfx objects, the address isn't the same when the function ends.

Another is the GOfactory::addPlayer function in the addBehaviour line. You create a shared_ptr<Parent> instead of shared_ptr<Child>.

I was able to convert all the shared_ptrs to unique_ptrs and got it to run.

The thing with unique_ptr though, is you can't have multiple copies, you have to move the instances around instead of copy them.

I attached a ZIP file of just the .h/.cpp files ( all of them ) so you can see the changes. Some aren't related to the problem though, just cosmetic sorry.
Attachments
Graceful-Engine-master.zip
(19.72 KiB) Downloaded 114 times
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

GracefulComet
Posts: 12
Joined: July 16th, 2017, 9:49 pm

Re: Climbing out of Pointer Hell

Post by GracefulComet » July 31st, 2017, 1:29 pm

So, i love the how clean you made everything look with your cosmetic changes. did you run the code through a linter? i do that sometimes when i make it too crazy like i did.

your code shows how big the gap is between your skill and mine is lol. i need lots more practice.

i learned a few things from reading through it. StringStream looks interesting, useing it removed a bunch of straight c code that i found on the internet to fix a few conversion problems i ran into. i want to know more about that so i will google it some more later.

running valgrind( a memory analyzer/sanitizer shows 0 memory issues minus some shit i did at the beginning. huge improvement compared to before


but im confused at some unexpected behavior. AnimationMSG does exactly what it suppose to, but the PhysicsMSG does not and im not sure why.

The address's of the void* Variables are correct in the start and finish of the PhysicsMSG. the math is being done. but for what ever reason it does not appear that the vector movespeed(the contents of the Variables ptr we are modifying) does not update after PhysicsMSG finishes.

i ran into this myself after you showed me the <parent> was wrong and <child> was correct back in your first few posts in this thread but i just assumed i fucked up somewhere at the time.

but seeing your code in action and that it does the same thing in a shared_ptr or a unique_ptr.


the solution that my brain is Picking at, would to have raw msg* 's for the messages themselves because we know when the message is complete and are able to call delete at the correct time. but continue to use unique_ptr's for the MSGreciever (because it is less memory and time expensive? ... idk just regurgitating what i read online)

EDIT. nvm msg* does not fix it... i dont know what particular semantic is causing this then.

EDIT EDIT:: FOUND IT. PeakatMessages twas the problem. i had experimented with useing the bottom of the vector instead of the top. it never got updated back tho.

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

Re: Climbing out of Pointer Hell

Post by albinopapa » July 31st, 2017, 6:38 pm

Hehe, forgot about changing the atoi to stringstream stuff.

A suggestion for the future. Write a file parser and make your files the same format. The two you have are in the same format, so you could load the files just as easily as you save them...

Code: Select all

void Sprite::LoadFromFile( std::string FileName, SDL_Renderer *Render )
{
	std::ifstream ifs( FileName );
	if( !ifs.is_open() )
	{
		std::cout << "file " << FileName << " not found " << std::endl;
		return;
	}

	std::string tempTexName;
	std::string tempTexID;
	int tempW = 0;
	int tempH = 0;
	int tempNcol = 0;
	int tempNrow = 0;
	int tempCTile = 0;
	float tempX = 0.f;
	float tempY = 0.f;

	ifs >>
		tempTexName >>
		tempTexID >>
		tempW >>
		tempH >>
		tempNcol >>
		tempNrow >>
		tempCTile >>
		tempX >>
		tempY;

	ifs.close();

	TheTextureManager::Instance()->LoadWErrorChecking( tempTexName, tempTexID, Render );

	m_textureID = tempTexID;
	m_tiles = TileMap( tempNcol, tempNrow, tempH, tempW );
	m_position = { tempX, tempY };

	m_movespeed = Vec2DF( 0, 0 );


	m_tiles.setCurTile( tempCTile );
	m_animOffset = 0;
	m_tiles.setNumAnimFrames( 0 );
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

Post Reply