21 May, 2009, elanthis wrote in the 21st comment:
Votes: 0
Using #define's with the numeric values is better still, since then you don't have to change the numeric values if 50 places if you ever need to change them. General good rule of thumb is to never use "magic numbers" anywhere outside of simple #define's in C or header constant definitions in C++.
21 May, 2009, David Haley wrote in the 22nd comment:
Votes: 0
That's exactly what people suggested, Elanthis. :rolleyes:
22 May, 2009, Skol wrote in the 23rd comment:
Votes: 0
David Haley said:
Quote
Yeah, bob, do IS_RANGER as the last one Brigan posted, so your macro would be like #define IS_RANGER (ch->class == class_lookup ("ranger"); blah blah blah etc.

Actually using the numeric values is a better solution as it will be quite a bit more efficient.


class_lookup("ranger") would check the table for the name and then return the number as IS_RANGER, but also allow for any changing of the class table without 'oh crap now it things IS_RANGER is a druid…?
It's still defined in merc.h and yields the same number, it just finds that number from the class table instead of a number you tell it to find. Would that somehow be a worse solution? I guess I'm not seeing the logic behind hard coding 19 vs making it find it (like a definition of IS_SWORD would be like v[1] === weapon_lookup ("sword") or a similar check) from a table that you might change.
22 May, 2009, Skol wrote in the 24th comment:
Votes: 0
Although, I suppose it would be less processor intensive as it's just looking at ch->class for an int instead of checking against a table.
22 May, 2009, David Haley wrote in the 25th comment:
Votes: 0
Quote
I'm not seeing the logic behind hard coding 19 vs making it find it (like a definition of IS_SWORD would be like v[1] === weapon_lookup ("sword") or a similar check) from a table that you might change.

The difference is that one solution forces it to do the lookup every single time whereas the other one only makes you change the constant when your class table actually changes – which surely is a relatively rare event, no?
You could get the best of both worlds – assuming things actually changed enough to matter – by having a global variable that is set at boot time while reading from config files.
22 May, 2009, Sandi wrote in the 26th comment:
Votes: 0
If you've ever looked at Merc/RoM's const.c you'd understand that changing the order (i.e., the number) of the classes would only be done by a masochist. :sad:
22 May, 2009, kiasyn wrote in the 27th comment:
Votes: 0
Hades_Kane said:
But yeah, I agree with Sandi, the easiest way for class and race checks are using macros.

I have them defined in my code like:

#define IS_PRIEST(ch)	(  ch->class == 17 )
#define IS_BERSERKER(ch)( ch->class == 18 )
#define IS_MFIGHTER(ch) ( ch->class == 19 )



Shouldn't this read more like

#define IS_PRIEST(ch)	(  (ch)->class == 17 )
#define IS_BERSERKER(ch)( (ch)->class == 18 )
#define IS_MFIGHTER(ch) ( (ch)->class == 19 )


otherwise it wont expand the ch argument and use whatever local ch variable is initialized at the function call?
22 May, 2009, David Haley wrote in the 28th comment:
Votes: 0
What? No, it's fine as is. The parenthesis thing in macros is used for other reasons than "expanding" the argument (what do you mean by that anyhow?).

It's used to ensure proper grouping. Here's is a totally contrived example:

#define MY_MACRO(n) (n * 2)

and then used MY_MACRO like so:

MY_MACRO(2 + 5)

this would expand to 2 + 5 * 2
(which equals 12)
which is quite different from the expected macro expansion, (2+5) * 2
(which equals 14)

I'm not sure what you meant by expanding the argument itself and initializing it to local variables though.
22 May, 2009, elanthis wrote in the 29th comment:
Votes: 0
You should always put () around the macro parameters. You never know when someone might try passing in some_character_array + 4 as the ch parameter to the IS_* macros. ;)

And my point about numeric constants is that a macro like IS_RANGER(ch) ((ch)->class == 17) is bad form and going to increase maintenance over something like IS_CLASS(ch,cl) ((ch)->class == (cl)). That way you need the numeric values defined in one single place (the CLASS_* macros) and your IS_CLASS and other references (character creation, among others) don't need to copy that magic number.
22 May, 2009, David Haley wrote in the 30th comment:
Votes: 0
Quote
And my point about numeric constants is that a macro like IS_RANGER(ch) ((ch)->class == 17) is bad form and going to increase maintenance over something like IS_CLASS(ch,cl) ((ch)->class == (cl)). That way you need the numeric values defined in one single place (the CLASS_* macros) and your IS_CLASS and other references (character creation, among others) don't need to copy that magic number.

Well, I had in mind something more like
#define CLASS_RANGER 123
#define IS_RANGER(ch) ((ch)->class == CLASS_RANGER)

An IS_CLASS macro works too but is of somewhat dubious utility. I mean, ch->class == CLASS_RANGER vs. IS_CLASS(ch, CLASS_RANGER) ……

Quote
You should always put () around the macro parameters. You never know when someone might try passing in some_character_array + 4 as the ch parameter to the IS_* macros. ;)

Well, sure. The point is that the parentheses have to do with grouping, not expanding to use local variables etc.
22 May, 2009, Skol wrote in the 31st comment:
Votes: 0
Quote
The difference is that one solution forces it to do the lookup every single time whereas the other one only makes you change the constant when your class table actually changes – which surely is a relatively rare event, no?

Thanks David, that's exactly what I was pondering in the second post heh.
Quote
If you've ever looked at Merc/RoM's const.c you'd understand that changing the order (i.e., the number) of the classes would only be done by a masochist.

Sandi, rofl, no kidding!!

I wonder… could you set up a for loop that only happens at boot that then does the defines? I haven't seen it done anywhere that I'd noticed. That way it'd be based off the existing race table, but only check once. Perhaps a linked list that generates on boot? It would make the lookup once, but also allow CLASS_RANGER to be an int defined at that time also. (Sorry, I know, still a bit hack here but I'm getting better hehe).
22 May, 2009, David Haley wrote in the 32nd comment:
Votes: 0
Skol said:
I wonder… could you set up a for loop that only happens at boot that then does the defines?

No, you can only set #defines at compile time, you would have to do what I said in post #25 and work with variables.

Skol said:
Perhaps a linked list that generates on boot?

What exactly are you planning on doing with this linked list? :wink:
22 May, 2009, flumpy wrote in the 33rd comment:
Votes: 0
is there no concept of enums in c / c++?

for gods sake, if there is, use them please. my eyes are bleeding :cry: :wink:

fyi i found this:

http://www.cprogramming.com/tutorial/enu...

is this relevant?
22 May, 2009, Skol wrote in the 34th comment:
Votes: 0
Well, kind of thinking outloud, but yeah if we can only set defines at compile, the it'd take a separate program to generate them beforehand. The linked list would be something created from the table, then read to create the defines, but it would be creating an array off an array, so redundant. I was thinking more something that you could change in-game, but that's another tangent heh.

Basically, I was thinking of a way to take the table and generate the defines from it at boot time so it only has to happen once. I've seen some Rom games that have class editors etc, I'm wondering if they do it that way? Or have it write/read to an external file when you make changes?

I know it seems like a lot of work to do simply to have the defines but… something like this:
// in merc.h
#define CLASS_FILE "../src/class_file.h"

// called before compile?
void make_class_defines()
{
FILE *fp;
int race;
char buf[MAX_STRING_LENGTH];

if ((fp == fopen (CLASS_FILE, "w")) == NULL);
{
//blah puke something
return;
}

// For simplicity I added upper_name to the race_table structure.
// I suppose I could simply use UPPER and go the length of 'name'.
for (race = 0; race_table[race].upper_name != NULL; race++)
{
sprintf (buf, "#define CLASS_%25.25s %d\n", race_table[race].upper_name, race);
fprintf (fp, buf);
}
fprintf (fp, "\r\n");
fclose (fp);
log_string ("Race defines written to class_file.h.");
}
22 May, 2009, Skol wrote in the 35th comment:
Votes: 0
flumpy said:
is there no concept of enums in c / c++?

for gods sake, if there is, use them please. my eyes are bleeding :cry: :wink:

fyi i found this:

http://www.cprogramming.com/tutorial/enu...

is this relevant?

Nice link flumpy, I read it and bookmarked it. Thanks!
I've seen a _few_ enums used in Rom code (which mine started as in 96), but not enough.
Generally I have written if checks to keep things within set bounds, but I think enums would work better. (Like in OLC I've set limits based off the game, dice size based on weapon types etc, each could become an enum like say dagger_size etc).
22 May, 2009, David Haley wrote in the 36th comment:
Votes: 0
I think you're kind of going in circles… If the list of classes is known at boot time, it suffices to have one global variable per class and then assign values as you read in the classes. I think you're really make life unnecessarily complicated by trying to generate actual defines.

First identify clearly the problem you're trying to solve, and only then solve it – don't make solutions for problems that aren't clear yet. :wink:

flumpy said:
is there no concept of enums in c / c++?

Yes there is, but it's not really the problem being discussed, namely the reordering of the class table (which would require reordering the enums as well, of course). I still am not sure this is actually a problem, because the class table is reordered extremely infrequently…
22 May, 2009, David Haley wrote in the 37th comment:
Votes: 0
Quote
Generally I have written if checks to keep things within set bounds, but I think enums would work better. (Like in OLC I've set limits based off the game, dice size based on weapon types etc, each could become an enum like say dagger_size etc).

You still need bounds checking especially when taking input from OLC.
22 May, 2009, Skol wrote in the 38th comment:
Votes: 0
Rofl, no kidding David. Was kind of just thinking outloud (in circles hehe).
The thought was that if the table changed, the defines changed without having to manually go and change them.
I did some race re-ordering once (after adding a slew of npc races) and it made me grumpy, that's what prompted the line of thought and possible solution etc.
22 May, 2009, Skol wrote in the 39th comment:
Votes: 0
David Haley said:
Quote
Generally I have written if checks to keep things within set bounds, but I think enums would work better. (Like in OLC I've set limits based off the game, dice size based on weapon types etc, each could become an enum like say dagger_size etc).

You still need bounds checking especially when taking input from OLC.

Man no kidding, there were like NO safeguards in the OLC when I started on it. Hammered on that thing for years and finally it makes sense and keeps builders from haphazardly messing things up.

IE:
// Rom code:
REDIT (redit_mana)
{
ROOM_INDEX_DATA *pRoom;

EDIT_ROOM (ch, pRoom);

if (is_number (argument))
{
pRoom->mana_rate = atoi (argument);
send_to_char ("Mana rate set.\n\r", ch);
return TRUE;
}

send_to_char ("Syntax: mana <#xnumber>\n\r", ch);
return FALSE;
}

//Mine
REDIT( redit_mana )
{
ROOM_INDEX_DATA *pRoom;
char buf[MSL];
int rate;

rate = atoi(argument);

EDIT_ROOM(ch, pRoom);

if (is_number(argument))
{
if ((rate > 250 || rate < -100) && ch->level < MAX_LEVEL)
{
sprintf (buf, "{rREDIT: {yMana rate left at %d percent.{x\r\n"
" Range is -100 to 250.\r\n", pRoom->mana_rate);
send_to_char (buf,ch);
return FALSE;
}
pRoom->mana_rate = rate;
sprintf (buf, "{rREDIT: {yMana rate set to %d percent of normal.{x\r\n", pRoom->mana_rate);
send_to_char (buf,ch);
return TRUE;
}

send_to_char ( "{rREDIT: {ySyntax : mana <-100 to 250>{x\r\n"
" IE: mana 100\r\n", ch);
return FALSE;
}

Countless other places (ok, countable, but a pain to count ;p) as well. Like it doesn't originally check to make sure a weapon type is valid in oedit, simple to add a weapon lookup and if it's NULL spit back a 'hey, that weapon type doesn't exist', along with restating what weapon type you set it to when you change it so that builders don't overlook and type max instead of mace and end up with 0 (dagger or whatever).

Ps. Sorry for the tangent, thinking outloud again (in circles hehe) ;)
22 May, 2009, Davion wrote in the 40th comment:
Votes: 0
Skol said:
I wonder… could you set up a for loop that only happens at boot that then does the defines?


That's exactly how ROM handles its gsn's. You could likely just copy that system and plug it in with ease.
20.0/63