Jump to content
  • Advertisement
Sign in to follow this  
_Sigma

Is something like this ok?

This topic is 4682 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Ever since I learnt about classes, I've loved using them. But in my game, I think I may have gone a little overboard. Is this ok?
DDback->Blt(&Player.Animation_Walk_Up.Sequence[Player.currentFrame].dest_rect,Player.Animation_Walk_Up.Sequence[Player.currentFrame].DDTile, &Player.Animation_Walk_Up.Sequence[Player.currentFrame].src_rect,DDBLT_WAIT | DDBLT_KEYSRC,NULL);

OR

 Player.Animation_Walk_Right.Sequence[Player.currentFrame].dest_rect.left = Player.x;
The DDback->Blt() is DirectX, so it doesn't count. But, the rest is mine. Is that way too many objects? Cheers

Share this post


Link to post
Share on other sites
Advertisement
Whether or not it is too many objects depends on the details of the project.

I think it would be better to write a setting method, with a descriptive name, in the interface of the class to encapsulate all the indirection to other member classes.

Share this post


Link to post
Share on other sites
The problem is that it is a nightmare to read. And there doesn't appear to be any encapsulation of data at all, which would get a lot of people riled up.

It would probably be more generally acceptable to say something like...


Animation *anim = player->getAnimation[ANIM_WALK_RIGHT];
Sequence *seq = anim->getSequence( player->getCurrentFrame() );
seq->setDestRec( /* whatever */ );


Of course, I'm just making type names up and stuff, but this sort of thing is much more readable in my opinion.

Share this post


Link to post
Share on other sites
Quote:
Original post by _Sigma
But in my game, I think I may have gone a little overboard. Is this ok?


It's rarely ok to go overboard with anything. [wink]
But you're the one who have to deal with your code. So you'll have to decide if it's ok.
Looks messy to me though.

But yeah, what happened to encapsulation?

Share this post


Link to post
Share on other sites
Greatly simplified, encapsulation here means that you should hide implementation of specific code (this case: rendering of a player sprite) from the end-user. This helps to concentrate on the task you actually want to do; Just render the sprite, nothing else.

Here's how I do it in my sprite code:

CGameObject Player; // Create our sprite
Player.MoveRel(-1, 0); // Move 1.0 to left in relative coordinates
Player.SetAnimation(ANIM_WALK_LEFT);
Player.Render(); // ... And render him

You see how data is being hidden from the coder? Talk about cool huh :)

Share this post


Link to post
Share on other sites
"Encapsulation" means that access to an objects members is restricted to only those accesses that you deem appropriate.

For example... Let's change one of the lines that you used...
Player.Animation_Walk_Right.Sequence[Player.currentFrame].dest_rect.left = -2849309485.2945;

Is that what you want? Probably not... if you only allow access to that variable through an accessor method you can validate the values, etc.

sequence->getDestRec()->setLeft(-2849309485.2945);

Now, the setLeft() method can do some validation on the value.

Generally speaking, if someone (a potential employer, perhaps) looks at your code and sees you directly accessing any class members like you were doing, they will think less of you as a programmer.

Like someone else said, though, if this is just for you and you don't care what anyone else thinks then it doesn't really matter.

It's a good idea though to have getter/setter methods for all of your data, and if I were you I would try to get in the habit.

Share this post


Link to post
Share on other sites
ALright, I think I follow. So something like this then?

Sprite Player

...

Sprite::Render(Int Direction)
{
case(Direction)
{
case Right:
DDback->Blt(&Player.Animation_Walk_Up.Sequence[Player.currentFrame].dest_rect,Player.Animation_Walk_Up.Sequence[Player.currentFrame].DDTile, &Player.Animation_Walk_Up.Sequence[Player.currentFrame].src_rect,DDBLT_WAIT | DDBLT_KEYSRC,NULL);
break;

[other cases here]
}
}

...

Player.Render(RIGHT);

So something like that then eh?

PS> How do you do the [code=cpp] thing again?

Share this post


Link to post
Share on other sites
There's no need for all that repetition (copy'n'pasting). At least do something like this:
const Sequence &seq = Player.Animation_Walk_Up.Sequence[Player.currentFrame];

DDback->Blt(&seq.dest_rect, seq.DDTile, &seq.src_rect, DDBLT_WAIT | DDBLT_KEYSRC, NULL);

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

Participate in the game development conversation and more when you create an account on GameDev.net!

Sign me up!