Jump to content
  • Advertisement
Sign in to follow this  
Juha Ker

Game Code Review

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

Me and my pal are studying in a university of applied sciences here in Finland and as an extra curricular activity we're programming some games. Now, we've gotten some relatively working stuff done (2D platformer stuff) with C++ and SDL but the problem is that pretty much nobody in our school even uses C++ or have written any sort of game engine code and so we can't really find anyone there to give any useful feedback on the code.

 

I know the code is a mess (could use documentation) and if I used a ton of effort I could probably figure out ways to refactor it in some sensible way. But it would be so wonderful if someone could just skim it through and give us just a couple of pointers how to make everything make more sense.

 

Here's what we've got so far:

https://github.com/Ianuarius/KnightBrawl/tree/master/Sources

 

I'm not necessarily asking anyone here to go through the code (it's around 35 classes), but if someone could point us to a place where we could get some feedback on our code, that would be EXCELLENT.

Edited by Ianuarius

Share this post


Link to post
Share on other sites
Advertisement

I'm not going to reveiew everything, but this is what I saw :

1. Almost every class for some reasion refers to an object of type "Window"?. Why is this? In most of the cases I saw you are actually interested in the SDL_Renderer (which makes sense). But why a Texture, Sprite, Animation(or a Knight) needs to know anything about "Window"?

2. #define INFINITE_LOOP -1          this gives me nothing. I don't know what it means or when it is used.

3. Your sprite class should be named "SpriteSheet" usually "Sprite" means a single image. You "Sprite" describes multiple "sub-images" contained in one texture.

4. Animation inherits ??"Sprite". And I really don't see why. That means that you cannot have animation that has images on more than one texture(ok that is a rare case but still). Sprite* as a member would be better?

5. "Key.h",  what is described by that class?

6 : "class MyClass {.......private? : };"

7. In Effect default co?structor "tmp_input(9999)," what is 9999, is it a special magic number, or it is just by default something bit enough?

 

Over all the code looks well formatted?, but there where several places (identical to 7) where there is just no description.

Share this post


Link to post
Share on other sites

Thank you imoogiBG for the reply.

 

Yeah, I need to start adding descriptions to the whole thing. The two of us mostly know whatever the other person meant and we talk with eachother a lot and even do programming as a pair at times. So, the lack of descriptions hasn't been a problem to us so far, but at the back of my head I know it's a disaster waiting to happen. :D

 

I, in fact, use that 9999 in other spot, too. It's basically just a magic number that is never used unless it is specifically used as a flag of sorts. IT worked so I didn't give it another thought, altho, I wish there was a better way to do it.

 

It seems to me that the infinite loop thing is a similar situation where it's more of a default value that's only used when specifically asked. (Or rather when something else is NOT specifically askked.) So, animations by default play on an infinity loop.

 

Yeah, I gotta start sorting out the other things you mentioned.

 

Thank you very much!

Share this post


Link to post
Share on other sites
I downloaded the program, made a copy, and started browsing the first 10-15 .h and .cpp file, typing comments of random stuff I found.

Then ran diff to get my comments out (in diff format). See attached file.
You probably need to put it next to the sources to make sense of the comment.

In general, looks nice.

Share this post


Link to post
Share on other sites

Thank you, Alberth, for your great comments!

 

I feel kinda bad that you made such an effort to go through the Color class when we pretty much just implemented that from somewhere else. biggrin.png I haven't ever done operator overloading myself, because I haven't yet found a spot where it would've been handy. Yeah, I have no idea what adding together colors would do, either. The same thing with the Input class. We pretty much just added comments in Finnish but it was waaaay back when... since you need an input class right off the bat and we just forgot about it after that. smile.png I actually need to start rewriting an input class that would be more useful, straight forward, and also work with the gamepads. My code for the gamepads is currently so unintuitive that I almost want to just delete it all. Almost. Ha!

 

But yeah, super useful comments. I didn't know about the enum naming conventions! Gotta start using that, too.

 

Thank you, again. smile.png

 

EDIT: The thing about getters and setters is that I don't think we have enough experience in programming to make that call ourselves, yet. We just do it because the teacher at our java class told us to always use them. I don't really like them, either, but I use them because, hey, somebody that probably knows better told me that it's useful. I'm gonna start thinking about it more from now on. :)

Edited by Ianuarius

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.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!