Re: Circlemud design issues

From: James Turner (turnerjh@XTN.NET)
Date: 04/20/98


George <greerga@CIRCLEMUD.ORG> writes:

> >char    *one_argument(char *argument, char *first_arg);
>
> Gets one argument, ignoring fill words.

[...]

> >void    half_chop(char *string, char *arg1, char *arg2);
>
> Simple extension of any_one_arg to allow arbitrary number of arguments.
>
> >Five functions (at least) for doing more or less the same thing.  This
>
> They more or less build on each other to do different things.

They are redundant, though.  Too specialized, when more general
functions will do every bit as good and be more useful in other situations.

> You could use arguments for differing functions:
>
> snip(argument, result, S_NOFILL | S_QUOTE);
>
> But that would result in one unreadable function as opposed to many smaller
> functions that have a very defined purpose.

That's not the answer either.  However, there is middle ground, and I
feel I've reached it in my code.  It simplifies the above and makes it
easier to follow what the code does.

> >I've done so in my code and will post a snippet if anyone would like
> >(basically I have a function that splits a string into words, optionally
> >respecting quotes and double quotes, and stores the results globally).
>
> globally?  Speaking of threads...

You won't catch me speaking of threading ;)

It would be simple to make it thread-safe.  That's a non-issue, really.

> >I've got OBJ_DATA, CHAR_DATA, ROOM_DATA, etc.  I believe Envy uses the
> >same naming scheme.  I have a sed script I've used to replace my
> >code's references to the full 'struct char_data' names that I would be
> >glad to give you if you're interested.
>
> Again, I don't see the advantage besides saving the 'struct' typing.

It's more clear what the data actually is.  The struct makes it harder
to follow, makes prototypes and declarations longer, and distracts the
reader from what actually is being declared.

> You lose:
> * the distinct identifier that the function is a spell or command.
> * time because you have to change 30 spells (or 100 player commands) when
>   you need a new argument.

I can make ACMD's for things that aren't commands.  Just because it is
declared that way doesn't mean it IS a command.  Call the functions
do_* or cmd_* or whatnot.  As for changing the spells and commands --
if your making the change in the structure just for one or two
commands, you're probably going about it wrong.  If most commands will
take advamtage of the changes, then you need to go into each function
anyway.

> void spell_create_water(int level, struct char_data *ch, struct char_data
> *victim, struct obj_data *obj)
> {
>
> Hmm...
>
> or
>
> void spell_create_water(int level, CHAR_DATA *ch, CHAR_DATA *ch, OBJ_DATA
> *obj)
> {
>
> Which is easier to maintain?

Either that has the parameters clearly declared in the declaration and
not hidden by a macro, though I prefer the latter.  Be honest, how
likely are you to need to change the declarations without changing the
functions themselves?  Besides,

s/void[         ]+do_\([A-Za-z_]+\)(\(.+\), \(.+\), \(.+\),
\(.+\))/void do_\1(\2, \3, \4, \5, int my_new_param_for_commands)/g

will add parameters to the functions.  A similar sed command for the
spells.  But I don't think it is very likely to need to add parameters
to the command/spell declarations.

> I guess you'd like to type that huge block of prototypes out in
> interpreter.c then?

No.  A prototype generating script, or a sec script, will expand them
in-place to proper functions.

> I'm assuming you don't like CREATE() either then.

CREATE is gone from my code also, replaced with void *mud_calloc(int,
int).  It wasn't that difficult to replace all occurances in the code
with proper functions -- five minutes work with emacs.

> >struct ll_data {
> >  struct ll_data *next;
> >  void *data;
> >};
>
> Lots of unnecessary casting, small objects to increase memory usage by
> alignment issues, ugh.  Then you cannot traverse the list when given a
> structure because it doesn't have a reverse pointer to itself in the list.

Not that much unnecessary casting -- it can be hidden with wrapper
functions and thereby ensure type safety.  As for traversing a list
given an arbitrary member, how often is that necessary?  More likely
is an entry being in more than one list, which would cause the need
for structure members like next, next_in_room, etc.

> >From structs.h:
>    struct char_special_data char_specials;      /* PC/NPC specials */
>    struct player_special_data *player_specials; /* PC specials */
> (*)struct mob_special_data mob_specials;        /* NPC specials */
>
> (*) - Could be a pointer though.

The current arrangement is suboptimal (to put it mildly).  The weak
inheritance would IMO be more efficient, more clear, and make it
easier to expand later on.  Plus, there are way too many embedded
structures anyway.

--
James Turner               turnerjh@xtn.net
                           http://www.vuse.vanderbilt.edu/~turnerj1/


     +------------------------------------------------------------+
     | Ensure that you have read the CircleMUD Mailing List FAQ:  |
     | http://democracy.queensu.ca/~fletcher/Circle/list-faq.html |
     +------------------------------------------------------------+



This archive was generated by hypermail 2b30 : 12/15/00 PST