Jump to content
  • Advertisement
Sign in to follow this  
stitchs_login

Unity Pong Project near completion; looking for Critique.

This topic is 1993 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

Hello community,

I have completed a small Pong project, I am quite happy with it and am looking towards the community now that I am at a point to share it. At some points it got me down, as this was my first outing with SFML and there are some aspects that I definitely need to understand better (such as the ordering of event queues and key-presses), but I am happy that i persevered.

I wasn't sure where to put this, it is kind of an announcement, but I am also looking for a code critique and some feedback. Basically, I have gone back to basics and am trying to code a game as quickly as possible, with as few mistakes as possible, but they definitely exists. Currently I am aware that the Paddle and Ball classes share some methods, and this will lead me to developing an inheritance structure, this is one of the known issues and an example of what I am aware of in my code.

I am trying to find as many shortfalls and understand why some concepts shine, through the experience of not using them.

A few things I am aware of:

  • Hard-coded values,
  • Movement that jutters at times.
  • Over-diluted with methods that could be combined.
  • Unclear comments in some places.
  • A few methods that are not used or have no body.


    I appreciate any feedback on my code and the application itself, and look forward to your comments. Below you will find two rar files attached, one with the exe and one with the Header/CPP files.

    Regards,

    Stitchs. Edited by stitchs

Share this post


Link to post
Share on other sites
Advertisement
Hey Stitchs,

The executable complains about MSVCP100.dll not being present, you might as well want to include that.
Furtermore, I took a quick look at your code, and it seems okay to me. I don't know how much experience you have in C++, but I think this program is written with a good sence of object orientation. One thing though, why do you save the score in the bat, and not in the arena?

Aart

Share this post


Link to post
Share on other sites
One problem I see is that, probably because of hard coded values, what's happening is that no matter what angle the ball hits the player controlled paddle from, the ball always goes towards the bottom of the screen. This can be easily fixed by changing some values however, so it's not that big of a deal :).

Also, before you continue reading, realize I also finished Pong with SFML in C++ only about a month ago.

Another problem in your code is over-commenting. Holy crap, you're probably getting half of what you should be getting done finished because you're commenting obvious things. And if you're having to write comments for obvious things, it's bad code. An example would be this code in your Paddle.cpp:

void Paddle::SetPositionY(float yPosIn, float elapsedTime)
{
if(position_.y < (yPosIn - 10))
{
// prevents paddle going off screen
if(getArea().Bottom < 800)
{
position_.y += speed_ * elapsedTime;
}
}
else if(position_.y > (yPosIn + 10))
{
if(getArea().Top > 0)
{
position_.y -= speed_ * elapsedTime;
}
}
}


This is hard to understand, even though you have that comment.

It would have been better to have some const int's like so:

enum Screen
{
Left = 0,
Right = 800,
Top = 0,
Bottom = 600
};


Now look at your code:

void Paddle::SetPositionY(float yPosIn, float elapsedTime)
{
if(position_.y < (yPosIn - 10))
{
// prevents paddle going off screen
if(getArea().Bottom < Screen.Right)
{
position_.y += speed_ * elapsedTime;
}
}
else if(position_.y > (yPosIn + 10))
{
if(getArea().Top > Screen.Left)
{
position_.y -= speed_ * elapsedTime;
}
}
}


Not to mention all the other variables with other names. If you corrected those, then it'd be way, way, way easier to understand. Also, what's with the _ after the name of everything? Edited by superman3275

Share this post


Link to post
Share on other sites
Instead of using if-else chain to manage states, you could have used a nice polymorphism.
[source lang="cpp"]while(running)
{
currentState->update();
currentState->draw();
//change what currentState points to when necessary
}[/source] Edited by lride

Share this post


Link to post
Share on other sites
Thank you all for your responses, I am not currently at my home computer and will make a fuller reply tomorrow, when I have had a chance to compare the notes with my code. Two things I picked up on from Superman and rip-off; the trailing '_' after variables and using 'k' in Enumerated types is something I have been following from the Google Style guide on C++. I wanted to find a way of making my code consistent. If it seems wrong, then I will look into changing that for my next project.

I only pick this out because it is the one I can answer off the top of my head.

Thanks again and I will reply in full tomorrow,

Stitchs.

Share this post


Link to post
Share on other sites

Two things I picked up on from Superman and rip-off; the trailing '_' after variables and using 'k' in Enumerated types is something I have been following from the Google Style guide on C++. I wanted to find a way of making my code consistent. If it seems wrong, then I will look into changing that for my next project.
[/quote]
These are stylistic things, so they aren't "wrong" necessarily. I use the trailing underscores myself in some projects. The "k" prefix is one I haven't seen. But if you think it makes sense for you, that is OK.

Given that C++ places the enum names in the enclosing scope, I prefer to prefix the enum names with some "tag" that groups them, e.g.

enum Direction {
DirectionNone,
DirectionUp,
DirectionDown
// ...
};

// Later
Direction direction = DirectionUp;

Some might find duplicating the name of the enumeration like this distasteful, and I can't say they are wrong.

Some people prefer to place global enums inside a namespace instead:

namespace Direction {
enum Type {
None,
Up,
Down
// ...
};
}

// Later
Direction::Type direction = Direction::Up;

However IMO now your declaration syntax starts getting quite funky.

There isn't a definitive answer, but it is good to think about these things and not just blindly applying them. Edited by rip-off

Share this post


Link to post
Share on other sites

Some people prefer to place global enums inside a namespace instead:

namespace Direction {
enum Type {
None,
Up,
Down
// ...
};
}


With C++11
you can just use
[source lang="cpp"]enum class Direction: unsigned char //You can even specify underlying data type
{
None,
Up,
Down
};
[/source]

Note this doesn't allow implicit type conversion
[source lang="cpp"]int direction=Direction::None;//Error
Direction direction=Direction::None//Ok[/source] Edited by lride

Share this post


Link to post
Share on other sites
I know this reply comes late, I do hope that it is still a valid to post in topic.

I am currently changing the code to match the suggestions made here. I have one question: would a method named 'OnPaddleCollision' be of type Bool or type Void, I can't differentiate between the two. I would of gone with bool, but what I have written in the method does not need to return true or false for the arena to reference, which makes me want to use Void as its' type.

Or, to go against my usual thought train, would I: check for a collision with the paddle/ball in the arena and, if one occurs, call the void Ball::OnPaddleCollision(). Saying it like this seems to make slightly more sense, I guess I just need some outside input to help concrete this in my head.

Thanks and I look forward to your replies,

Regards,

Stitchs.

Share this post


Link to post
Share on other sites

I know this reply comes late, I do hope that it is still a valid to post in topic.
[/quote]
It is perfectly fine. If the thread had started to be a few months old, maybe. There is no hard rule, just common sense AFAIK. Additional leeway would probably be given in this instance as this thread is very specific to your project, as opposed to another member bumping this to ask a follow up question.


I am currently changing the code to match the suggestions made here.
[/quote]
Great! When you're happy with it, you can post it here again for further feedback.


would a method named 'OnPaddleCollision' be of type Bool or type Void
[/quote]
I don't believe there is any sensible meaning to the return value of OnPaddleCollision, thus I would use the "void" version. I'd recommend against including return types when the result is irrelevant to the caller.


would I: check for a collision with the paddle/ball in the arena and, if one occurs, call the void Ball::OnPaddleCollision()
[/quote]
Yes. Edited by rip-off

Share this post


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

  • Advertisement
  • Advertisement
  • Popular Tags

  • Popular Now

  • Advertisement
  • Similar Content

    • By Hive Entertainment
      I have already chosen my favorite from the concept artist which one do you like better 1 2 or 3?
       
      Also still looking for a writer and a 3d artist message if you are interested!

    • By Armaan Gupta
      Hey There,
      I am a developer and Im working on a blockchain based infinite runner type game. Right now, I am working on releasing the beta version with a couple other game developers, but would love to expand the team and have other talented and bright people contributing. The game portion of the project isnt very complicated, and wouldnt require anyone to pull thier hair out for it.
      If you are interested in joining a project, interested in the idea, or would like some more information, please don't hesitate to ask either by commenting, discord (username: Guppy#7625), or by email (armaangupta01@gmail.com).
      Thank you!
    • By Joydeep Mani
      I am trying to build a particle system for unity based on "Destiny particle architecture " released in Siggraph.
      I encounter a problem in trying to understand how they iterated this:

      Converted to spline function and the result is

      i wanted to know how did they converted the function in the editor to represent the graph ??
       
    • By Xerath Dragons
      This is my first experiment use for create my original character little cute dragon chibi use zbrush and blender and use unity3d assest free for enviroment scene you have feedback?
       


    • By Aryndon
      Project Redemption is an semi-fantasy RPG with a linear story and an elaborate combat system.
      We are building in Unity and are currently looking animators and artists.
      What we are looking for
      -Someone who is okay with split revenue/profits when finished
      -Collaborate with others in the team. Do you have an idea/thought on what should be included? Tell us!
      -Someone who wants to work with people that are passionate about this project
      If you are interested. Please message me and I will get back to you as soon as possible! Or add me on Discord AJ#6664
    • By Aggrojag
      Hello!
      I'm working on a game that is a narrative driven dark comedy with some small aspects of platforming and puzzle solving. The project is rather small as well. It touches on topics such as suicide, mental illness, family, corruption, free-will, and redemption.
      This project is exercise in polish, this means some experimentation will be needed along with some reworking of assets as they're provided.
      This will be a revshare model.
      First, I'm looking for a 2D sprite artist, not pixelated, that can compliment the style of the attached images, and be similar to the temporary character.
      We are looking to bring on a SFX designer at this time. Full list of required SFX will be available upon request, as well as a build with all elements related to sound implemented in some form (many SFXs pulled from the web for now). Will likely require some field recording, and some pretty strange SFX for when things get weird. I imagine a good portion of these will be quite fun to create.
      Lastly, I'm looking for a male voice actor, English should be your primary language. There will be at minimum two characters that will need to be brought to life through vocals. The first voice is similar to Marvin from Hitchhiker's Guide to the Galaxy. A reference for the second voice would be a mix of Ren (Ren & Stimpy), and Android 21 (DragonBallFighterZ). Due to formatting, I'm not including YouTube links in the post, sorry.
       
      WIP Scene with our custom shaders attached (platforms are lazily placed, as this was an asset test):

      A scene with dynamic lighting and temp character:

       
      Unshaded asset:

      If you made it to the bottom, thank you, and I look forward to hearing from you.
    • By SickTwistGames
      Ok, firstly, Hi.
       
      This is my first post on this forum. I am an Indie Dev making my first game so bear with me when I say dumb stuff, I'm on a huge learning curve.
       
      My first question is about inventory systems for unity. I am trying to make a survival type game with crafting. I have purchased Inventory manager pro by devdog from the unity asset store and it seems like a pretty powerful assett but for an intermediate coder its a little tough to use.  I'm beginning to wonder if it was the right purchase.
      So my question is.... does anyone have any experience of inventory plugins / systems for unity and can anyone reccomend a system to me?
      It needs to have the following: Loot system, crafting system, character sheet, blueprint system,  character stats system. Ideally with as little coding as possible.
       
      Thanks
    • By ethancodes
      I've got a bug with my brick breaker style game. The bricks move down one line at a time ever 1.5 seconds. What appears to be happening is occasionally the ball will be just about to hit the brick when the brick moves down a line, and now the ball is behind it. I'm not sure how to fix this. I have two ideas but I'm not sure of implementation. 1 solution would be to check where they were and where they are going to be before rendering the frame. Then if they crossed paths, then register the brick as hit. Solution 2 would be change how the bricks move. I could maybe slide them down line by line, instead of a jump down. I'm not sure of this will fix the issue or not. Any ideas?
    • By Pixeye
      I wrote an extension for unity inspector that allows to group/fold variables. 
      Available on github  , cheers!

       
    • By rakshit Rao
      I'M interested in programming tools (For animation, UI, etc). Can anyone suggest me the resources where I can start learning or which technologies I need achive it.
       
      Thanks,
      Rakshit
  • Forum Statistics

    • Total Topics
      631070
    • Total Posts
      2997746
×

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!