MudBytes
Pages: << prev 1, 2, 3 next >>
Do_who function
gorex
Apprentice




Group: Members
Posts: 21
Joined: Sep 14, 2008

Go to the bottom of the page Go to the top of the page
#16 Posted Sep 23, 2008, 10:51 pm

It's exactly has Fizban has it posted.

Fizban
Wizard




Group: Members
Posts: 606
Joined: Jan 8, 2007

Go to the bottom of the page Go to the top of the page
#17 Posted Sep 23, 2008, 11:03 pm

Here is what I wrote before with one change, I realized in the sprintf I didn't change the %2d to %3s but that is below his error and should be unrelated.
.........................
http://www.tbamud.com/files/tbaMUDcom%20Development%20Crew%20%20Fizban.png
The Builder Academy
4 Dimensions

Skol
Sorcerer






Group: Members
Posts: 465
Joined: May 17, 2006

Go to the bottom of the page Go to the top of the page
#18 Posted Sep 24, 2008, 12:06 am

Vaporize (or comment out /* like this */:
Code (text):
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
/*
         * Figure out what to print for level.
    */
    switch (wch->level)
    {
         case MAX_LEVEL - 0 : rank = "IMP"; break;
     case MAX_LEVEL - 1 : rank = "CRE"; break;
 case MAX_LEVEL - 2 : rank = "SUP"; break;
 case MAX_LEVEL - 3 : rank = "DEI"; break;
 case MAX_LEVEL - 4 : rank = "GOD"; break;
 case MAX_LEVEL - 5 : rank = "IMM"; break;
 case MAX_LEVEL - 6 : rank = "DEM"; break;
 case MAX_LEVEL - 7 : rank = "ANG"; break;
 case MAX_LEVEL - 8 : rank = "AVA"; break;
 case MAX_LEVEL - 9 : rank = "BLD"; break;
 default: rank = strdup(wch->level); break;
         } 


(yeah, that ENTIRE thing)

and also
Code (text):
1
2
3
rank = strdup(wch->level);

See, if you simply want level, you can later call it just
as %d (or %-3d) (with the corresponding being wch->level).

// so later instead of this:
Code (text):
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
  sprintf( buf, "[%3d %6s %s] %s%s%s%s%s%s%s%s\n\r",
     rank,
     wch->race < MAX_PC_RACE ? pc_race_table[wch->race].who_name
     : "    ",
     class,
     wch->incog_level >= LEVEL_HERO ? "(Incog) " : "",
     wch->invis_level >= LEVEL_HERO ? "(Wizi) " : "",
     clan_table[wch->clan].who_name,
     IS_SET(wch->comm, COMM_AFK) ? "[AFK] " : "",
             IS_SET(wch->act, PLR_KILLER) ? "(KILLER) " : "",
             IS_SET(wch->act, PLR_THIEF)  ? "(THIEF) "  : "",
     wch->name,
     IS_NPC(wch) ? "" : wch->pcdata->title );


You'd have:
Code (text):
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
  sprintf( buf, "[%-3d %6s %s] %s%s%s%s%s%s%s%s\n\r",
     wch->level,
     wch->race < MAX_PC_RACE ? pc_race_table[wch->race].who_name
     : "    ",
     class,
     wch->incog_level >= LEVEL_HERO ? "(Incog) " : "",
     wch->invis_level >= LEVEL_HERO ? "(Wizi) " : "",
     clan_table[wch->clan].who_name,
     IS_SET(wch->comm, COMM_AFK) ? "[AFK] " : "",
             IS_SET(wch->act, PLR_KILLER) ? "(KILLER) " : "",
             IS_SET(wch->act, PLR_THIEF)  ? "(THIEF) "  : "",
     wch->name,
     IS_NPC(wch) ? "" : wch->pcdata->title );



For the Class part?

First, change that char const class crap to:
char class[32]; // or bigger than your longest class name etc.

Then THIS part:
Code (text):
1
2
3
class = class_table[wch->class].who_name;

becomes
Code (text):
1
2
3
4
5
sprintf (class, "%32s", class_table[wch->class].who_name != NULL 
class_table[wch->class].who_name : "Unknown"); 
// the second part is simply a safety

OR
Code (text):
1
2
3
sprintf (class, "%32s", class_table[wch->class].who_name); // that works also


Comment out /* these around */
Code (text):
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
switch ( wch->level )
{
  default: break;
  case MAX_LEVEL - 0 : class = "IMP"; break;
  case MAX_LEVEL - 1 : class = "CRE"; break;
  case MAX_LEVEL - 2 : class = "SUP"; break;
  case MAX_LEVEL - 3 : class = "DEI"; break;
  case MAX_LEVEL - 4 : class = "GOD"; break;
  case MAX_LEVEL - 5 : class = "IMM"; break;
  case MAX_LEVEL - 6 : class = "DEM"; break;
  case MAX_LEVEL - 7 : class = "ANG"; break;
  case MAX_LEVEL - 8 : class = "AVA"; break;
  case MAX_LEVEL - 9 : class = "BLD"; break;
  }
} 


Anyway, let me know if that helps, if not, post your _entire_ do_who function and I'll post the fixed one.
.........................
-=-=-=-=-=-=-=-
Skol/Dave/Zivilyn

http://www.ansalonmud.com/images/ansalon_banner_2008.jpg
www.AnsalonMUD.com

Ansalon & Rom Builders Guide
Rom Area Sanctuary

Skol
Sorcerer






Group: Members
Posts: 465
Joined: May 17, 2006

Go to the bottom of the page Go to the top of the page
#19 Posted Sep 24, 2008, 12:07 am

Assuming no one beats me to it heh. ;)
.........................
-=-=-=-=-=-=-=-
Skol/Dave/Zivilyn

http://www.ansalonmud.com/images/ansalon_banner_2008.jpg
www.AnsalonMUD.com

Ansalon & Rom Builders Guide
Rom Area Sanctuary

gorex
Apprentice




Group: Members
Posts: 21
Joined: Sep 14, 2008

Go to the bottom of the page Go to the top of the page
#20 Posted Sep 24, 2008, 7:19 am

I still want IMP, DEM, CRE, blah blah to appear.  However, instead of being under class I want it to show up under level.  So...do who for the IMP should look like
[IMP  Ava  Cle]  Gorex the blah.  Instead of [409 Ava IMP] Gorex the blah.

David Haley
Wizard






Group: Members
Posts: 5,730
Joined: Jun 30, 2007

Go to the bottom of the page Go to the top of the page
#21 Posted Sep 24, 2008, 9:18 am

No need to sprintf when you just want to grab a string constant...

Code (text):
1
2
3
4
5
const char* class;
class = class_table[wch->class].who_name;
if (class == NULL) { class = "Unknown"; }


Note proper placement of const.

The sprintf method given is dangerous anyhow, because
(a) it uses magic numbers for the maximum size
(b) if the maximum size increases, the code will cause a buffer overflow

It's also wasteful because it's not necessary to do any copying when you already have all the strings lying around as constants, and all you want is to remember which constant is relevant.
.........................
-- d.c.h --
BabbleMUD Project (custom codebase)
Legends of the Darkstone (head coder)
http://david.the-haleys.org
.........................

Davion
Idle Hand




Group: Administrators
Posts: 1,188
Joined: May 14, 2006

Go to the bottom of the page Go to the top of the page
#22 Posted Sep 24, 2008, 10:25 am

if you use strdup on rank, make sure you free it
.........................
http://mudbytes.net/mudbytessignature-davion2.png

Fizban
Wizard




Group: Members
Posts: 606
Joined: Jan 8, 2007

Go to the bottom of the page Go to the top of the page
#23 Posted Sep 24, 2008, 12:19 pm

Just curious, maybe it's just me, but after looking at ROMs do_who I'm rather disgusted, it suffers from massive bloat (ie. why the hell are things like (MAX_LEVEL - 1) not defined in a .h file somewhere and things like if ( arg[0] == '\0' ) just seem cryptic to me when compared to if (!*arg) because the latter doesn't confuse people who don't understand numbering in an index (ie. why arg[3] references the 4th character and not the third). Am I the only one that thinks the code is absolutely, messy or is ROM's do_who coded in an acceptable manner in most people's eyes?
.........................
http://www.tbamud.com/files/tbaMUDcom%20Development%20Crew%20%20Fizban.png
The Builder Academy
4 Dimensions

Last edited Sep 24, 2008, 12:20 pm by Fizban
David Haley
Wizard






Group: Members
Posts: 5,730
Joined: Jun 30, 2007

Go to the bottom of the page Go to the top of the page
#24 Posted Sep 24, 2008, 12:26 pm

I agree that the levels should be #defined somewhere, but I disagree about !*arg being so much obviously and absolutely clearer. Actually, I prefer the indexed version... You don't need to worry about confusing people with 3 vs. 4 when you're talking about zero. Additionally, keeping it with characters reinforces the concept that we're talking about strings, not boolean values.

In general though I turn off my style judging when looking at most MUD code... I'd never get anything done if I made the code conform to good style conventions (where good == something a high-standards professional software developer would produce).
.........................
-- d.c.h --
BabbleMUD Project (custom codebase)
Legends of the Darkstone (head coder)
http://david.the-haleys.org
.........................

Skol
Sorcerer






Group: Members
Posts: 465
Joined: May 17, 2006

Go to the bottom of the page Go to the top of the page
#25 Posted Sep 24, 2008, 2:51 pm


DavidHaley said:
No need to sprintf when you just want to grab a string constant...

Code (text):
1
2
3
4
5
const char* class;
class = class_table[wch->class].who_name;
if (class == NULL) { class = "Unknown"; }


Note proper placement of const.

The sprintf method given is dangerous anyhow, because
(a) it uses magic numbers for the maximum size
(b) if the maximum size increases, the code will cause a buffer overflow

It's also wasteful because it's not necessary to do any copying when you already have all the strings lying around as constants, and all you want is to remember which constant is relevant.


I hear you on the sprintf method, one thing though, since they're going to output it via sprintf (line by line) later on in the code, why even mess with it?
Rather, wait until the sprintf statement and simply make it %s, then class_table[wch->class].who_name == NULL ? "Unknown" : class_table[wch->class].who_name. That make sense?
.........................
-=-=-=-=-=-=-=-
Skol/Dave/Zivilyn

http://www.ansalonmud.com/images/ansalon_banner_2008.jpg
www.AnsalonMUD.com

Ansalon & Rom Builders Guide
Rom Area Sanctuary

David Haley
Wizard






Group: Members
Posts: 5,730
Joined: Jun 30, 2007

Go to the bottom of the page Go to the top of the page
#26 Posted Sep 24, 2008, 2:54 pm

IMO having temporary variables makes the printing cleaner. Compare

("...%s...", class_name)

and

("...%s...", ..., class_table[wch->class].who_name == NULL ? "Unknown" : class_table[wch->class].who_name


Since using the temporary variable is very easy (just literally assign the appropriate pointer to class_name) I think the first one is better than the second. It's much more obvious what the intention is: you don't have to actually read what that condition is doing.
.........................
-- d.c.h --
BabbleMUD Project (custom codebase)
Legends of the Darkstone (head coder)
http://david.the-haleys.org
.........................

Skol
Sorcerer






Group: Members
Posts: 465
Joined: May 17, 2006

Go to the bottom of the page Go to the top of the page
#27 Posted Sep 24, 2008, 3:23 pm

Thanks David, I think I can see places in the code where it'd very much make sense to use that approach over the old sprintf (or in lieu of part of it, magic buffer sizes and all that issue heh). It seems we still end up doing the same steps, point to it, make sure it's not NULL, then spit it back out, just that the const char approach doesn't tie up extra (or overrun). That sound about right?

Appreciate the feedback btw, I have been solo as a coder for a few years and it's easy to pick up habits. Was much easier to keep my code clean when I had the pro-programmer looking goin 'eww, fix that, yuck, and go read AB on C again'. heh.
.........................
-=-=-=-=-=-=-=-
Skol/Dave/Zivilyn

http://www.ansalonmud.com/images/ansalon_banner_2008.jpg
www.AnsalonMUD.com

Ansalon & Rom Builders Guide
Rom Area Sanctuary

David Haley
Wizard






Group: Members
Posts: 5,730
Joined: Jun 30, 2007

Go to the bottom of the page Go to the top of the page
#28 Posted Sep 24, 2008, 3:32 pm

Skol said:
It seems we still end up doing the same steps, point to it, make sure it's not NULL, then spit it back out, just that the const char approach doesn't tie up extra (or overrun). That sound about right?

Yup, that's exactly it. Instead of copying the string, you just store a pointer to it. (Unless, of course, you actually want to modify it, in which case you need a copy...)

Skol said:
Appreciate the feedback btw, I have been solo as a coder for a few years and it's easy to pick up habits. Was much easier to keep my code clean when I had the pro-programmer looking goin 'eww, fix that, yuck, and go read AB on C again'. heh.

No problem. :smile: My approach is to try to always write clean cod,e even if it's "just for me" -- that way I avoid the bad habits, and in fact develop good habits. Over time it becomes much easier to write clean code from the first go.
.........................
-- d.c.h --
BabbleMUD Project (custom codebase)
Legends of the Darkstone (head coder)
http://david.the-haleys.org
.........................

gorex
Apprentice




Group: Members
Posts: 21
Joined: Sep 14, 2008

Go to the bottom of the page Go to the top of the page
#29 Posted Sep 24, 2008, 7:35 pm

I feel like all of this still doesn't solve my problem.

David Haley
Wizard






Group: Members
Posts: 5,730
Joined: Jun 30, 2007

Go to the bottom of the page Go to the top of the page
#30 Posted Sep 24, 2008, 9:40 pm

Why aren't you able to do whatever you're trying to do? I thought we were talking about the above because it was getting in your way or you were getting errors because of it...
.........................
-- d.c.h --
BabbleMUD Project (custom codebase)
Legends of the Darkstone (head coder)
http://david.the-haleys.org
.........................

Pages:<< prev 1, 2, 3 next >>

Valid XHTML 1.1! Valid CSS!