Code quality (and a new contributor)

Everything related to the code /
Tout ce qui touche au code
Post Reply
User avatar
chkno
Posts: 9
Joined: Fri Jun 20, 2008 11:49 pm
Location: USA

Code quality (and a new contributor)

Post by chkno » Wed Jun 25, 2008 6:59 am

Fan of TA with programming skills in OpenGL and/or C++? You can help.
Please come to visit us in the forums.
I would like to help.

However, looking through the code, I see some very unpleasant things. For example, stuff like this.

1) The struct and the data are carefully lined up. Just cast the data to the type of the struct and access the fields as needed.
2) This is the *third* exact copy of this sequence in this file with only slightly different calling conventions. Copy/pasting 65 lines code to change the behaviour of six lines at the end is not a sustainable development practice.

I suspect the quality of the code is turning other potential developers away.

But, I am a fan of TA. I want to see your project succeed. I want to help.

My question: If I submit patches to clean up this kind of thing, will they be considered, or would I be wasting my time?

Here's a patch to clean up tnt.cpp. Comments?

milipili
Posts: 545
Joined: Thu Nov 02, 2006 8:52 am
Location: Paris (France)
Contact:

Post by milipili » Wed Jun 25, 2008 1:51 pm

I have started a new branch for these reasons and started to rewrite a lot of code (see the thread ofr this subject). But it takes time to do it and I think you could help obviously :)

May be we can merge changes to the trunk and remove the branch now ?

I think you should ask to zuzuf if it can create a svn account for you

Your patch is welcome but please avoid uppercase class names, we never know if it is a macro of a class... :) For my part if \r could be removed too... Did you use the tnt.* files from the trunk or the branch folder ? I think it does not matter I have not modified this file yet.
Damien Gerard
Ta3d & Yuni Developer

User avatar
chkno
Posts: 9
Joined: Fri Jun 20, 2008 11:49 pm
Location: USA

Post by chkno » Wed Jun 25, 2008 5:52 pm

I followed the local convention that I saw in this file for the uppercase class names. I would be happy to help fix broken local conventions to match project-wide conventions.

I was working out of the trunk here. I left the \r's as I found them. I would be happy to help impose some uniformity and strip them out. I agree that they do not belong.

milipili
Posts: 545
Joined: Thu Nov 02, 2006 8:52 am
Location: Paris (France)
Contact:

Post by milipili » Thu Jun 26, 2008 7:51 am

In fact with the branch, I try to :
- Reorder all classes (in a near future with more appropriated namespaces)
- Rename classes. Uppercase names are always use for defines and macros and it is the expected behavior.
- Fix all obvious errors
- Improve inlining (when it is really needed)
- Improve prototypes (mainly with the const keyword)
- Remove all ugly and not really debugable nested if/then/else statements (anyway VisualStudio does not support it)
- Removed all C code

In the same time, make some part of the code more thread-safe, but I will not change anything about that for the moment. A better modelisation will help to do it. In fact the best way I think would be to use the Boot::thread library but it has not been discussed with others yet (and there is a lot of stuff to do before).
Damien Gerard
Ta3d & Yuni Developer

User avatar
zuzuf
Administrateur - Site Admin
Posts: 3281
Joined: Mon Oct 30, 2006 8:49 pm
Location: Toulouse, France
Contact:

Post by zuzuf » Thu Jun 26, 2008 10:03 am

great :D

I've read your patch, no doubt you can help :wink: . We've still lots of work to do, and we need to clean up a several things. I've not found time to work much on TA3D for 2 weeks so the trunk folder hasn't changed much. I think we could merge your branch with trunk this week end and apply coding conventions to all the project renaming what needs to be renamed.
=>;-D Penguin Powered

User avatar
Balthazar
Moderator
Posts: 2055
Joined: Wed Nov 01, 2006 4:31 pm
Location: Russian Federation
Contact:

Post by Balthazar » Thu Jun 26, 2008 1:41 pm

Yay!! Cool! The more talanted developers - the better project will become! I`ll try to make as much noise as possible to attract more attention to the project :)

Best of luck, guys! Hope to play TA3D with all of you soon :)

User avatar
zuzuf
Administrateur - Site Admin
Posts: 3281
Joined: Mon Oct 30, 2006 8:49 pm
Location: Toulouse, France
Contact:

Post by zuzuf » Sat Jun 28, 2008 12:56 pm

I applied your patch and uploaded a commit :)
=>;-D Penguin Powered

Post Reply

Who is online

Users browsing this forum: No registered users and 37 guests