Noob learns to code in 3 months

The Partridge Family were neither partridges nor a family. Discuss.
User avatar
Yumtard
Posts: 575
Joined: January 19th, 2017, 10:28 pm
Location: Idiot from northern Europe

Re: Noob learns to code in 3 months

Post by Yumtard » June 28th, 2017, 3:07 pm

I'm having some issues with hw 5.

My first thought was to make new memefield inside game and change the constructor of memefield to take width and height. But that doesn't work because width and height must be static constexpr.

ceofil wrote:Congrats man!!! I'm so happy for you :D :D :D :D :D
Zedtho wrote:Holy shit! That's freaking awesome! Congratz!
Thanks to both of you :D

ceofil
Posts: 39
Joined: April 13th, 2017, 8:35 pm

Re: Noob learns to code in 3 months

Post by ceofil » June 28th, 2017, 6:41 pm

Yumtard wrote:My first thought was to make new memefield inside game and change the constructor of memefield to take width and height. But that doesn't work because width and height must be static constexpr.
It doesn't have to be static constexpr when dynamically allocating.

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

Re: Noob learns to code in 3 months

Post by albinopapa » June 28th, 2017, 10:13 pm

Yeah man, congrats.
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: Noob learns to code in 3 months

Post by chili » June 29th, 2017, 1:11 am

First off, conglaturations!

Second, great idea with the T5 homework. But why do you think that wid/height must be static constexpr. Just because we've done something one way for our whole life doesn't mean that it is the only alternative. ;)
Chili

User avatar
Yumtard
Posts: 575
Joined: January 19th, 2017, 10:28 pm
Location: Idiot from northern Europe

Re: Noob learns to code in 3 months

Post by Yumtard » June 29th, 2017, 12:01 pm

Thanks:)

I had some stuff come up so still haven't put much time into hw 5. Will try to get some work in today.

What I meant was I can't do it the normal way like
Having this in the h file
Tile field[width * height];

and have width and heigh be normal ints and init them in the constructor.

Also tried to make new tile field in the constructor but that didn't work either.

Will look more into it a bit later

User avatar
Yumtard
Posts: 575
Joined: January 19th, 2017, 10:28 pm
Location: Idiot from northern Europe

Re: Noob learns to code in 3 months

Post by Yumtard » June 29th, 2017, 12:32 pm

clicked buttons until it worked :O

Think I'll need to rewatch some stuff though. This is still confusing.
Maybe I should also upload code so we can see if I've done it in a reasonable way

User avatar
Yumtard
Posts: 575
Joined: January 19th, 2017, 10:28 pm
Location: Idiot from northern Europe

Re: Noob learns to code in 3 months

Post by Yumtard » June 29th, 2017, 1:19 pm

in MemeField

constructor:
field = new Tile[width*height];

in the h file:
Tile* field;

in game:

Code: Select all

const SelectionMenu::Size s = menu.ProcessMouse( e );
			switch( s )
			{
			case SelectionMenu::Size::Small:
				field = new MemeField(gfx.GetRect().GetCenter(), 10, 9, 9);
				state = State::Memesweeper;
				break;
			case SelectionMenu::Size::Medium:
				field = new MemeField(gfx.GetRect().GetCenter(), 40, 16, 16);
				state = State::Memesweeper;
				break;
			case SelectionMenu::Size::Large:
				field = new MemeField(gfx.GetRect().GetCenter(), 99, 30, 16);
				state = State::Memesweeper;
				break;

else
{
if (e.GetType() == Mouse::Event::Type::LPress)
{
delete field;
state = State::SelectionMenu;
}
}



I'm not deleting the Tile field inside of MemeField. Do I need to do it?
If I'm deleting field inside of game then doesn't the tile field also get deleted anyways?

https://github.com/Yumtard/dynamic-memes


btw: if I close down the game while the sound is playing after I fail, I get an assertion fail. Why is that? It's this one

Code: Select all

SoundSystem::Channel::~Channel()
{
	assert( !pSound );
	if( pSource )
	{
		pSource->DestroyVoice();
		pSource = nullptr;
	}
}

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

Re: Noob learns to code in 3 months

Post by albinopapa » June 29th, 2017, 3:31 pm

I thought about using the ctor to change the size like you did, but I had my reason not to in this case.

Wanted to point out something to you:

Code: Select all

MemeField::MemeField( const Vei2& center,int nMemes, int width_in, int height_in)
	:
	topLeft( center - Vei2( width * SpriteCodex::tileSize,height * SpriteCodex::tileSize ) / 2 ),
	height(height_in),
	width(width_in)
{
	//...
}
Do you know why this works? The reason I ask is because you use members width and height to initialize topleft, seemingly before initializing them. I know why this works, just wanted to get your take on it.
Spoiler:
It's because width and height are declared before topleft in header file. Just be careful of this if you plan on doing things this way. I might use the width_in and height_in params to initialize topleft just to be safe if I ever forget about declaration ordering.


Yes, you should be deleting the Tile *field each time you delete MemeField *Game::field. The easiest way to do this is by adding a destructor to MemeField ( check my repo for example ) and delete[] the field pointer in there. This way when you delete Game::field, the ~MemeField destructor is called and then Tile *field will be deleted also, with no extra function calls on your part.

I didn't think about making Game::field a pointer, honestly, I almost refuse nowadays to make single instance pointers with the exception of using polymorphism. There is a bug in my solution now that I'm reading this and have looked over yours. For some reason, the game wouldn't go back to the menu when game was over, so I didn't think about it. Anyway, if that were the case, since I don't use the constructor, I would be leaking memory since I don't free the MemeField::field pointer between games. Same goes for your current solution. If you aren't freeing the dynamically allocated Tile array between games, you too are leaking memory.

When chili goes over std::unique_ptr/vector or RAII, you'll learn destructors will be rare and handling resources will be somewhat simplified.

Good solution, just need to fix the mem leak and you've got it. The biggest reason I didn't use the constructor is because I read you were going to go that route, and I didn't want to do the same thing.
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
Yumtard
Posts: 575
Joined: January 19th, 2017, 10:28 pm
Location: Idiot from northern Europe

Re: Noob learns to code in 3 months

Post by Yumtard » June 29th, 2017, 3:42 pm

albinopapa wrote:I thought about using the ctor to change the size like you did, but I had my reason not to in this case.

Wanted to point out something to you:

Code: Select all

MemeField::MemeField( const Vei2& center,int nMemes, int width_in, int height_in)
	:
	topLeft( center - Vei2( width * SpriteCodex::tileSize,height * SpriteCodex::tileSize ) / 2 ),
	height(height_in),
	width(width_in)
{
	//...
}
Do you know why this works? The reason I ask is because you use members width and height to initialize topleft, seemingly before initializing them. I know why this works, just wanted to get your take on it.
Spoiler:
It's because width and height are declared before topleft in header file. Just be careful of this if you plan on doing things this way. I might use the width_in and height_in params to initialize topleft just to be safe if I ever forget about declaration ordering.

Yeah I was aware of that, but good point about switching their positions in the initializer list anyways to make it less confusing and safer!

albinopapa wrote: Yes, you should be deleting the Tile *field each time you delete MemeField *Game::field. The easiest way to do this is by adding a destructor to MemeField ( check my repo for example ) and delete[] the field pointer in there. This way when you delete Game::field, the ~MemeField destructor is called and then Tile *field will be deleted also, with no extra function calls on your part.
Yeah I was unsure where in the file to delete the field but added destructor just like you after looking at your code :)

btw I found this interesting:
if (field)

Apologize to Chili if he's gone over this before and I've forgotten it.
So I assume field is true if it's not null?

albinopapa wrote: I didn't think about making Game::field a pointer, honestly, I almost refuse nowadays to make single instance pointers with the exception of using polymorphism. There is a bug in my solution now that I'm reading this and have looked over yours. For some reason, the game wouldn't go back to the menu when game was over, so I didn't think about it. Anyway, if that were the case, since I don't use the constructor, I would be leaking memory since I don't free the MemeField::field pointer between games. Same goes for your current solution. If you aren't freeing the dynamically allocated Tile array between games, you too are leaking memory.

When chili goes over std::unique_ptr/vector or RAII, you'll learn destructors will be rare and handling resources will be somewhat simplified.

Good solution, just need to fix the mem leak and you've got it. The biggest reason I didn't use the constructor is because I read you were going to go that route, and I didn't want to do the same thing.
I'm confused, why do i have memory leak when restarting the game?

Shouldn't this part here take care of it?

Code: Select all

if (e.GetType() == Mouse::Event::Type::LPress)
				{
					delete field;
					field = nullptr;
					state = State::SelectionMenu;
				}
Edit: or did you mean I had a leak because before looking at your code I wasn't deleting the field inside MemeField?

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

Re: Noob learns to code in 3 months

Post by albinopapa » June 29th, 2017, 4:59 pm

if (field)

Apologize to Chili if he's gone over this before and I've forgotten it.
So I assume field is true if it's not null?
Yes, if field is null and null is 0 and 0 is same as false then that section gets skipped. If field is not null, condition is true.
I'm confused, why do i have memory leak when restarting the game?

Shouldn't this part here take care of it?

Code: Select all

            if (e.GetType() == Mouse::Event::Type::LPress)
            {
               delete field;
               field = nullptr;
               state = State::SelectionMenu;
            }
Edit: or did you mean I had a leak because before looking at your code I wasn't deleting the field inside MemeField?
albinopapa wrote:If you aren't freeing the dynamically allocated Tile array between games, you too are leaking memory.
Yeah, that's why I specified Tile *field. The field in game is MemeField *field, so I didn't think there would be confusion, sorry.
Yes, I was referring to the fact that simply deleting the MemeField *field in Game doesn't delete[] the Tile *field in MemeField unless you either use the destructor or call some MemeField::ReleaseField() function before you delete MemeField *Game::field in Game.
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