Angband.oook.cz
Angband.oook.cz
AboutVariantsLadderForumCompetitionComicScreenshotsFunniesLinks

Go Back   Angband Forums > Angband > Development

Reply
 
Thread Tools Display Modes
Old May 30, 2011, 07:14   #1
Derakon
Prophet
 
Derakon's Avatar
 
Join Date: Dec 2009
Posts: 8,943
Derakon is on a distinguished road
A plea for comments

I decided to take a stab at allowing ego items to have negative pvals, which requires understanding how the file parser works. This is...not helped by the fact that there are basically no comments for any of the code.

I know that the 2.8.3 days had very verbosely commented code; practically every line had a comment. That's not really necessary IMO, but on the flipside, every function more complicated than a getter or setter needs to have a comment. Please, when you modify a function, if you know what it does, comment it! If that function has a complex block within it, comment that too. You'll make everyones' lives easier. Commenting's not really fun, I know, but it'd be a big help in convincing people to contribute.
Derakon is offline   Reply With Quote
Old May 30, 2011, 07:20   #2
Timo Pietilš
Prophet
 
Join Date: Apr 2007
Location: Climbing up from hole I just dug.
Posts: 4,096
Timo Pietilš is on a distinguished road
Quote:
Originally Posted by Derakon View Post
I decided to take a stab at allowing ego items to have negative pvals, which requires understanding how the file parser works. This is...not helped by the fact that there are basically no comments for any of the code.

I know that the 2.8.3 days had very verbosely commented code; practically every line had a comment. That's not really necessary IMO, but on the flipside, every function more complicated than a getter or setter needs to have a comment. Please, when you modify a function, if you know what it does, comment it! If that function has a complex block within it, comment that too. You'll make everyones' lives easier. Commenting's not really fun, I know, but it'd be a big help in convincing people to contribute.
It would also help someone like me who doesn't really have any c-skills and is not interested in coding to make tiny adjustments here and there for personal copies. Old code was easy to read and understand, now I can't make head or tails about whats going on in the code.
Timo Pietilš is offline   Reply With Quote
Old May 30, 2011, 10:23   #3
Magnate
Angband Devteam member
 
Join Date: May 2007
Location: London, UK
Posts: 5,057
Magnate is on a distinguished road
Send a message via MSN to Magnate Send a message via Yahoo to Magnate
Quote:
Originally Posted by Timo Pietilš View Post
It would also help someone like me who doesn't really have any c-skills and is not interested in coding to make tiny adjustments here and there for personal copies. Old code was easy to read and understand, now I can't make head or tails about whats going on in the code.
I'm afraid this is one of the unfortunate side effects of having a lot of work done on the code by a large team. Each contributor has different coding styles and a different propensity for comments. We have a set of coding guidelines to keep the layout consistent, but that doesn't help with this particular problem.

Take the menu code for example - it was written by someone with way more expertise than me, whose comments were enough for someone with his/her expertise but not enough for me to understand it. So I can't make head or tail of it - but at least we have a decent menu system. The parser code is a very similar example (though its author happens to hang out on IRC so one can ask questions). I think the better you are at programming in C, the less inclined you are to think something needs a comment.

I do try and add comments where I can, but the catch-22 is that I don't touch the uncommented areas of code very much ...
Magnate is offline   Reply With Quote
Old May 30, 2011, 15:15   #4
takkaria
Veteran
 
takkaria's Avatar
 
Join Date: Apr 2007
Posts: 1,936
Donated: $40
takkaria is on a distinguished road
Quote:
Originally Posted by Derakon View Post
I decided to take a stab at allowing ego items to have negative pvals, which requires understanding how the file parser works. This is...not helped by the fact that there are basically no comments for any of the code.
I saw your comment on Trac; the parse_* functions in init2.c follow the form parse_[letter of _info array the info gets parsed into, e.g. e = ego, a = artifact_[letter at the beginning of the line being parsed, e.g. l or n]. Also, for the calls to parser_*, does looking in parser.h help you at all?
__________________
takkaria whispers something about options. -more-
takkaria is offline   Reply With Quote
Old May 30, 2011, 18:20   #5
Derakon
Prophet
 
Derakon's Avatar
 
Join Date: Dec 2009
Posts: 8,943
Derakon is on a distinguished road
Thanks, Takkaria. Knowing the pattern makes those function names much more accessible, but the pattern is not self-evident -- at least, not to me. I added a diff to that ticket that adds a simple comment to each explaining what it does; the comments may be massively redundant, but that's better than inscrutability.
Derakon is offline   Reply With Quote
Reply


Currently Active Users Viewing This Thread: 1 (0 members and 1 guests)
 
Thread Tools
Display Modes

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off

Forum Jump

Similar Threads
Thread Thread Starter Forum Replies Last Post
r1886 bug(s) and comments Psi Development 4 January 7, 2010 15:39
A Few Comments about V Malatar Vanilla 20 January 5, 2010 23:40
3.1.0 Comments/Observations evariste Vanilla 6 January 17, 2009 03:25
3.10 Comments awldune Vanilla 3 January 11, 2009 15:29
squelch comments/complaints ChodTheWacko Vanilla 2 July 28, 2007 18:50


All times are GMT +1. The time now is 08:09.


Powered by vBulletin® Version 3.8.11
Copyright ©2000 - 2019, vBulletin Solutions Inc.