adonthell-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Adonthell-devel] charedit update


From: Kai Sterker
Subject: Re: [Adonthell-devel] charedit update
Date: Fri, 27 Jul 2007 21:22:16 -0700

On 7/23/07, Andrew Phillips <address@hidden> wrote:

> I understand, and no hurry. I know you're busy enough getting 0.4.0 ready
> for general release.

Finally ... :-).

I had a look at the code and there are a couple of things that I
noticed. Hope I'm not too picky ;-).

1) Practically all of the ability, attribute, property and skill
classes have members name, cost and currentVal/rank in common. Having
this in a common, possibly abstract base class would avoid a lot of
code duplication.

2) Not so sure about that one. It might make sense to give the
property class a pointer to its underlying attribute. If the
implementation of getCurrentVal would be dynamically based on that
attribute, the property would always be up-to-date, even though the
value of the attribute changes.

3) Mostly cosmetic. Not sure if there are any standard naming
conventions in the C++ world. If so, Adonthell doesn't follow them
;-). However, we have our own set of rules that are followed more or
less throughout the codebase. Maybe they would be followed better if
they were documented somewhere ;-). Most importantly, class, method
and function names are all lower case, using underscore (_) as
delimiter between words.

You might find a couple more conventions when reading the code, but I
guess they aren't enforced as strictly, so feel free to consider them
as optional: class members start with an Uppercase letter, as does
each new word; no underscores (i.e. MemberVar). Getters for members
are usually the member name all lowercase (i.e. member_var()), setters
however include the set_ prefix (i.e. set_member_var()).


All in all, I believe implementing number (1) and partially following
(3) would be beneficial for now.

In the long run, I also think that we won't get around using the
standard loading/saving mechanism from Adonthell's base module,
otherwise integration with other parts of the character implementation
will be difficult. But seeing how that requires root access to a Linux
box, you shouldn't worry about that for now. We can easily change it
later.

I could also possibly zip my Windows dev environment and upload it
somewhere, if that would be of some use to you. I start to like how
the combination of CTD 4.0/Eclipse 3.3 has turned out (although I'm
also looking forward to XCode 3.0).

Kai




reply via email to

[Prev in Thread] Current Thread [Next in Thread]