TIGSource Forums

Developer => Technical => Topic started by: hagel on October 26, 2009, 09:41:25 AM



Title: Engine help (code inside)
Post by: hagel on October 26, 2009, 09:41:25 AM
Good evening gentlemen. :gentleman:
I am currently working on a platformer engine. It is written in C++ with the help of the Allegro library. It was supposed to have slopes, both 45° and 23°.
I was wondering if you could look over my code. Specifically the Character::Move code.
The main problem is how the character moves. Both generally and over slopes.

DOWNLOAD .ZIP (http://www.mediafire.com/?yiizzhkedma)

You can run the "game" by executing the .exe in the main directory. Or you can compile the code yourself. I've included a Code::Blocks project file for those who use it.

I am a beginner and this is some seriously messed up code.
Help/redesign is much appreciated.

Thank you. :gentleman:

EDIT: Please do NOT tell me to go to jnrdev.72dpiarmy.com.


Title: Re: Engine help (code inside)
Post by: Aquin on October 26, 2009, 10:01:38 AM
I haven't even looked at your code.  Relax, I'm still gonna help you out.

How exactly are you handling the hit detection for your slopes?  Are you using some hacky please-act-right-on-these-tiles?  Or are you doing line segment separation?  If you aren't using line segment separation, then I highly suggest it (and I'll tell you how to do it.)

As for motion, are you looking for static (ie Megaman-type) motion or a motion based on acceleration/velocity?  Given a platformer, I assume the former. 


Title: Re: Engine help (code inside)
Post by: hagel on October 26, 2009, 10:17:44 AM
How exactly are you handling the hit detection for your slopes?

It's basically a hitmask. I first check what tile you're on. Then I check if you're too deep down in the tile. It returns 0 if you're above the slope and returns the slopenumber if you're below it. That way I check if it's 0, else you're on the slope.


As for motion, are you looking for static (ie Megaman-type) motion or a motion based on acceleration/velocity?
The latter actually. In this game you are supposed to level up and with every level the speed increase. I think it will be weird if you at a press of a button immediately bolt away at the current level speed.

Thanks for the answer man.


Title: Re: Engine help (code inside)
Post by: Average Software on October 26, 2009, 10:42:27 AM
I was just browsing your code, and noticed this:

Code:
Level::Level()
{
w,h=32;
};

w,h=32 doesn't do what you (presumably) think it does.  It sets h to 32, but does nothing to w.  You probably want, w = h = 32.

Minor nit, but those semicolons after the closing braces of your functions are technically illegal (statement at global scope).  Very few compilers care, but some do and they won't take that code.


Title: Re: Engine help (code inside)
Post by: hagel on October 26, 2009, 10:48:02 AM
I was just browsing your code, and noticed this:

Code:
Level::Level()
{
w,h=32;
};

w,h=32 doesn't do what you (presumably) think it does.  It sets h to 32, but does nothing to w.  You probably want, w = h = 32.

Minor nit, but those semicolons after the closing braces of your functions are technically illegal (statement at global scope).  Very few compilers care, but some do and they won't take that code.

'w' and 'h' is never used so it doesn't matter, but thank you.

About the semicolons, I read some tutorial that it was a must to have semicolons after functions in classes. Originally I had none but added them after I read it.
Proves that you can't believe everything you read on the Internet. Not that I didn't know that before.

Thank you :gentleman:


Title: Re: Engine help (code inside)
Post by: Average Software on October 26, 2009, 11:05:27 AM
I was just browsing your code, and noticed this:

Code:
Level::Level()
{
w,h=32;
};

w,h=32 doesn't do what you (presumably) think it does.  It sets h to 32, but does nothing to w.  You probably want, w = h = 32.

Minor nit, but those semicolons after the closing braces of your functions are technically illegal (statement at global scope).  Very few compilers care, but some do and they won't take that code.

'w' and 'h' is never used so it doesn't matter, but thank you.

About the semicolons, I read some tutorial that it was a must to have semicolons after functions in classes. Originally I had none but added them after I read it.
Proves that you can't believe everything you read on the Internet. Not that I didn't know that before.

Thank you :gentleman:

You need one after the class definition, that is:

Code:
class Doodad
{
public:
    Doodad();
    ~Doodad();
    void DoTheDoodad();

private:
    int doodad_val;
}; // <-- Need one there.

But not after the functions themselves.


Title: Re: Engine help (code inside)
Post by: eddietree on October 27, 2009, 08:19:07 AM
Just a small tip on naming conventions of variables. Your w,h vars might be confused with local variables and if you have a big function, it can quickly become hard to seperate what are local variables and what are member variables. A good way to overcome this is to add a "m_" prefix before your member variables.

For example, use m_width, m_height instead of width,height.


Title: Re: Engine help (code inside)
Post by: Average Software on October 27, 2009, 09:10:57 AM
Just a small tip on naming conventions of variables. Your w,h vars might be confused with local variables and if you have a big function, it can quickly become hard to seperate what are local variables and what are member variables. A good way to overcome this is to add a "m_" prefix before your member variables.

For example, use m_width, m_height instead of width,height.

Strongly disagree, but to each their own.

Storage class prefixes on variables become a maintenance nightmare if you ever have to change the storage class, since now the variable name has to change.  As a program gets larger and more people work on it, the chances of the programmer actually making those changes seems to decrease.  When you prefix things like that, you're effectively embedding comments in the code, and look how rarely people update their comments.

I've been involved in large projects where people did that, things changed and the prefix never got changed.  Now you can't trust the prefixes, which makes them pointless in the first place.

I swear I found an article somewhere by some computer scientist that had actually managed to formally prove that variables prefixes are bad.  I must go looking for that sometime.


Title: Re: Engine help (code inside)
Post by: Ivan on October 27, 2009, 10:27:25 AM
I also would like to speak out against variable prefixes. Aside from what Average Software said, I also find that they make code less readable, that is less english-like, which I think is important for code readability (objectCount vs m_numObjects).

I think that longer, but more natural variable and function names generally go a long way towards code that you can actually read later on.


Title: Re: Engine help (code inside)
Post by: BorisTheBrave on October 27, 2009, 01:28:37 PM
Rubbish, variable's with m_ ftw. I use this for private variables. To access them externally you'd foobar(), not m_foobar. Refactor safe, and readable. I'm particularly confused as to how you think m_foobar is any harder to refactor than foobar. It it is no longer a member variable, then the code is certainly not going to compile without modifications everywhere you referenced it, so there's no danger that people will leave in the m_ longer than necessary.
For structs just for data purposes, and other visible member variables, I have no prefix.

On the OP's topic: I, and most other people here, rarely bother to download, compile and observe the problem first hand, then dig into the code to debug it. That's a lot of work. It's hard to read someone else code without getting distracted by other issues (see above). If you describe what is wrong, and perhaps sample pertinent code, it would be more productive.


Title: Re: Engine help (code inside)
Post by: Ivan on October 27, 2009, 01:51:52 PM
For me the thing with m_ style prefixes is that i've mostly seen it in code written by people who were hardcore C++ programmers (mostly the MFC kind) and their code, while in theory clean and well layed out, seemed like it was part of this paradigm of code as "code", that is to say, as something that was not meant to be read by a human being.

A good thing to contrast it would be Apple's Objective C API, for example, which has really long function and member names that are pretty much plain english (stringByAddingPercentEscapesUsingEncoding).

I guess what I'm trying to say is that prefixes, while they are designed to help you read code, seem to me more of a detriment to actual code readability, because they decrease readability by homogenizing code pieces. If you've ever had to read someone else's code, you'll know what staring at a class full of incomprehensible methods and members is like.


Title: Re: Engine help (code inside)
Post by: Average Software on October 27, 2009, 03:43:00 PM
It it is no longer a member variable, then the code is certainly not going to compile without modifications everywhere you referenced it, so there's no danger that people will leave in the m_ longer than necessary.

Not at all true.  I've seen members changed into class statics without the prefix being changed, I've also seen them changed into globals.  Both cases will compile with modification in most cases.

I read something once that said something like, "If you name a variable for what it represents, its type and storage should be obvious, so there is no need for a prefix.  If you don't know what a variable represents, knowing its type and storage doesn't help you any, therefore variable prefixes are useless visual noise."

Stroustrup's opinion is, as always, interesting:

Quote from: Bjarne Stroustrup
How do you name variables? Do you recommend "Hungarian"?

 No I don't recommend "Hungarian". I regard "Hungarian" (embedding an abbreviated version of a type in a variable name) a technique that can be useful in untyped languages, but is completely unsuitable for a language that supports generic programming and object-oriented programming - both of which emphasize selection of operations based on the type an arguments (known to the language or to the run-time support). In this case, "building the type of an object into names" simply complicates and minimizes abstraction. To various extent, I have similar problems with every scheme that embeds information about language-technical details (e.g., scope, storage class, syntactic category) into names. I agree that in some cases, building type hints into variable names can be helpful, but in general, and especially as software evolves, this becomes a maintenance hazard and a serious detriment to good code. Avoid it as the plague.

Emphasis mine.


Title: Re: Engine help (code inside)
Post by: Jason Bakker on October 27, 2009, 04:53:16 PM
What do you do if you have a class like Sprite, that has a member "width", and you want to pass in a new "width" through a SetWidth(float width) function? They're both width, but if you just do

Code:
width = width;

It won't work... I'd guess you do something like this->width = width? Or call the passed in width "newWidth" or something?

Just curious, because I find the m_ really helpful when reading/writing code, to know which variables are instance and which are member... but if there's a better way, I'd probably swap to that :P


Title: Re: Engine help (code inside)
Post by: BlueSweatshirt on October 27, 2009, 05:09:19 PM
I agree with abbreviation in some cases. OpenGL uses a kind of abbreviation I generally agree with.
Example being glVertex3f. gl means it's an OpenGL function, 3f means you're using 3 floating numbers as the arguments. It's hardly an obstruction, and I think necessary.(if not necessary, helpful and useful.)

But in the case of using 'm_' I disagree. It's unnecessary and I've found it to be only a burden to using the variable. If you have two variable names clashing, then simply, that's your fault.

There's also the case of "my", which in some cases I do agree with. But again, I think it's just obstructive to use it large quantities.

[EDIT]
"this->" is a really good method, I think. It's simple, and you still have clean variables to work with. When you're working with variables that might clash, you just throw that in front of the member variables.


Title: Re: Engine help (code inside)
Post by: Snakey on October 27, 2009, 08:58:38 PM
At the end of the day, it depends on who is viewing the code. If you are the sole programmer or lead programmer then you decide what things to use and what things now to use. Everyone has a different style of programming.

Hungarian notation or not, is a very similar argument to what exactly is the true way to brace or indent.


Title: Re: Engine help (code inside)
Post by: hagel on October 28, 2009, 02:37:31 AM
If you aren't using line segment separation, then I highly suggest it (and I'll tell you how to do it.)
I'm sorry but, when?


Also; can anyone say derail?


Title: Re: Engine help (code inside)
Post by: Average Software on October 28, 2009, 04:44:06 AM
I agree with abbreviation in some cases. OpenGL uses a kind of abbreviation I generally agree with.
Example being glVertex3f. gl means it's an OpenGL function, 3f means you're using 3 floating numbers as the arguments. It's hardly an obstruction, and I think necessary.(if not necessary, helpful and useful.)

This is a different thing altogether.  gl*3f, gl*3i, gl*3fv... are ways to compensate for C's lack of namespaces and overloading.  OpenGL bindings in other languages typically drop the gl in favor of a namespace, and all the 3f, 3i, crap for overloading.


Title: Re: Engine help (code inside)
Post by: hagel on October 31, 2009, 04:15:29 AM
I'm thinking that I'm too stupid for slopes. I'll make it with only box tiles instead *snore*.
I guess you have to know when to give up.


Title: Re: Engine help (code inside)
Post by: Hima on October 31, 2009, 06:39:44 AM
hagel, have you tried this link?
http://jnrdev.72dpiarmy.com/en/jnrdev2/ (http://jnrdev.72dpiarmy.com/en/jnrdev2/)

Or maybe you can just integrate box2d. Don't know if that's going to be overkill though XD


Title: Re: Engine help (code inside)
Post by: hagel on November 17, 2009, 10:43:12 AM
hagel, have you tried this link?
http://jnrdev.72dpiarmy.com/en/jnrdev2/ (http://jnrdev.72dpiarmy.com/en/jnrdev2/)

Or maybe you can just integrate box2d. Don't know if that's going to be overkill though XD
I didn't want to use box2d because I thought it would be overkill, yeah. About the jnrdev link, I wrote in the first post about that.

Anyways, yeah...  :noir:


Title: Re: Engine help (code inside)
Post by: jmp on November 17, 2009, 01:12:48 PM
Quote from: hagel
I am a beginner and this is some seriously messed up code.
Help/redesign is much appreciated.

A few basic tips, take them or not:

  • Favor the C++ versions of the C headers (cstdio, cmath, ctime, etc.). The difference is that the headers prefixed with “c” contain the functions in the namespace std. Also, at the moment you are including both cstdlib and stdlib.h, which is useless.[1]
  • Since you are writing C++, use pointers when you have to, otherwise prefer references. For example you have a function that modifies the value of a float that is passed by pointer. You could use a reference instead and you would also get rid of the pointer syntax.[2]
  • Try to avoid importing whole namespaces in the global scope unless it is really necessary (and it rarely is). The “using namespace std;” pollutes the current scope with everything in that namespace, which is not good style and results in potential naming conflicts. Remember that you can bring individual names by using e.g. “using std::cout;[3]
  • Use initialization lists in constructors.[4]

[1]: http://parashift.com/c++-faq-lite/coding-standards.html#faq-27.4
[2]: http://parashift.com/c++-faq-lite/references.html#faq-8.6
[3]: http://parashift.com/c++-faq-lite/coding-standards.html#faq-27.5
[4]: http://parashift.com/c++-faq-lite/ctors.html#faq-10.6


Title: Re: Engine help (code inside)
Post by: st33d on November 17, 2009, 02:21:20 PM
You could try a ball against the slope instead of a rectangle. It's working for me currently, but I'm doing a sliding game.


Title: Re: Engine help (code inside)
Post by: Zaphos on November 17, 2009, 03:02:28 PM
[1]: http://parashift.com/c++-faq-lite/coding-standards.html#faq-27.4
[2]: http://parashift.com/c++-faq-lite/references.html#faq-8.6
[3]: http://parashift.com/c++-faq-lite/coding-standards.html#faq-27.5
[4]: http://parashift.com/c++-faq-lite/ctors.html#faq-10.6
Since the c++ faq lite can be a bit silly at times, it might be fun to also read the c++ fqa lite's take:
[2] http://yosefk.com/c++fqa/ref.html#fqa-8.6
[4] http://yosefk.com/c++fqa/ctors.html#fqa-10.6

(I wouldn't suggest worrying too much about programming style, though!)


Title: Re: Engine help (code inside)
Post by: jmp on November 17, 2009, 03:21:52 PM
Quote from: Zaphos
Since the c++ faq lite can be a bit silly at times, it might be fun to also read the c++ fqa lite's take:

Fun, yes. But I for one would not take C++ advice from a self-proclaimed C++ hater.


Title: Re: Engine help (code inside)
Post by: Zaphos on November 17, 2009, 03:59:21 PM
I just mean to say timtowtdi -- there's some reasonable advice in the fqa though, and some questionable advice in the faq.  I would take advice from anyone who says reasonable things, and the fqa does that ... I tend to lean towards the proper faq's opinions on references and initializer lists, but it's good to see the fqa's caveats in addition to the faq's rules of thumb.  (The fqa is increasingly entertaining the more obnoxious the faq gets -- the chapter on exceptions is particularly fun.)


Title: Re: Engine help (code inside)
Post by: Garthy on November 17, 2009, 05:38:10 PM
Another approach is to get everything working with just boxes initially, and go back and add slopes later. That way you can get everything stable, and then make adjustments and improvements later on. If they're a pain at present, I'd leave them out. You may not need them, or might feel more confident implementing them later.

Re special member prefixes (eg. "m_"), I'm in the "don't do it" camp, but to each their own. There are pros and cons. I can work with code that does it, or doesn't. Doesn't bother me that much.

I will slap any non-rookie developer who regularly embeds type information into variable names though. ;)