05 Jan, 2010, jurdendurden wrote in the 1st comment:
Votes: 0
I'm running a quickmud base which I've added a new field to the skills/spells in const.c called requisite. it's set up in merc.h like so:

sh_int   * requisite;				//pre-requisite skill.


Here is an example skill in const.c: (&gsn_dagger is the pre-requisite skill necessary for backstab)


{
"backstab", {5, 5, 1, 5, 5, 5, 9, 9, 9}, {0, 0, 5, 0, 0, 0, 9, 9, 9},
spell_null, TAR_IGNORE, POS_STANDING,
&gsn_backstab, SLOT (0), 0, 24,
"backstab", "!Backstab!", "", FALSE, &gsn_dagger, STAT_DEX},


This compiles fine until I try to edit the function where you actually go to GAIN the skill in skills.c shown below:

if (!ch->pcdata->learned[skill_table[sn].requisite])
{
act ("$N {Ctells{x you, 'You must learn the pre-requisite skill first!'",
ch, NULL, trainer, TO_CHAR);
return;
}


Compiling this returns the error: array subscript is not an integer.
Which is true.

I've also tried it this way:

if (get_skill(ch, skill_table[sn].requisite) < 1)
{
act ("$N {Ctells{x you, 'You must learn the pre-requisite skill first!'",
ch, NULL, trainer, TO_CHAR);
return;
}


Which compiles but with a warning, saying: passing arg2 of 'get_skill' makes integer from pointer without a cast.

I've been fiddling with it for a while now but I just can't seem to comprehend what I need to do to fix this, any ideas?
05 Jan, 2010, JohnnyStarr wrote in the 2nd comment:
Votes: 0
what about:
if ((get_skill(ch, skill_table[sn].requisite)) < 1)


However, if get_skill does not return a number, then you will have to write it differently.
05 Jan, 2010, jurdendurden wrote in the 3rd comment:
Votes: 0
Tried that, and got the same result, but then I took a harder look at const.c, and realized two of my spells were missing the NULL needed in the pre-requisite spot. After popping them in, the code now compiles fine, however the function in skills.c does not work. Having that dagger skill will not allow me to gain backstab. I get my "You must learn the requisite skill" message with or without the dagger skill. What could I be missing?

EDIT: Ok, your line actually DID fix the compile error I was getting, (but not the do_gain function as I stated above). Can you explain why your line allowed the code to compile so that I may have a better understanding? Why did the extra set of parenthesis affect it so?
05 Jan, 2010, jurdendurden wrote in the 4th comment:
Votes: 0
Scratch that I'm still getting the compiler error. I just didn't do a clean make that's why it didn't warn me again. So back to the drawing board, so to speak :)
05 Jan, 2010, jurdendurden wrote in the 5th comment:
Votes: 0
On another note, I referenced how gsn's were set up throughout the mud, and in merc.h it's declared as

sh_int *    pgsn;            		/* Pointer to associated gsn    */


Then in db.c I saw that it had this little for loop to assign them all: (in the boot_db function)

/*
* Assign gsn's for skills which have them.
*/
{
int sn;

for (sn = 0; sn < MAX_SKILL; sn++)
{
if (skill_table[sn].pgsn != NULL)
*skill_table[sn].pgsn = sn;
}
}


Setting up a similar for loop for my requisite pointer seems to do nothing. Ideas?
05 Jan, 2010, Omega wrote in the 6th comment:
Votes: 0
Since your doing a skill lookup on the gsn itself, this is how I suggest you do it. (or something like it)

This should work… SHOULD being the key word.
// this is a hackjob
bool has_requisit(CHAR_DATA *ch, sh_int *gsn) {
if(!ch || !gsn) return false;
for(int sn = 0; sn < MAX_SKILL; sn++) {
if(skill_table[sn].requisit == gsn) {
if(ch->pcdata->learned[sn] > 0)
return true;
}
}
return false;
}


The code should work as follows..

// may be &skill_table, too tired to compile it myself.
if(has_requisit(ch, skill_table[skill_lookup("backstab")].requisit)) }
// do something
}
05 Jan, 2010, jurdendurden wrote in the 7th comment:
Votes: 0
Seems to be giving me the same effect. :( This is odd, I'm going to sleep on it.
05 Jan, 2010, ocson wrote in the 8th comment:
Votes: 0
As the compiler indicated, your trouble is that you were passing a pointer as the second arg to get_skill when it was expecting an integer. Perhaps you meant

if (get_skill(ch, *skill_table[sn].requisite) < 1)
05 Jan, 2010, elanthis wrote in the 9th comment:
Votes: 0
The reason your code failed is that you are using a POINTER to a sh_int, which is not in any way shape or form the same thing as a sh_int. You were using the .requisite structure field as an index to an array, but you cannot index an array with a pointer – you can only index an array with an integer. You could have just added a * dereference operator, e.g.

if (!ch->pcdata->learned[*skill_table[sn].requisite])


and it would have compiled and (mostly) worked. It might have just crashed depending on what you do for skills that have on pre-requisitive; is the pointer set to 0/NULL in that case? If so, you'd have to check to make sure that .requisite was not NULL before dereferencing. Perhaps in one line such as:

if (skill_table[sn].requisite != NULL && !ch->pcdata->learned[*skill_table[sn].requisite])


Darien's example is wrong because it is checking the requisite of the skill against the requisites of other skills, which is not the logic you want. It is basically looking for the first skill with the same pre-requisitive (which may just be itself) and not the actual pre-requisite skill itself.

Alternatively – and this is what I would do – just make the requisite field a regular int, not a pointer, and use it as the index into the skill table. I can't think of any advantage of making it a pointer. You will need a value to represent "has no pre-requisite," which you could just use -1 for. (Again, as with the pointer, make sure you check that the value of requisite is not -1 before using it to index into the skills table.) If you want to have multiple pre-requisites you might keep it a pointer to have it represent an array (terminated by a value of -1 or somesuch, or with a separate length field in the skill structure), but otherwise I wouldn't use a pointer for this.
05 Jan, 2010, David Haley wrote in the 10th comment:
Votes: 0
elanthis said:
I can't think of any advantage of making it a pointer.

Using a direct index into the table is tying your data structure implementation to the conceptual identifier of the skill. You might argue that this is so tied in already that it doesn't matter anymore (and in this case, it's probably true), but IMHO you should identify things by either direct reference (and then you save/load by identifier) or by a special identifier, not some data-structure-dependent artifact. (Note that this is different from having a structure that maps IDs to objects; the array is not such a mapping as it is filled up linearly and ids aren't guaranteed stable across reboots etc.)
05 Jan, 2010, jurdendurden wrote in the 11th comment:
Votes: 0
Thanks to all this is working correctly now. Much appreciation.
06 Jan, 2010, elanthis wrote in the 12th comment:
Votes: 0
Quote
Using a direct index into the table is tying your data structure implementation to the conceptual identifier of the skill. You might argue that this is so tied in already that it doesn't matter anymore (and in this case, it's probably true), but IMHO you should identify things by either direct reference (and then you save/load by identifier) or by a special identifier, not some data-structure-dependent artifact. (Note that this is different from having a structure that maps IDs to objects; the array is not such a mapping as it is filled up linearly and ids aren't guaranteed stable across reboots etc.)


From a purely academic do-this-the-correct-way standpoint, sure, I agree totally. From a practical lets-get-shit-done standpoint… meh. This is C, and adding abstraction that isn't actually directly buying you an actual real advantage is just a huge waste of time and effort. If it were written in Lua or some other dynamic language, each skill would be an object/table and I would just use references to those objects/tables, or some form of lookup token (skill name string for example) that isn't tied to a particular data structure. In this case, though, thinking of the skill table index as the skill's unique identifier is perfectly logical, workable, easy, and safe approach, and it's unlikely that the underlying data structure will ever change.

If the code is written in a clean enough fashion (it isn't in this case), then the index is really just a magic number you pass in to some function to get the actual skill struct pointer, and you can thus change the implementation from an array to anything else and then just treat that number as an arbitrary unique identifier with no changes to anything else in the code. If anything, and especially in C/C++, I'd actually suggest staying away from storing direct pointers/references in most cases, and instead using a unique identifier and a lookup function. Storing direct references makes things really tricky when you want to add/remove data, such as deleting a skill at runtime. In this case it might be trivial to just loop over all skills and unset the prerequisite reference if necessary, but if you end up storing references to skills elsewhere then the amount of work and code to do a simple delete can quickly approach the unmanageable. Perhaps in this case there are no plans to allow runtime addition or deletion of skills so it's not important. For things that are definitely dynamic I would never ever ever store a direct reference. Using a unique ID and requiring the code to lookup that ID and then check the result in order to dereference the entity is much safer and barely more complicated than using raw pointers/references. Some people foolishly think the overhead of the lookups are too great, but lo and behold today's biggest professional games running at 120 FPS with high-end graphics, complex physics, and intricate gameplay are pretty much all using such systems. The overhead is totally and completely negligable in all but the most extreme cases, and no MUD is going to qualify as one of those extreme cases.
06 Jan, 2010, jurdendurden wrote in the 13th comment:
Votes: 0
To elanthis and David: Thank you both for taking the time to clarify what exactly you meant. I'm pretty new in terms of coding when it comes to C, and haven't ever had to deal with pointers in the past, so you can imagine that they can be pretty mysterious to me at times. Once again much appreciation to both of you for slightly demystifying their significance for me.
06 Jan, 2010, elanthis wrote in the 14th comment:
Votes: 0
Pointers are the most complicated part of C for most people, in my experience. I highly recommend you look around for some tutorials that go over them. I think Tyche had something on his site (could be wrong) with a good link on pointers for new C coders.
06 Jan, 2010, David Haley wrote in the 15th comment:
Votes: 0
elanthis said:
In this case, though, thinking of the skill table index as the skill's unique identifier is perfectly logical, workable, easy, and safe approach, and it's unlikely that the underlying data structure will ever change.

Sort of. It makes for funky hacks when you add things to the sorted skill table, precisely because you can't add them to the sorted table! (You'd break all your references by moving things.)

elanthis said:
then the index is really just a magic number you pass in to some function to get the actual skill struct pointer, and you can thus change the implementation from an array to anything else and then just treat that number as an arbitrary unique identifier with no changes to anything else in the code.

Again, only sort of. In this case the number isn't guaranteed to be stable across reboots, so you really can't treat it as a consistent identifier. It's useful for a session at a time, but not as a globally unique identifier suitable for storage.

elanthis said:
If anything, and especially in C/C++, I'd actually suggest staying away from storing direct pointers/references in most cases, and instead using a unique identifier and a lookup function.

Yeah, I agree. Pointers should be avoided unless they're needed, e.g., for performance, but as you say it's exceedingly unlikely that this kind performance difference means anything whatsoever. In any case, I also prefer having a proper identifier and passing that through a lookup function.

elanthis said:
Storing direct references makes things really tricky when you want to add/remove data, such as deleting a skill at runtime.

Funnily enough this can also make the array approach tricky, because you need to keep around the skill object in the sorted skill table so that your binary search still works. If you were to remove it from the skill table, then shifting everything around breaks the references. Indexes into an array aren't that different from pointers, if you think about it. (in pointer arithmetic pseudo-code, &skill == skill_table + skill_index)

Really, in the end of the day, the Dikuland skill table implementation kinda sucks.
06 Jan, 2010, elanthis wrote in the 16th comment:
Votes: 0
Quote
Sort of. It makes for funky hacks when you add things to the sorted skill table, precisely because you can't add them to the sorted table! (You'd break all your references by moving things.)


Quote
Again, only sort of. In this case the number isn't guaranteed to be stable across reboots, so you really can't treat it as a consistent identifier.


I don't follow. What says the table has to be sorted (and not some other data structure), and what says the identifier can't be stable?

Quote
Really, in the end of the day, the Dikuland skill table implementation kinda sucks.


Fixed it for you.
06 Jan, 2010, David Haley wrote in the 17th comment:
Votes: 0
elanthis said:
I don't follow. What says the table has to be sorted (and not some other data structure), and what says the identifier can't be stable?

That's how it is now (disclaimer: in SMAUG, at least, and I'm guessing that its family is the same). It's not that it has to be that way. It should be indexed in some efficient way somewhere so that skills can be easily searched. But if we're already talking about changing somewhat core things like that, we might as well change things to a sane identifier system in the first place.

(Look up the difference between skill numbers and slot numbers in SMAUG to see what kind of mess results from this setup.)
0.0/17