[ott3d] Coding guidelines

Discussions related to programming Transport Empire.

Moderator: Transport Empire Moderators

Post Reply
User avatar
An00biS
Engineer
Engineer
Posts: 97
Joined: 27 Aug 2007 18:46
Location: mapy.cz/?query=rajhrad
Contact:

[ott3d] Coding guidelines

Post by An00biS » 17 Sep 2009 17:13

Hello.

In this thread I'd like to discuss all coding conventions of the ott3d branch.

Specifically, I'd like to settle on a coding style. Currently, styles are being mixed, which is something that drives me :evil:
I was able to count three different naming conventions:

Code: Select all

// 1:
void autofit_tracks();
// 2:
bool handle_Window_mouseEnter(const CEGUI::EventArgs &e);
// 3:
void updateLightmap();
I propose to stick to this single coding style (please :roll: )

Code: Select all


void someFunction(); // Function
int someVariable; // Variable
Entity* someEntity; // Pointer

class SomeClass{
    private:
        int mSomeAttribute; // Member variable = attribute
        Entity* mSomeEntity; // Member pointer
    public:
        void someMethod(int someArgument, int anotherArgument); // Member function = method
        void aMethodWithLongName();
}

Also, It's a mess to put methods of one class into various source files. An implementation of a class should be in a single file and if it's not possible, then all the files with it's implementation should at least have appropriate names.

And finally, even naming the source files should follow some rule. Doesn't matter which it is, but it should be consistent.

~An00biS
[Update Aug 2013] FlexRails track buiding demo coming soon!

DaleStan
TTDPatch Developer
TTDPatch Developer
Posts: 10285
Joined: 18 Feb 2004 03:06
Contact:

Re: [ott3d] Coding guidelines

Post by DaleStan » 17 Sep 2009 22:47

An00biS wrote:

Code: Select all

Entity* someEntity; // Pointer
class SomeClass{
        Entity* mSomeEntity; // Member pointer
}
Bad plan. That's asking for

Code: Select all

Entity* someEntity,* otherEntity;
Entity* mSomeEntity,* mOtherEntity;
Or, worse,

Code: Select all

Entity* someEntity, otherEntity;
Entity* mSomeEntity, mOtherEntity;
Try this instead:

Code: Select all

Entity *someEntity, *otherEntity;
Entity *mSomeEntity, *mOtherEntity;
To get a good answer, ask a Smart Question. Similarly, if you want a bug fixed, write a Useful Bug Report. No TTDPatch crashlog? Then follow directions.
Projects: NFORenum (download) | PlaneSet (Website) | grfcodec (download) | grfdebug.log parser

User avatar
lonwolf89
Engineer
Engineer
Posts: 55
Joined: 09 Sep 2009 18:36
Skype: same as y!
Location: Bucharest
Contact:

Re: [ott3d] Coding guidelines

Post by lonwolf89 » 18 Sep 2009 19:22

The auto-fit needs to be places in some kinda class that handles construction (doing it in gameManager screws up everything).

Long names drives this crazy:

Code: Select all

void aMethodWithLongName();
making it:

Code: Select all

void a_Method_With_Long_Name();
will pop up in ur eyeball better.

Tell me what code style you prefer: 1 2 or 3

Code: Select all

// 1:
void autofit_tracks();
// 2:
bool handle_Window_mouseEnter(const CEGUI::EventArgs &e);
// 3:
void updateLightmap();
(also note, i never start function names or variables or after the character '_' with capital, so that should stay as a guideline :)

User avatar
An00biS
Engineer
Engineer
Posts: 97
Joined: 27 Aug 2007 18:46
Location: mapy.cz/?query=rajhrad
Contact:

Re: [ott3d] Coding guidelines

Post by An00biS » 18 Sep 2009 22:06

?( Your post makes little sense to me.

First, there are conflicting statements:
1:
lonwolf89 wrote:making it:

Code: Select all

void a_Method_With_Long_Name();
will pop up in ur eyeball better.
2:
lonwolf89 wrote:i never start function names or variables or after the character '_' with capital, so that should stay as a guideline :)
And second:
lonwolf89 wrote:Tell me what code style you prefer: 1 2 or 3
I think my preferred coding style is obvious from my first post. But it's number 3 anyway.

Regards
~An00biS
[Update Aug 2013] FlexRails track buiding demo coming soon!

User avatar
lonwolf89
Engineer
Engineer
Posts: 55
Joined: 09 Sep 2009 18:36
Skype: same as y!
Location: Bucharest
Contact:

Re: [ott3d] Coding guidelines

Post by lonwolf89 » 19 Sep 2009 09:40

i realized after i turned off the pc and didn't bother to edit it xD

3 will stay then.
I think my preferred coding style is obvious from my first post
had a little attention problem. all the functions (and class member variables, but i remember these are fine) are being renamed now

also note, functions starting with "_" are supposed to be inline simple return functions.

User avatar
lonwolf89
Engineer
Engineer
Posts: 55
Joined: 09 Sep 2009 18:36
Skype: same as y!
Location: Bucharest
Contact:

Re: [ott3d] Coding guidelines

Post by lonwolf89 » 19 Sep 2009 19:41

a small update: i renamed all the odd looking functions and adding a new class ConstructionManager that will handle deforming the terrain, brushes, track placing (with autoFit() and everything else) so that adding a new functionality or improving something will be easier to spot and change (also AutoFit.cpp and the other odd looking GameManager_interfaceEvents.cpp are gone).

It's not ready yet, but when it will be, i'll upload on the svn and also change the wiki section (as always) to reflect the new update.

So, if you want to code something in the game source right now, wait for the new clean code, or use the current one as a test guideline.

User avatar
lonwolf89
Engineer
Engineer
Posts: 55
Joined: 09 Sep 2009 18:36
Skype: same as y!
Location: Bucharest
Contact:

Re: [ott3d] Coding guidelines

Post by lonwolf89 » 20 Sep 2009 11:55

code cleaned and on svn :) you can check the next systems that will be coded (at least them) on the wiki under development.

User avatar
An00biS
Engineer
Engineer
Posts: 97
Joined: 27 Aug 2007 18:46
Location: mapy.cz/?query=rajhrad
Contact:

Re: [ott3d] Coding guidelines

Post by An00biS » 26 Sep 2009 09:35

Hello.

I'm working with Code::Blocks IDE on linux and I want to upload my project files in the svn so there's a build script for linux.
Which makes me think about the SVN file layout, as currently there's everything in root and it could get messy soon

== My suggestion ==


Root directory
Readme files, sample config files, real config files and other misc data. This will make the root of ott3d distribution, this is where binaries would be output and run.

"Projects" directory
This is where the IDE project files will be put.

"Source" directory
Source files and header files. Nothing else.

If agreed, I'll reorganize the data with my commit. I'll only leave lonwolf's VisualC project flie and Checkpoints*.txt among the sources so I don't break anyting.

~An00biS
[Update Aug 2013] FlexRails track buiding demo coming soon!

User avatar
lonwolf89
Engineer
Engineer
Posts: 55
Joined: 09 Sep 2009 18:36
Skype: same as y!
Location: Bucharest
Contact:

Re: [ott3d] Coding guidelines

Post by lonwolf89 » 26 Sep 2009 10:37

you don't need to do that. the txt will be deleted by me (i added the checkpoints and something similar to a xml config file in a cfg) and i'll put the vcproject under projects (just in case one wants to open it and look in the proprietes).

i also added some more code in the new files and new managers coming in the next commit (sound and day/night transition + update on wiki on the library section)

User avatar
An00biS
Engineer
Engineer
Posts: 97
Joined: 27 Aug 2007 18:46
Location: mapy.cz/?query=rajhrad
Contact:

Re: [ott3d] Coding guidelines

Post by An00biS » 13 Oct 2009 23:01

More complaints :roll:

I don't like all ott3d code being wrapped into Ogre namespace. I suggest to use our own namespace or none; i.e. the default namespace. I've removed all namespace{} wraps in my working copy and added namespace qualifiers where necessary.

Also, I don't like constructor arguments being prefixed with "m". This should be reserved for members only.
When the args have same name as members, a "this" keyword needs to be used to work with both. But if we want to use the "this" keyword, what's the point of using the "m" prefix for members?
[Update Aug 2013] FlexRails track buiding demo coming soon!

User avatar
lonwolf89
Engineer
Engineer
Posts: 55
Joined: 09 Sep 2009 18:36
Skype: same as y!
Location: Bucharest
Contact:

Re: [ott3d] Coding guidelines

Post by lonwolf89 » 14 Oct 2009 10:26

naming them with m in the constructor gives the coder a hint that the pointers parsed are also members of the class that created them. So that you would know those arguments are being passed just so that the class can use them. The names can be changed, but i see it more natural this way since when you create the class you need some pointers to variables that are engine-related and stored in your classes prefixed with M. Also using "this" pointer won't disturb anything :P

If there is a namespace defined then i probably forgot to remove it, the point was to use the default namespace;

User avatar
An00biS
Engineer
Engineer
Posts: 97
Joined: 27 Aug 2007 18:46
Location: mapy.cz/?query=rajhrad
Contact:

Re: [ott3d] Coding guidelines

Post by An00biS » 27 Oct 2009 10:46

I'd like to settle the placement of member variables (attributes) inside a class definition.

Currently, it's agreed to do it "like it's done in OGRE headers", but that's ambiguous. Most files put attributes on the bottom of the class def, but some put it right after the "private" or "protected" keyword, which is often somewhere in the middle of a file. That's annoying because (unless you're using symbol browser, which I don't) it's hard to locate, one has to slowly scroll the file to find it. Attributes on the top or bottom of the file are found easily.

I prefer for the attribute defs to be placed on top, i.e:

Code: Select all

class Locomotive
{
    // == Attributes ==
    protected:
        float mMaxSpeed;
        float mWeight;
        float mPower;
        float mSpeed;

    // == Methods ==
    public:
        void setSpeed(float speed);
}
EDIT:
Another thing I just came across:

Let's never use the "using namespace" declaration outside of a block. Let's use

Code: Select all

namespace ABCD{
     /* code here */ 
}
or

Code: Select all

{
    using namespace ABCD;
    /* code here */
} // The namespace is returned to previous state. 
The problem with "using namespace" is: After it's declared globally in one file, it affects all subsequentially included files. So, ott3d classes with same name as library classes give problems. Example:
g++ wrote:/home/an00bis/codeblocks/ott3d/ott3d/TerrainManager.h|14|error: reference to 'ConfigFile' is ambiguous|
/home/an00bis/codeblocks/ott3d/ott3d/ConfigFile.h|30|error: candidates are: class ConfigFile|
/home/an00bis/include/OgreMain/OgreConfigFile.h|57|error: class Ogre::ConfigFile|
...
||=== Build finished: 50 errors, 0 warnings ===|
Namespaces are designed to avoid clashes between app code and lib code. Mixing app with lib's namespace beats their purpose. Let's avoid it.

~An00biS
[Update Aug 2013] FlexRails track buiding demo coming soon!

User avatar
Expresso
Tycoon
Tycoon
Posts: 1709
Joined: 09 Aug 2004 00:14
Location: Gouda, the Netherlands

Re: [ott3d] Coding guidelines

Post by Expresso » 27 Oct 2009 18:29

This page contains a lot of hints on how to create unmaintainable code. Perhaps a good idea to take a look over there for some hints on how to create maintainable code.

User avatar
lonwolf89
Engineer
Engineer
Posts: 55
Joined: 09 Sep 2009 18:36
Skype: same as y!
Location: Bucharest
Contact:

Re: [ott3d] Coding guidelines

Post by lonwolf89 » 27 Oct 2009 21:31

if you find missplaced member variables (at the beginning or at the end) just place them down or send a pm to the person responsible of the cruel act!!!11!

No seriously, must've escaped my eyes if there are things like that at copy/pasting code from one place to another.

And since we rely on Ogre source code let's keep the style:

Code: Select all

class MyClass
{

//methods
public:
   MyClass();
   ~MyClass();
   void f();

protected:
   int g(float x);
private:
   int parseAbs(int y);

//members
public:
   char mUnused;
protected:
   float *mAnotherVariable;
private:
   SceneManager *mSceneMgr;
};
if there are mix-ups in the order of the placing, it will be revised.

User avatar
An00biS
Engineer
Engineer
Posts: 97
Joined: 27 Aug 2007 18:46
Location: mapy.cz/?query=rajhrad
Contact:

Re: [ott3d] Coding guidelines

Post by An00biS » 01 Nov 2009 23:26

To all M$VC devs:

When assigning value to enum variables, don't use the enum name as class/namespace name. It's a microsoft extension which doesn't work in GCC.

Code: Select all

MyEnum e;
e = MyEnum::someValue; // BAD!
e = someValue // Good!
~An00biS
[Update Aug 2013] FlexRails track buiding demo coming soon!

Post Reply

Return to “Transport Empire Coding”

Who is online

Users browsing this forum: No registered users and 1 guest