another DaJAngband hacking question

[will_asher]

I try not to ask for coding help here, but I've wasted most of the day trying to figure this out and can't get it to work right.

I'm modifying the object generation code to catch spellbooks and change it into the type of spellbook that the character can use.

Here's what I put into the get_obj_num(int level) function in object2.c
(in the Process probabilities loop, I included the chest part to show exactly where it is in the code)

		/* Hack -- prevent embedded chests */
		if (opening_chest && (k_ptr->tval == TV_CHEST)) continue;
		
		/* DAJ: appropriate spellbooks much more likely to appear than others */
	    /* this is a must-have because of having several spell realms */
	
	    /* check for spellbook tval */
	    if ((k_ptr->tval >= TV_MAGIC_BOOK) && (k_ptr->tval < TV_GOLD))
	    {
           /* is it an appropriate spell book? */
           if (!(k_ptr->tval == cp_ptr->spell_book))
           {
              k_ptr->tval = cp_ptr->spell_book;

              /* Hopefully, this will actually change it into the correct spellbook */
              k_idx = lookup_kind(k_ptr->tval, k_ptr->sval);
              
              /* change the index */
              table[i].index = k_idx;
              
		      /* Get the actual kind */
		      k_ptr = &k_info[k_idx];
           }
        }

I don't know what the table[i] stuff means.

In game, this is what happens:
(playing a ranger who uses nature magic)
I get a "Book of Nature Magic []" with the name of a book of a different spell realm in the []. I can cast spells from it as if it were the nature realm spellbook but the name is wrong (and I can't stand giving up on this piece of code when it's this close to working right).

[Nick]

Originally Posted by will_asher

In game, this is what happens:
(playing a ranger who uses nature magic)
I get a "Book of Nature Magic []" with the name of a book of a different spell realm in the []. I can cast spells from it as if it were the nature realm spellbook but the name is wrong (and I can't stand giving up on this piece of code when it's this close to working right).

It could be just a problem with the description (from object1.c) - I would look at the description code first, anyway, as that should point you toward the problem.

[Pete Mack]

When you make the assignment to k_ptr->tval, is it pointing to a k_info object? If so, k_ptr should technically be declared

object_type const *k_ptr;

and the assignment is a big mistake. (You are corrupting the k_info array!)

Yet more evidence that const correctness is a Good Thing...

[will_asher]

I guess I should give up on changing the wrong type of spellbook into the right one. I really don't know much about arrays or consts. I was kindof trying to BS it into working because I can't make sense out of it without taking a couple years to actually learn C well.
I wish I could delete this thread now..

I guess I'll just do the much easier thing of making it reject it and start over when it is about to create the wrong type of spellbook.

[Pete Mack]

I'm 99.99% sure that my suggestion was the right one. (I took a look at the source, and k_ptr is pointing to k_idx.)

Here's what is happening:
1. k_ptr is a pointer to the object type in question (a value in k_info.)
2. You assign something to the tval:

              k_ptr->tval = cp_ptr->spell_book;

3. You do some other stuff, but in the mean time, the tval of k_info[k_idx] has been changed from one magic type to another. After this, there are duplicate values of (tval, sval) in the k_info array. One is the correct one, the other is the one that just got overwritten. That one has the book name (and color, etc) for the other class.

If you want to do this right, do:

           if (!(k_ptr->tval == cp_ptr->spell_book))
           {
              /* k_ptr->tval = cp_ptr->spell_book;   OH, NO, MR BILL!!!!!!!!!! */

              /* This _will_ locate the correct spellbook. */
              /* NOTE: this probably a mistake for Kelek getting changed to 
               * Word of God, because Kelek is a lot more common, and shallower. */
              k_idx = lookup_kind(cp_ptr->spell_book, k_ptr->sval);
              
              /* change the index */
              table[i].index = k_idx;
              
		      /* Get the actual kind */
		      k_ptr = &k_info[k_idx];
           }

EDIT:
This kind of error wouldn't occur if the various _info arrays were declared const, with non-const overrides during initialization in info2.c.

However, it would mean a lot of code churn to make the various _ptr values agree on const correctness.

[will_asher]

Originally Posted by Pete Mack

I'm 99.99% sure that my suggestion was the right one. (I took a look at the source, and k_ptr is pointing to k_idx.)

According to what you're saying, I was closer than I thought. When you said that I'm corrupting the array I thought that meant that I was doing it completely wrong, but I guess I misunderstood.

Originally Posted by Pete Mack

Here's what is happening:
1. k_ptr is a pointer to the object type in question (a value in k_info.)
2. You assign something to the tval:

              k_ptr->tval = cp_ptr->spell_book;

3. You do some other stuff, but in the mean time, the tval of k_info[k_idx] has been changed from one magic type to another. After this, there are duplicate values of (tval, sval) in the k_info array. One is the correct one, the other is the one that just got overwritten. That one has the book name (and color, etc) for the other class.

If you want to do this right, do:

           if (!(k_ptr->tval == cp_ptr->spell_book))
           {
              /* k_ptr->tval = cp_ptr->spell_book;   OH, NO, MR BILL!!!!!!!!!! */

              /* This _will_ locate the correct spellbook. */
              /* NOTE: this probably a mistake for Kelek getting changed to 
               * Word of God, because Kelek is a lot more common, and shallower. */
              k_idx = lookup_kind(cp_ptr->spell_book, k_ptr->sval);
              
              /* change the index */
              table[i].index = k_idx;
              
		      /* Get the actual kind */
		      k_ptr = &k_info[k_idx];
           }

thanks a lot for the help

Originally Posted by Pete Mack

/* NOTE: this probably a mistake for Kelek getting changed to
* Word of God, because Kelek is a lot more common, and shallower. */

Why didn't I think of that? Maybe I should rethink this and go back to just having it reject the wrong book (for a different reason than not being able to my code to work)..
I have an alchemy/misc realm used by alchemists, thieves, and archers. Books 7, 8, and 9 have the same depth and rarity (depth 60, rarity 2), but one is used only by alchemists, one used only by archers, and one used only by thieves. Obviously that would be bad if a depth 60 9th book was converted to a 9th book of another realm. I really should've thought of that..
I guess if I'm going to make the spells of those three classes that much separate, it would be more correct just to make them each have their own realm with less books in those three realms and then I could use the code you suggested. Or maybe I could just make it reject alchemy realm books if the player is not an alchemist, archer, or thief, and convert the books of other realms to the correct one..

But I can use what I learned here whether I actually use this bit of code or not.

[Pete Mack]

According to what you're saying, I was closer than I thought. When you said that I'm corrupting the array I thought that meant that I was doing it completely wrong, but I guess I misunderstood.

Well, yes and no. "Corrupting" means writing garbage (bogus values) into some place you don't expect. It almost always means a severe bug, but it also usually means that you did something that looks "almost" correct. Sometimes it's very subtle and takes a long time to debug, or even to detect.

[Pete Mack]

I guess if I'm going to make the spells of those three classes that much separate, it would be more correct just to make them each have their own realm with less books in those three realms and then I could use the code you suggested. Or maybe I could just make it reject alchemy realm books if the player is not an alchemist, archer, or thief, and convert the books of other realms to the correct one..

Your idea is fundamentally sound. In V Rogues more or less can't use MB6 and MB9, but nonetheless share realms with mages and rangers. The right way to do it is to make sure the wrong books don't get generated in the first place, or you will start running into frequency errors like the Kelek->Wrath of God problem. Note that this will still cause problems with money, as out-of-depth books will never be found for Warriors. But since you are using eddie's gold system, IIRC, that is a good thing.

[PaulBlay]

Is this sort of thing why find_inven is used in Angband's store.c with

static int find_inven(const object_type *o_ptr)

?

I couldn't work out what that 'const' was for.

[Pete Mack]

@Paul:

Yes. It's a hint that says: the incoming value to this function will never be affected by this function. And if you accidentally do something like the tval problem above, the compiler will complain.

See, for example:
http://en.wikipedia.org/wiki/Const_correctness