freem wrote:As some might have noticed on IRC I am trying to understand the code since 2-3 days (for now, I have only replaced some variables with arrays). Since the code is, pardon me, quite ugly by some places with a lot of hard-coded stuff,
I have to say that I agree with you that it can be rather ugly in places. I think this is partially due to the cruft accumulated by many, many years of development by many, many individuals and varying levels of code review (from the good amount that we have currently to the seemingly nonexistent amount of review of some projects we've inherited from) and we should make every effort to fix.
freem wrote:[...] and since peoples does not seem to be against refactoring, I am thinking care about that design to make it easier to understand for newcomers as I.
If it is accepted, I could take the occasion to enhance internal documentation, by example by adding some doxygen comments here and there.
Go for it! I would suggest making your commits to a new branch, and splitting those commits up either by each individual function or at least by each individual file. (E.g., your commit message might read like "Add Doxygen comment to G_SomeFunction()", or "Add Doxygen comments to functions in g_local.h".) I would suggest calling your branch "doygen", and we could merge it with master periodically. That way nothing breaks accidentally and master doesn't get spammed relentlessly with commit messages.
freem wrote:This will be a long job to complete, but it does not needs to be finished to be useful, I think.
What do you think about that?
I agree with you 100%. The more documentation the better, of course, but some documentation is always better than none.
freem wrote:Any ideas of areas to work, by example?
Whatever area you are interested in working on, I think; it's hard to gauge what will benefit others the most (unless someone else on the team would like to request something specific). I have myself spent many, many hours trying to write Doxygen documentation for another project and got burned out from doing it, which is why it's important that you work on whatever you find to be interesting.
freem wrote:Also, I think I will flood a little this forum (and optionally irc) to show which part of the code I will refactor, to avoid unneeded merging conflicts or dual working.
You should absolutely coordinate with us what areas you are working on, and just for that reason! And yes, IRC is probably the best place to do that.
freem wrote:I guess that to ease this the first point to go is to split those big header files (like g_local.h) to have one header per implementation file (and include them in g_local.h in a first time to avoid breaking things).
Sounds like a good idea to me.
freem wrote:I perfectly know that I have no possibility to use templates and real classes, but OOP is not a language-dependent feature, it is a way to think which I would like to introduce there, because it could really help to add/remove races/buildings/whatever.
Yeah, you mentioned "C++-" in one of your bug reports. Just to be clear (and for those who may not know what we are talking about), I assume you're talking about this pattern:
Code: Select all
typedef struct {
// members
} PseudoClass;
void PseudoClass_DoStuff(PseudoClass *const basicallyTheThisPointer, /* args */);
whereby the first argument always acts as a sort of a this pointer, which is basically how C++ works in practice anyway (though MSVC usually sticks this in ecx instead of pushing it on the stack like a normal arg, but whatever, close enough).
Anyway, if you think that pattern makes sense in certain places—and more importantly, makes the code more easy to understand—by all means, go ahead.