Re: CryptRL 0.6340 first ASCII release

Crypt wrote in news:ebshka$14qv$1@xxxxxxxxxxx:
On 2006-08-15 07:18:10, Brendan Guild wrote:
Crypt wrote in news:ebo5r9$gi0$1@xxxxxxxxxxx:
Any feedback appreciated.
[snip]
If you really mean that 'any' feedback is appreciated, then I
would be pleased to give you feedback on the insides of your
game. Though I am sure that is not what you had in mind and I
would not be at all surprised if that kind of feedback would be
taken more personally than general game comments.

You will see that i don't use packages. I know this is evil but i
started without packages and it's a bit too late to add some now.
It's not a problem for me but i'm sure it will be very ambarassing
for anyone else trying to look inside my code. Sorry about that.

Packages can be helpful in keeping a very large project organized,
but when you have around 100 classes, I don't think it is at the
point of being a serious concern. This is especially true when you
are the only person working on the game.

If I were you, I would be more concerned by the size of the Entite
(or 'Entity') class, which has over 300 public members. I am certain
that this design can be simplified and I will try to come up with a
few suggestions.

I believe that one of the keys to having a clean, readable game is
that each class represent an easily definable set of objects,
something that can be understood at a glance. Part of being easily
definable is having a small interface of a few well understood
functions.

When each class has a small set of carefully defined functions,
people will be easily able to understand your classes, then you can
work on helping them understand your entire game by making the
relationships between classes clear. Inheritance and containment
relationships are two of the most easily recognizable and so they
should be used often. After that, breaking the classes up into
packages where classes have more relationships to other classes
within the same package than to classes of other packages will make
things even easier to follow.

This does not mean that one must never have a large interface; I am
well aware that sometimes it is impossible to avoid, but it is at
least something to strive for. When you add new features to your
game, you should be creating new classes, not new functions within a
class. If your design forces you to always add new functions to your
classes with each new feature, then you should be concerned.

One powerful trick that I have encountered is to represent stats and
attributes of a character or creature by objects instead of by
members. For example:

int getForce()
int getDexterite()
int getConstitution()
int getIntelligence()

int getForce_Mod()
int getDexterite_Mod()
int getConstitution_Mod()
int getIntelligence_Mod()

void setForce_Mod(int m)
void setDexterite_Mod(int m)
void setConstitution_Mod(int m)
void setIntelligence_Mod(int m)

These 12 members can be replaced with just a few:

int get(Attribute a)
void set(Attribute a, int value)

Using this style, force, dexterity, constitution, intelligence, and
their modifiers become objects instead of members, allowing you to
simplify both the interface and the implementation of the class. In
the case of CryptRL, doing this could hugely simplify things.

There are interesting choices to be made with a design like this. You
could have a pair of additional functions:
int getMod(Attribute a)
void setMod(Attribute a)
This reduces the number of objects that you will need and firmly
indicates the connection between an attribute and its modifier. In
CryptRL, it seems that most things have modifiers, so this might be
appropriate.

Another choice is in deciding what the class of attributes should be.
Attributes usually have names, which is perfect for using them as
keys to access the various numbers in each entity, but attribute
objects could also contain formulas that allow one attribute to be
calculated from others. The maximum and minimum values of an
attribute could be in the attribute object, or the attribute could
contain other attributes. If there is an attribute called 'maxHP',
then the 'hp' attribute could hold the 'maxHP' in such a way that the
maximum value for 'hp' will be drawn from 'maxHP'.

On the other hand, do not be afraid of small classes. Small classes
are easy to understand and that is a very good thing. In this sense,
the ideal class has no members at all. So there is no need to make
the attribute class complicated.

CryptRL uses an interesting technique for copying its entity objects.
Every subclass of Entite has a matching member function of Entite
with the word 'clone' prefixing the name of the class. For example,
Creature is a subclass of Entite, so Entite has a member function
called 'cloneCreature'.

This means that Entite must be modified if a new class of entities is
ever introduced and the number of functions in Entite must increase
even further. Both of these things feel highly undesirable to me, but
fortunately they are easily fixed. One merely needs to combine all of
these functions into one:
Entite clone()
And then cast the cloned entity into whichever subclass is
appropriate. If the function name 'clone' is already in use, then
'copy' (or perhaps 'copie') could be used.

In the current design, one cannot clone without knowing the specific
subclass of the entity because all but one of the functions return
null, but with a unified copy function, one can copy an abstract
entity safely.

Once the number of functions in your classes starts plummeting, you
will probably notice functions that have a bit of an awkward fit. In
CryptRL, there are the sign functions, five overloaded functions with
the name 'sign' that apply to various number types and return -1 or 1
depending on the sign of the given number. These are good an useful
functions, but they are declared to be public and static within
classes that have nothing to do with finding signs. It seems odd to
find them as public static members of the Creature class, for
example. Also: Monde (aka 'world'), Personnage (aka 'character'),
Piege (aka 'trap'), and TerrainPanel

I do not mean that the sign functions are not needed by the
implementation of Creature, but there is no need to make them public
so that the users of the Creature class are forced to learn them. Go
crazy with private helper functions, give them strange names, make
dozens of them just because you can, because no one will care, but in
public you surely want your class to be clean and shiny, with an
extremely well thought-out and meaningful set of functions.

Finally, I recommend that you should not be afraid of introducing
more classes if it means reducing the number of public members in
each class. If you have a collection of getters and setters in a
class that can be meaningfully grouped together somehow, then they
can be replaced by a single getter and a single setter which operate
on a new class of objects that contain all of the needed values.

For example:
void setColAscii(Color asc)
Color getColAscii()
void setCharAscii(String asc)
String getCharAscii()

Could be replaced by:
void setSymbol(AsciiSymbol asc)
AsciiSymbol getSymbol()
where AsciiSymbol is a new class of objects that hold both ascii
information and color information. In this case it is a small thing,
but when the number of members is already very large, every little
bit helps.

Hopefully, this can all be viewed as constructive criticism and is
not going to scare people away from publishing their source. I firmly
believe that we can help each other to make the job of creating
roguelikes easier for everyone.
.

Relevant Pages

  • Re: CryptRL 0.6340 first ASCII release
    ... taken more personally than general game comments. ... which has over 300 public members. ... int getDexterite() ... void setDexterite_Mod ...
    (rec.games.roguelike.development)
  • [EXPL] XBlast Local Root Exploit
    ... The game can be played with at least two players and up to four ... A buffer inside XBlast could be overflowed by passing a long $HOME ... void banner; ... void memret (char *, int, int, int); ...
    (Securiteam)
  • _fmemcopy and _fmemset not found Turbo C 2.01
    ... I trying to write a simple game and have run into a dead end. ... I am using the wrong version of turbo c or did i not install it correctly? ... int screen_width; ... void draw_pixel; ...
    (comp.os.msdos.programmer)
  • My code wont compile!
    ... This game is crap but that's what I expect (but of course not the ... void gameover; ... void gotoxy (int x, int y); ... cprintf(" "); ...
    (comp.compilers.lcc)
  • Help in Java swings(internal Frame)
    ... public int getSize() ... public void valueChanged{ ... private JScrollPane scrollPane1; ... public class PeakContainer extends JInternalFrame ...
    (comp.lang.java.programmer)