Code reformatting project [DONE]

Development directions, tasks, and features being actively implemented or pursued by the development team.

Code reformatting project [DONE]

Postby chuck_starchaser » Wed Feb 10, 2010 5:34 pm

The Code Reformatting Project

History:
This topic began in the AI thread, where we discussed the ugliness of the current formatting, and
how difficult it makes it to understand the code --too hard to even clean it up, not to speak of
refactoring. Safemode mentioned he felt tempted to run a formatter on all the code. I liked the
idea because, with a tool and an agreed format, it would become possible for me to reformat code
to my personal likings (more like needs, really); and then reformat it back before committing.
Pheonixstorm posted some links to formatters, and I fell in love with uncrustify, for its vast set
of configuration settings. After working with uncrustify for 3 or 4 days, I managed to come up
with a pair of configuration sets (unc_me.cfg and unc_them.cfg) that work reversively. By
that I mean that I can take a file A.cpp formatted with unc_them, reformat with unc_me to B.cpp,
then reformat B.cpp with unc_them to C.cpp, and verify that C.cpp was identical to A.cpp.
However, this was not as trivial as it may sound: There are non-reversible changes in a me-them
reformat, such as a space added before some trailing comments. Also, there seemed to be a bug
in uncrustify that when multiple comment lines are below a closing brace, one of them jumps
above the closing brace at each reformatting step.
The way I solved these issues was by reformatting files multiple times consecutively, using a
shell batch file. What the initial formatter script does is it reformats the file given in its argument
eleven times consecutively with the unc_them settings, then 11 times with unc_me, and again
11 times with unc_them.
But even that was not enough. One remaining problem was with a blank line occasionally after
a single-statement if body when followed by else if. Apparently another uncrustify bug. To fix it,
I put together a config that only removes braces of single statement conditional blocks, --nothing
else, to pre-process the files; and called it unc_pre.cfg.
So, the batch file actually runs uncrustify with unc_pre, followed by 11 passes of unc_them, 11
passes of unc_me, and 11 passes of unc_them. (Only takes 7 seconds on my machine for a
big file, like aggressive.cpp.)
The resulting file, now, is the one to commit, and can be reformatted with unc_me, and then
back again with unc_them, just a single pass each way, and result in no differences. In other
words, the long initial processing results in a "settlement" of all reversibility issues.

Yesterday --as of this writing-- I created a "reformat" svn branch and committed a first set of
files for discussion and feedback:

aggressive.h
aggressive.cpp
comm_ai.h
comm_ai.cpp
communication.h
communication.cpp

For the curious, here's an example of aggressive.cpp reformatted with my personal settings:
http://deeplayer.com/uncrustify/chucks_ ... essive.cpp

The reformatted files compile with no problems, by the way.

But then I was asked for more info, such as the configurations used. I decided to post not
only the configurations but also the batch files, and full instructions for how to get set up
with uncrustify.


Getting started (in Ubuntu)

Ubuntu has uncrustify, but a stable version (0.53 I think) that doesn't work. You'll need the most
recent version of uncrustify, from the git repository. If you don't have git, you can install it via
Ubuntu's Synaptic package manager; but it's not called "git"; they call "git" something else; you
need to get "git-base". Once you got git, you can get uncrustify by, at the console, typing:
Code: Select all
$ git clone git://uncrustify.git.sourceforge.net/gitroot/uncrustify/uncrustify

or
Code: Select all
$ git clone git://github.com/bengardner/uncrustify.git

Then, cd into the new uncrustify folder and type
Code: Select all
$ ./configure

followed by
Code: Select all
$ make

and optionally
Code: Select all
$ sudo make install


Under home, you should create a hidden uncrustify folder ( ~/.uncrustify ) to place the .cfg
files into, though uncrustify doesn't seem to see them, anyways, I end up having to spell out
the full path, but anyhow, it's where the batch files assume they are, so might as well do it. :)
Into that folder you can download these three files:
http://deeplayer.com/uncrustify/unc_pre.cfg
http://deeplayer.com/uncrustify/unc_them.cfg
http://deeplayer.com/uncrustify/unc_me.cfg

Also under your home folder, you want to creat a bin folder ( ~/bin ), if you don't already have
one. Anything in ~/bin is automatically added to your path variable, so it executes from any
folder you're in, but if you create /bin just now, you'll need to log out and log back in before
Ubuntu takes notice of it. This is where we will place the shell scripts. Download the following
scripts into it:
http://deeplayer.com/uncrustify/reformat11.sh
http://deeplayer.com/uncrustify/initial_reformat.sh
http://deeplayer.com/uncrustify/reformat_cs_style.sh
http://deeplayer.com/uncrustify/reformat_vs_style.sh
You'll need to chmod them to make them executable, of course. I think it's chmod a+x <name>.

Finally, you probably want to check out the reformat branch:
https://vegastrike.svn.sourceforge.net/ ... s/reformat


To do:

One thing I need to change is the settings for the removal of conditional block braces of
single statement blocks. Some single statements can span many lines, such as an if, else if
chain, for example. There's a setting I saw that prevents removal of braces when the block
spans more than N lines, where N can be set. I think I'd set it to 1.
Just need to find that setting again...

By the way, if you want to experiment with uncrustify, one link to keep handy is this one:
http://github.com/bengardner/uncrustify ... efault.cfg

Another thing to do is produce similar instructions for Windows users, and put them into
the Wiki. I've lost access to the vs wiki, due to the underscore in my username; so please
someone volunteer. Pretty please...
Last edited by chuck_starchaser on Fri Feb 26, 2010 7:25 am, edited 1 time in total.
User avatar
chuck_starchaser
Elite
Elite
 
Posts: 8014
Topics: 195
Joined: Thu Sep 04, 2003 9:03 pm
Location: Montreal

Share On:

Share on Facebook Facebook Share on Twitter Twitter Share on Digg Digg

Re: Code reformatting project

Postby chuck_starchaser » Wed Feb 10, 2010 6:25 pm

Update:
chuck_starchaser wrote:To do:

One thing I need to change is the settings for the removal of conditional block braces of
single statement blocks. Some single statements can span many lines, such as an if, else if
chain, for example. There's a setting I saw that prevents removal of braces when the block
spans more than N lines, where N can be set. I think I'd set it to 1.
Just need to find that setting again...

Done. Looks much better. Config files re-uploaded to the same links. Reformatted the
same 6 files in the reformat branch and committed; but the .h files did not change, so
only the 3 cpp files were updated. Revision 12622.
User avatar
chuck_starchaser
Elite
Elite
 
Posts: 8014
Topics: 195
Joined: Thu Sep 04, 2003 9:03 pm
Location: Montreal

Re: Code reformatting project

Postby charlieg » Thu Feb 11, 2010 7:10 am

Actually I would advise against the removal of braces from single line conditionals.

It is one of those things that looks tidy and slightly reduces screen 'dirt' - but it can be a nightmare when it comes to going through old code or creating accidental mistakes where something is missed out of a conditional or gets inadvertantly caught in a conditional.

The braces, whilst not necessary, make it much easier to see what is going on. I work with a lot of code where the original authors put single line conditionals on the same line because they valued conciseness, but reading through large amounts of it and seeing what is going on becomes quite hard.

Good formatting is easy to understand foremost, concise secondmost. You should be able to glance at something and know what is going on - know where to look without having to read each character.

Consider:

Code: Select all
if (simple_bool) return true;
if (someCall() && !simple_bool && !another_bool) return false;

vs

if (simple_bool) {
    return true;
}
if (someCall() && !simple_bool && !another_bool) {
    return false;
}


Oh, real easy to understand, right? It is. But consider you hadn't yet looked at the containing function and you had established that it was returning false incorrectly. Finding that 'return false' is instant in the braced version because the returns appear at a consistent indentation and are distinctly separated from the conditional logic. You have to read the entire unbraced version to find that 'return false'.

So, you argue, you do this:

Code: Select all
if (simple_bool)
    return true;
if (someCall() && !simple_bool && !another_bool)
    return false;


All the benefits, none of the problems?

Not quite. I have seen huge spanners in the works that have required debugging sessions because of something accidentally being left in that gets caught in the conditional by an accidental paste or a tired edit.

Code: Select all
if (simple_bool)
    return true;
if (someCall() && !simple_bool && !another_bool) true;
    return false;


Spot the difference? Even if this simple example, that can be a bitch to spot. Now extrapolate that to tens, hundreds of files with hundreds, thousands of lines of code, and you are digging potholes that you'll fall down later on.

Keep it simple. Good, consistent, CLEAR code style beats concise each and every day of the week.

Remember, even for a single man project there are always 2 developers - you when you wrote the code, and you once you have forgotten [some] it.
Free Gamer - free software games compendium and commentary!
FreeGameDev forum - open source game development community
User avatar
charlieg
Elite Mercenary
Elite Mercenary
 
Posts: 1328
Topics: 56
Joined: Thu Mar 27, 2003 4:51 pm
Location: Manchester, UK

Re: Code reformatting project

Postby chuck_starchaser » Thu Feb 11, 2010 10:07 am

charlieg wrote:
Code: Select all
if (simple_bool)
    return true;
if (someCall() && !simple_bool && !another_bool)
    return false;


All the benefits, none of the problems?

Not quite. I have seen huge spanners in the works that have required debugging sessions because of something accidentally being left in that gets caught in the conditional by an accidental paste or a tired edit.

Code: Select all
if (simple_bool)
    return true;
if (someCall() && !simple_bool && !another_bool) true;
    return false;


Spot the difference? Even if this simple example, that can be a bitch to spot.
Yeah, but this is misindented. The last return false should not be indented. And it's possible a programmer would misindent like that, but after running it through the formatter the indentation will get fixed, and the error will be apparent.

In any case, the way the settings are now,
  • 1 - single statement conditional bodies are NOT put on the same line; they are on the next.
  • 2 - braces are only removed if the single statement only spans a single new-line AND if all other else if/else blocks in the chain are also single statement AND single line, and therefore removed. IOW, in a chain of if else if else if else blocks, the braces are either all removed or all kept.
So, the ONLY situation the braces are removed is when the single statement block is single line and not part of an else if else chain, except in the rare case where all the blocks in the chain are single statement AND single line.
I think this should be pretty safe.

EDIT:
By the way, I also forced newlines after semicolons (except in for statements), so, multiple statements in one line will be no more.
User avatar
chuck_starchaser
Elite
Elite
 
Posts: 8014
Topics: 195
Joined: Thu Sep 04, 2003 9:03 pm
Location: Montreal

Re: Code reformatting project

Postby pheonixstorm » Thu Feb 11, 2010 11:58 am

As long as you can get it to work w/o breaking the code I suggest we adopt this as soon as your done. Having clean code to work with will be very nice. BTW did you try astyle too or just uncrusty?
Because of YOU Arbiter, MY kids? can't get enough gas. OR NIPPLE! How does that mkae you feeeel? ~ Halo
User avatar
pheonixstorm
Elite
Elite
 
Posts: 1567
Topics: 113
Joined: Mon Jan 25, 2010 7:03 pm

Re: Code reformatting project

Postby chuck_starchaser » Thu Feb 11, 2010 12:16 pm

I just looked at the info page for astyle and didn't get a positive impression. I could be wrong, but I think astyle doesn't give you fine control of every detail; it gives you "style packages" to choose from that you specify via command line arguments.
With uncrustify, most settings have { ignore, add, remove, force } choices. If you look at my two config files, I very rarely use ignore, since I want it to produce a predictable output, reversible to/from conversions with different settings. So you can have filters that are relaxed and change little; or filters that are tight as a nut, like mine.
It doesn't seem to me astyle even has tightness adjustments. It probably does a good job for you if you happen to like one of the styles.
Furthermore, if you look at their news page, it says that the latest version would produce changes in files that would not have happened before... That is EXACTLY what we don't want: A tool that will produce different results at each new release. The way uncrustify works, features are added, while compatibility is kept with old features, except for bug fixes. That's what we want; --reliable results, so we can rest assured files won't show superfluous changes on diffs all the time.
User avatar
chuck_starchaser
Elite
Elite
 
Posts: 8014
Topics: 195
Joined: Thu Sep 04, 2003 9:03 pm
Location: Montreal

Re: Code reformatting project

Postby chuck_starchaser » Thu Feb 11, 2010 12:55 pm

chuck_starchaser wrote:
charlieg wrote:
Code: Select all
if (simple_bool)
    return true;
if (someCall() && !simple_bool && !another_bool) true;
    return false;


Spot the difference? Even if this simple example, that can be a bitch to spot.
Yeah, but this is misindented. The last return false should not be indented. And it's possible a programmer would misindent like that, but after running it through the formatter the indentation will get fixed, and the error will be apparent.
Just wanted to add, the code example above, after passing through uncrustify with the present settings, would become...
Code: Select all
if (simple_bool)
    return true;
if (someCall() && !simple_bool && !another_bool)
    true;
return false;
...and as such the bug becomes very visible.
User avatar
chuck_starchaser
Elite
Elite
 
Posts: 8014
Topics: 195
Joined: Thu Sep 04, 2003 9:03 pm
Location: Montreal

Re: Code reformatting project

Postby chuck_starchaser » Thu Feb 11, 2010 4:44 pm

Okay, feedback time is up :D Started reformatting in earnest.
I finished reformatting all the files in /vegastrike/src/cmd/ai,
did a make clean and make, and vegastrike still runs.
So, I decided to do all the folders under cmd, recursively, and
in collide2 there's one file where uncrustify chokes...
/vegastrike/src/cmd/collide2/OPC_Picking.cpp
where uncrustify says that there's an unmatched open brace in line 23.
I find it in line 24, rather,
http://vegastrike.svn.sourceforge.net/v ... iew=markup
Now, the braces seem to be okay, but what I see is the #endif for
that #ifdef OPC_RAYHIT_CALLBACK on line 26, after the open brace
of the namespace, the #endif is at the bottom of the file AFTER the
namespace's closing brace...

Yep, after moving the #endif above the closing brace, uncrustify
was able to reformat it no problemo. Never mind... false alarm.
User avatar
chuck_starchaser
Elite
Elite
 
Posts: 8014
Topics: 195
Joined: Thu Sep 04, 2003 9:03 pm
Location: Montreal

Re: Code reformatting project

Postby klauss » Thu Feb 11, 2010 5:09 pm

I wouldn't process opcode, it's an external library. We might want to be able to accept patches applicable to the original version for instance, and reformatting would impede that.
Oíd mortales, el grito sagrado...
Call me "Menes, lord of Cats"
Wing Commander Universe
User avatar
klauss
Elite
Elite
 
Posts: 7243
Topics: 55
Joined: Mon Apr 18, 2005 7:40 am
Location: LS87, Buenos Aires, República Argentina

Re: Code reformatting project

Postby chuck_starchaser » Thu Feb 11, 2010 6:02 pm

No problem; I'll just delete them and svn up.
Need to know which files, tho.
That #endif in the wrong place is a bug, however.
Any other libraries I should be aware of?
There's copies of stdafx.h in several folders.
User avatar
chuck_starchaser
Elite
Elite
 
Posts: 8014
Topics: 195
Joined: Thu Sep 04, 2003 9:03 pm
Location: Montreal

Re: Code reformatting project

Postby klauss » Thu Feb 11, 2010 6:10 pm

chuck_starchaser wrote:No problem; I'll just delete them and svn up.

You want to "svn revert".
Oíd mortales, el grito sagrado...
Call me "Menes, lord of Cats"
Wing Commander Universe
User avatar
klauss
Elite
Elite
 
Posts: 7243
Topics: 55
Joined: Mon Apr 18, 2005 7:40 am
Location: LS87, Buenos Aires, República Argentina

Re: Code reformatting project

Postby chuck_starchaser » Thu Feb 11, 2010 7:20 pm

All .h and .cpp under /src/cmd done and committed. Revision 12625.
User avatar
chuck_starchaser
Elite
Elite
 
Posts: 8014
Topics: 195
Joined: Thu Sep 04, 2003 9:03 pm
Location: Montreal

Re: Code reformatting project

Postby chuck_starchaser » Thu Feb 11, 2010 8:57 pm

Sorry, I forgot to try make before committing, and now I found a file doesn't compile...
Code: Select all
src/cmd/basecomputer.cpp: In function ‘void showUnitStats(Unit*, std::string&, int, int, Cargo&)’:
src/cmd/basecomputer.cpp:4761: error: expected ‘}’ before ‘else’
src/cmd/basecomputer.cpp: At global scope:
src/cmd/basecomputer.cpp:4774: error: expected unqualified-id before ‘if’
make[1]: *** [src/cmd/basecomputer.o] Error 1
make[1]: Leaving directory `/home/d/vs/vegastrike'
make: *** [all] Error 2


Update: Never mind; it wasn't an uncrustify problem; it was a problem of multi-line
macros without do{ ... }while(0) wrappers in basecomputer.cpp... three of them.
Fixed. Now it makes. Revision 12626.
User avatar
chuck_starchaser
Elite
Elite
 
Posts: 8014
Topics: 195
Joined: Thu Sep 04, 2003 9:03 pm
Location: Montreal

Re: Code reformatting project

Postby chuck_starchaser » Fri Feb 12, 2010 2:00 am

All done; all sources reformatted. Revision 12627.
User avatar
chuck_starchaser
Elite
Elite
 
Posts: 8014
Topics: 195
Joined: Thu Sep 04, 2003 9:03 pm
Location: Montreal

Re: Code reformatting project

Postby chuck_starchaser » Fri Feb 12, 2010 9:47 am

I'm trying to fix python_class.h so that it can be reformatted.
As I mentioned, the problem is with macros that open a brace but don't close it, macros that close a parenthesis they never opened, and stuff like that. My goal is to rewrite the macros so that the opening or closing braces and stuff are not part of the macro, but have to be placed before or after the macro, at the point of instantiation.
Managed to do it with the begin and end python module macros, but the begin and end class macros are pretty hard to understand, so I've been trying to manually reformat the file to make it more readable.
As I did so, I found something suspicious:
Code: Select all
#if BOOST_VERSION != 102800

#define PYTHON_BASE_BEGIN_INHERIT_CLASS(name,NewClass,SuperClass,myclass) { \
    boost::python::class_builder < SuperClass, NewClass, boost::noncopyable > Class (myclass
#define PYTHON_BEGIN_INHERIT_CLASS(name,NewClass,SuperClass,myclass) \
    PYTHON_BASE_BEGIN_INHERIT_CLASS(name,NewClass,SuperClass,myclass) \
);
#define PYTHON_BASE_BEGIN_CLASS(name,CLASS,myclass) { \
    boost::python::class_builder <CLASS> Class (myclass
#define PYTHON_BEGIN_CLASS(name,CLASS,myclass) \
    PYTHON_BASE_BEGIN_CLASS(name,CLASS,myclass) \
);
#define PYTHON_DEFINE_METHOD(modul,fun,funname) \
    modul.def (funname,fun)
#define PYTHON_DEFINE_METHOD_DEFAULT(modul,fun,funname,deflt) \
    modul.def (funname,deflt)

#else // if BOOST_VERSION != 102800 { ... } else ...

#define PYTHON_BASE_BEGIN_INHERIT_CLASS(name,NewClass,SuperClass,myclass) { \
    boost::python::class_builder < SuperClass ,NewClass > Class (name,myclass);
#define PYTHON_BEGIN_INHERIT_CLASS(name,NewClass,SuperClass,myclass) \
    PYTHON_BASE_BEGIN_INHERIT_CLASS(name,NewClass,SuperClass,myclass) \
    Class.def (boost::python::constructor<>());
#define PYTHON_BASE_BEGIN_CLASS(name,CLASS,myclass) { \
    boost::python::class_builder <CLASS> Class (name,myclass);
#define PYTHON_BEGIN_CLASS(name,CLASS,myclass) \
    PYTHON_BASE_BEGIN_CLASS(name,CLASS,myclass) \
    Class.def (boost::python::constructor<>());
#define PYTHON_DEFINE_METHOD(modul,fun,funname) \
    modul.def (fun,funname)
#define PYTHON_DEFINE_METHOD_DEFAULT(modul,fun,funname,defaultfun) \
    modul.def (fun,funname,defaultfun)

#endif // if BOOST_VERSION != 102800 { ... } else ...

I would expect something like,
if BOOST_VERSION < 102800
or
if BOOST_VERSION <= 102800
or
if BOOST_VERSION > 102800
or
if BOOST_VERSION >= 102800
but not
if BOOST_VERSION != 102800 ... :roll: ... or, did something very special happen only once at that revision?
User avatar
chuck_starchaser
Elite
Elite
 
Posts: 8014
Topics: 195
Joined: Thu Sep 04, 2003 9:03 pm
Location: Montreal

Re: Code reformatting project

Postby klauss » Fri Feb 12, 2010 10:47 am

Please, don't mess with python binding macros.

They're designed to work with several boost versions and are really complex.

Perhaps it's best if uncrustify doesn't touch those files.
Oíd mortales, el grito sagrado...
Call me "Menes, lord of Cats"
Wing Commander Universe
User avatar
klauss
Elite
Elite
 
Posts: 7243
Topics: 55
Joined: Mon Apr 18, 2005 7:40 am
Location: LS87, Buenos Aires, República Argentina

Re: Code reformatting project

Postby chuck_starchaser » Fri Feb 12, 2010 11:09 am

Alright, then; reverted.
So, Klauss, I'm basically done; everything is reformatted, and the engine builds without errors.
However, there's a couple of little gottchas at run-time:
  • The advertising screens don't show during game loading, white rectangle in thin black frame.
  • Local star (I think, could be a planet) shows a square box around it.
I'm 100% positive the problem is some multi-line macro instantiated as a single statement in a
conditional block somewhere. Had such problems in basecomputer.cpp, but they caused the
compiler to issue an error, and so I was able to fix them (wrap them in do{ ... }while(0)'s).
But I've no idea now where to even start looking for where such evil macros might be defined
to cause these problems. Could you give me some pointers?

Image
User avatar
chuck_starchaser
Elite
Elite
 
Posts: 8014
Topics: 195
Joined: Thu Sep 04, 2003 9:03 pm
Location: Montreal

Re: Code reformatting project

Postby klauss » Fri Feb 12, 2010 11:18 am

I'd suggest you revert everything in src/gfx, and then try building and testing. If the problem's gone, uncrustify in batches to try and find the problem.

This however suggests the script is sensitive to macro stuff... you could grep for #define statements and visually inspect them (I bet there aren't too many out there) to see if any one catches your eye.

For this particular problem, you should look for the cause in gfx, although it's not impossible that the problem could originate somewhere else.
Oíd mortales, el grito sagrado...
Call me "Menes, lord of Cats"
Wing Commander Universe
User avatar
klauss
Elite
Elite
 
Posts: 7243
Topics: 55
Joined: Mon Apr 18, 2005 7:40 am
Location: LS87, Buenos Aires, República Argentina

Re: Code reformatting project

Postby chuck_starchaser » Fri Feb 12, 2010 4:18 pm

I was out most of the day, but getting back to work now.
I'm going alphabetically through all the files in gfx, looking for bad macros. Found none
until I got to mesh_bxm.cpp, and in there I found dozens of multi-line macros without
do-while wrappers. Fixed them all, then put in all the missing semicolons, and it built
and run; but the problems are still there; so, continuing...
User avatar
chuck_starchaser
Elite
Elite
 
Posts: 8014
Topics: 195
Joined: Thu Sep 04, 2003 9:03 pm
Location: Montreal

Re: Code reformatting project

Postby chuck_starchaser » Fri Feb 12, 2010 4:47 pm

Well, mesh_gfx had a few multi-line macros too.
Now I was just looking at planetary_transform.h, and on line 41, the destructor
for class PlanetaryTransform looks pretty destructive to me ...
Code: Select all
    virtual ~PlanetaryTransform()
    {
        while (1)
            ;
    }
User avatar
chuck_starchaser
Elite
Elite
 
Posts: 8014
Topics: 195
Joined: Thu Sep 04, 2003 9:03 pm
Location: Montreal

Re: Code reformatting project

Postby klauss » Fri Feb 12, 2010 4:54 pm

chuck_starchaser wrote:
Code: Select all
    virtual ~PlanetaryTransform()
    {
        while (1)
            ;
    }


WTF?
^ read loudly and with anger

1) We can be sure that code is never used
2) We can agree that code should never be

I vote for deletion of planetary transform. And not just "rm planetary_transform.*"... better "schred planetary_transform.*"

;)
Oíd mortales, el grito sagrado...
Call me "Menes, lord of Cats"
Wing Commander Universe
User avatar
klauss
Elite
Elite
 
Posts: 7243
Topics: 55
Joined: Mon Apr 18, 2005 7:40 am
Location: LS87, Buenos Aires, República Argentina

Re: Code reformatting project

Postby chuck_starchaser » Fri Feb 12, 2010 11:59 pm

klauss wrote:
chuck_starchaser wrote:
Code: Select all
    virtual ~PlanetaryTransform()
    {
        while (1)
            ;
    }


WTF?
^ read loudly and with anger

1) We can be sure that code is never used
2) We can agree that code should never be

I vote for deletion of planetary transform. And not just "rm planetary_transform.*"... better "schred planetary_transform.*"

;)
Could it be that it's used as a base class and it's not supposed to be instantiated, and derived classes MUST override the dtor? But then why not make it pure virtual, eh? I'll try deleting it, see what happens.

Finished wrapping macros in all of the gfx folder, but I also added const to many functions, and now I'm
dealing with errors due to many functions that look const but aren't.

By the way, my old friend Koya, now my room-mate, installed TRAC in the junction server, and mirrored
the vs repository in sf, so, we're on trac :)
Timeline (minus wiki changes):
http://wcjunction.com/trac/timeline?fro ... ate=Update
Branches view by most recent commit:
http://wcjunction.com/trac/browser/bran ... ate&desc=1
He's going to put an auto-update cron job on it, so it stays current.
User avatar
chuck_starchaser
Elite
Elite
 
Posts: 8014
Topics: 195
Joined: Thu Sep 04, 2003 9:03 pm
Location: Montreal

Re: Code reformatting project

Postby chuck_starchaser » Sat Feb 13, 2010 1:44 am

r12629
Committed changes to files under /src/gfx. All command macros are now wrapped in do{}while(0)'s.
Also, I added const to const functions for about 1/3 of the files.
The problems at runtime haven't gone away, tho; so tomorrow I'll tackle the main /src folder.
User avatar
chuck_starchaser
Elite
Elite
 
Posts: 8014
Topics: 195
Joined: Thu Sep 04, 2003 9:03 pm
Location: Montreal

Re: Code reformatting project

Postby safemode » Sat Feb 13, 2010 2:42 am

cmon, you have to agree that the branch idea is definitely a win here.
Ed Sweetman endorses this message.
safemode
Developer
Developer
 
Posts: 2150
Topics: 84
Joined: Sun Apr 22, 2007 6:17 pm
Location: Pennsylvania

Re: Code reformatting project

Postby chuck_starchaser » Sat Feb 13, 2010 11:35 am

safemode wrote:cmon, you have to agree that the branch idea is definitely a win here.

Damn right you were; I've seen the light now ;-)
Alright, 25 more minutes of Quirks and Quarks (science radio show) and I'm getting back to work.

EDIT:
Got a nice workflow happening, now:
I have vim and gedit opened.
After I finish working on a set of related .h and .cpp files, I reuncrustify them and start a make
from within vim, which has the advantage that if there's a compile error I get to the code line
where it happens just by pressing enter. And while make is going, I work on the next set of
related .h and .cpp files in gedit. ;-)

EDIT2:
And, by the way, I'm expanding the scope of fixups to include putting virtual in dtors and
explicit in single arg ctors, as well as const's on functions and macro wrappers.

EDIT3:
And before anyone jumps, I know that adding virtual to dtors can slow down things, but on
the other hand, for 99% of classes the slowdown will be insignificant; and for those that it
isn't insignificant, there should be a big comment warning that the dtor should be non-
-virtual and why.
User avatar
chuck_starchaser
Elite
Elite
 
Posts: 8014
Topics: 195
Joined: Thu Sep 04, 2003 9:03 pm
Location: Montreal


Next

Return to Engine Development

Who is online

Users browsing this forum: No registered users and 0 guests

cron