Transport Tycoon Forums

The place to talk about Transport Tycoon
It is currently Tue Dec 12, 2017 12:06 pm

All times are UTC




Post new topic  Reply to topic  [ 15 posts ] 
Author Message
PostPosted: Thu Sep 17, 2009 5:13 pm 
Offline
Engineer
Engineer
User avatar

Joined: Mon Aug 27, 2007 6:46 pm
Posts: 97
Location: mapy.cz/?query=rajhrad
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:
// 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:

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!


Top
   
PostPosted: Thu Sep 17, 2009 10:47 pm 
Offline
TTDPatch Developer
TTDPatch Developer

Joined: Wed Feb 18, 2004 3:06 am
Posts: 10285
An00biS wrote:
Code:
Entity* someEntity; // Pointer
class SomeClass{
        Entity* mSomeEntity; // Member pointer
}

Bad plan. That's asking for
Code:
Entity* someEntity,* otherEntity;
Entity* mSomeEntity,* mOtherEntity;
Or, worse,
Code:
Entity* someEntity, otherEntity;
Entity* mSomeEntity, mOtherEntity;


Try this instead:
Code:
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


Top
   
PostPosted: Fri Sep 18, 2009 7:22 pm 
Offline
Engineer
Engineer
User avatar

Joined: Wed Sep 09, 2009 6:36 pm
Posts: 55
Skype: same as y!
Location: Bucharest
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:
void aMethodWithLongName();


making it:

Code:
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:
// 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 :)


Top
   
PostPosted: Fri Sep 18, 2009 10:06 pm 
Offline
Engineer
Engineer
User avatar

Joined: Mon Aug 27, 2007 6:46 pm
Posts: 97
Location: mapy.cz/?query=rajhrad
?( Your post makes little sense to me.

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

Code:
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!


Top
   
PostPosted: Sat Sep 19, 2009 9:40 am 
Offline
Engineer
Engineer
User avatar

Joined: Wed Sep 09, 2009 6:36 pm
Posts: 55
Skype: same as y!
Location: Bucharest
i realized after i turned off the pc and didn't bother to edit it xD

3 will stay then.

Quote:
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.


Top
   
PostPosted: Sat Sep 19, 2009 7:41 pm 
Offline
Engineer
Engineer
User avatar

Joined: Wed Sep 09, 2009 6:36 pm
Posts: 55
Skype: same as y!
Location: Bucharest
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.


Top
   
PostPosted: Sun Sep 20, 2009 11:55 am 
Offline
Engineer
Engineer
User avatar

Joined: Wed Sep 09, 2009 6:36 pm
Posts: 55
Skype: same as y!
Location: Bucharest
code cleaned and on svn :) you can check the next systems that will be coded (at least them) on the wiki under development.


Top
   
PostPosted: Sat Sep 26, 2009 9:35 am 
Offline
Engineer
Engineer
User avatar

Joined: Mon Aug 27, 2007 6:46 pm
Posts: 97
Location: mapy.cz/?query=rajhrad
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!


Top
   
PostPosted: Sat Sep 26, 2009 10:37 am 
Offline
Engineer
Engineer
User avatar

Joined: Wed Sep 09, 2009 6:36 pm
Posts: 55
Skype: same as y!
Location: Bucharest
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)


Top
   
PostPosted: Tue Oct 13, 2009 11:01 pm 
Offline
Engineer
Engineer
User avatar

Joined: Mon Aug 27, 2007 6:46 pm
Posts: 97
Location: mapy.cz/?query=rajhrad
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!


Top
   
PostPosted: Wed Oct 14, 2009 10:26 am 
Offline
Engineer
Engineer
User avatar

Joined: Wed Sep 09, 2009 6:36 pm
Posts: 55
Skype: same as y!
Location: Bucharest
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;


Top
   
PostPosted: Tue Oct 27, 2009 10:46 am 
Offline
Engineer
Engineer
User avatar

Joined: Mon Aug 27, 2007 6:46 pm
Posts: 97
Location: mapy.cz/?query=rajhrad
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:
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:
namespace ABCD{
     /* code here */
}
or
Code:
{
    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!


Top
   
PostPosted: Tue Oct 27, 2009 6:29 pm 
Offline
Tycoon
Tycoon
User avatar

Joined: Mon Aug 09, 2004 12:14 am
Posts: 1293
Location: Gouda, The Netherlands
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.


Top
   
PostPosted: Tue Oct 27, 2009 9:31 pm 
Offline
Engineer
Engineer
User avatar

Joined: Wed Sep 09, 2009 6:36 pm
Posts: 55
Skype: same as y!
Location: Bucharest
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:
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.


Top
   
PostPosted: Sun Nov 01, 2009 11:26 pm 
Offline
Engineer
Engineer
User avatar

Joined: Mon Aug 27, 2007 6:46 pm
Posts: 97
Location: mapy.cz/?query=rajhrad
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:
MyEnum e;
e = MyEnum::someValue; // BAD!
e = someValue // Good!

~An00biS

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


Top
   
Display posts from previous:  Sort by  
Post new topic  Reply to topic  [ 15 posts ] 

All times are UTC


Who is online

Users browsing this forum: No registered users and 1 guest


You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot post attachments in this forum

Search for:
Jump to:  
Powered by phpBB © 2000-2017 phpBB Limited

Copyright © Owen Rudge/The Transport Tycoon Forums 2001-2017.
Hosted by Zernebok Hosting.