A plea for comments

[Derakon]

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.

[Timo Pietilä]

Originally Posted by Derakon

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.

[Magnate]

Originally Posted by Timo Pietilä

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

[takkaria]

Originally Posted by Derakon

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?

[Derakon]

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.