next »

[hagel]

Good evening gentlemen.
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

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.

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

[Aquin]

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. 

[hagel]

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.

[Average Software]

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.

[hagel]

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

[Average Software]

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

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.

[eddietree]

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.

[Average Software]

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.

[Ivan]

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.

[BorisTheBrave]

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.

[Ivan]

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.

[Average Software]

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:

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.

[Jason Bakker]

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

[BlueSweatshirt]

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.

[Snakey]

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.

[hagel]

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?

[Average Software]

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.

[Hima]

hagel, have you tried this link?
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

[hagel]

hagel, have you tried this link?
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... 

[jmp]

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