INFO
things not to do - some examples
DESCRIPTION
This is a list of things you shouldn't do in LPC, compiled by
Profezzorn. Most things in here still _work_ it's just that you
shouldn't do them anyway, because you give away the fact that you
can't code.
1) useless functions.
This is a good example of a useless function
void init() { ::init(); }
Since init will be called in the inherited object anyway if it
is removed.
Here is another example:
int query_value() { return 0; }
Since the return value for a function that doesn't exist is always
zero, this function can be removed. Even if this function returned
a value it it could be removed if this object inherited a standard
object, since it most likely has a value variable or a set_value
function. Same thing goes for realm and query_inorout.
2) useless global variables, an example:
object deer;
void reset()
{
if(!present("raindeer"))
{
deer="/std/lib"->make("raindeer");
move_object(deer,this_object());
}
}
In this example the variable 'deer' will use up memory for no good
reason as it doesn't store anything that will be used later. This
variable should be made local, or removed totally. This is how to
make it local:
void reset()
{
object deer;
if(!present("raindeer"))
{
deer="/std/lib"->make("raindeer");
move_object(deer,this_object());
}
}
3) Clone things in reset() before setting up the description of the
room. This is a bad thing, because if the cloning fails, (and it
will eventually) no exits will be added to the room and the mortal
will be stuck there. The reset function in a room should look
something like this:
void reset(int arg)
{
if(!arg)
{
/* set long, short, light and add exits and items here */
}
/* clone monstes and other stuff here, but only if they
* weren't present already
*/
}
4) Don't have call_outs running when there isn't players around.
All call_outs that do call_outs to themselves should either have
REALLY long delays or stop when there hasn't been a player around
for a while. This is to preserve cpu. For walking monsters, this
is done automatically if you use the walk functions in
/obj/walkingmonster.
5) Constant arrays/mappings.
It is tempting to write things like this:
#define FOO_MAP ([ \
"home", "/players/profezzorn/workroom", \
"away", "/players/profezzorn/bungalow", \
])
and then use it like this: FOO_MAP[cmd] in one or more places in
the code. Avoid this! lpmud implements this very inefficiently and
it might take seconds to get the value if the array is large. There
are two simple solutions, either you use a switch() instead, or you
store the mapping in a variable somewhere and use that. (only once
please).
Note that doing:
mapping foo=([
"home", "/players/profezzorn/workroom",
"away", "/players/profezzorn/bungalow", \
]);
(foo is a global variable) is faster, but just as bad if this is
anything but a room. This is because there will be one copy of the
mapping for each clone. (and one for the master object) For large
arrays/mappings this can take quite a lot of memory. Using a
switch() or calling a function in a room or daemon object that
returns the value is safe.
6) sscanf or lower_case on command argument.
Don't do this:
void init() { add_action("buy","buy"); }
int buy(string s)
{
sscanf(s,"%d",s);
/* rest of buy code */
}
If the command "buy" is given without an argument, s will be zero
and the sscanf will fail with an error. Make sure you check that
the arguments to your command functions are nonzero before using
them, this can be done like this;
int buy(string s)
{
if(!s)
{
notify_fail("Buy what?\n");
return 0;
}
sscanf(s,"%d",s);
/* rest of buy code */
}
This warning also applies to functions like present(),
find_living(), find_player() etc. etc.
7) Don't make things too generic.
doing this in a guild object:
string guild;
string query_guild() { return guild; }
int reset(int arg)
{
guild="stupid guild";
}
would be too generic, it wastes codespace and variable space, it
would be simpler, faster and use less memory to write:
query_guild() { return "stupid guild"; }
8) Don't use this_object()->
Function calls to 'this_object()' does NOT need to use the arrow
syntax. It is much neather, smaller and faster to call the
function directly, simply replace the text this_oject()-> with
nothing and all and it will still work. Example:
write(this_object()->short());
_should_ be:
write(short());
9) Don't use find_living()
The function find_living() might be good for tell and locate. BUT
it is not good for attack spells or feelings, since it searches the
whole mud. Just imagine a 'hit' command using find_living... You
do "hit rabbit", and the command fails because it finds a rabbit on
the other side of the mud. Or even worse, it doesn't fail and you
hit the rabbit on the other side of the mud... Use present instead,
then check that it is a living by calling query_living() in the
resulting object. (If any, don't forget to check!) Here is a small
example of how to do it:
int hit(string who)
{
object target;
notify_fail("Hit who?\n");
if(!who) return 0;
target=present(lower_case(who),environment(this_player()));
if(!target) return 0;
if(!target->query_living())
{
write("You don't want to hit that, you could hurt yourself.\n");
return 1;
}
"/std/msg"->msg("\bPRON hit\b$ \2OBJ.\n",this_player(),target);
return 1;
}
Help topics available:
You are guest number 205 since November 2019.
This file was last modified: June 2000.