DaJAngband hacking: help with the code (please, I wont ask much!)

[will_asher]

As you probably know from my previous posts, I'm attempting to create a variant while knowing extremely little of the C language. This is possible mainly because of how well the Angband code is organized and commented (thanks again). All I know of the C language is from the tutorial "beej's Guide to C programming" and from looking at the Angband code. (I only read the first few sections of the tutorial thoroughly, but I kindof scanned the rest for the answer to my question and didn't find it.)

I would really like a figure out how to add a new magic realm and I've almost done it. Here's my question:

I can figure out that the following is an either/or statement, but I can't figure out how to turn it into an If/then/elseif statement.

text_out(s_text + s_info[(cp_ptr->spell_book == TV_MAGIC_BOOK) ? spell : spell + PY_MAX_SPELLS].text);

I would guess I would do something like

IF (cp_ptr->spell_book == TV_MAGIC_BOOK) s_info = spell;
ELSE IF (cp_ptr->spell_book == TV_PRAYER_BOOK) s_info = spell + PY_MAX_SPELLS;
ELSE s_info = spell + (PY_MAX_SPELLS * 2);

Obviously, I'm not sure this is right, and even if I was, I wouldn't know how to do the text_out() part. Can someone help me with this please?

thanks in advance

[Pete Mack]

int idx;

idx = spell;
if (cp_ptr->spell_book == TV_PRAYER) idx += PY_MAX_SPELLS ;
else if(cp_ptr->spell_book == TV_NATURE) idx += 2*PY_MAX_SPELLS;
text_out(s_text + s_info[idx]);

(I used TV_NATURE; you should use whatever the name of your new realm is.)

You can't increment s_info(!) It is a constant global variable, and you will really screw things up.

[will_asher]

Thank you!

You can't increment s_info(!) It is a constant global variable, and you will really screw things up.

That much I figured out after another minute of looking at the code. I was right about to edit my message with a new guess when I saw your post.
For your amusement(?): my new guess would have been:

IF (cp_ptr->spell_book == TV_MAGIC_BOOK) idx = spell;
ELSE IF (cp_ptr->spell_book == TV_PRAYER_BOOK) idx = spell + PY_MAX_SPELLS;
ELSE idx = spell + (PY_MAX_SPELLS * 2);

text_out(s_text + s_info[idx].text);

I probably would have remembered to add the "int idx;" part later, but I still wouldn't have gotten it right. I would never have known to take the ".text" part out.

EDIT: I put your suggestion in, and it's not working. It's giving me a "invalid operands to binary +" error message for the text_out line. I don't know what that means.

[Pete Mack]

Yours is just fine. Mine was shorter, but wrong. (I didn't pay attention to the .text in the original.) Use either one, but definitely keep the .text!

[will_asher]

oh.. okay. thanks again.

[roustk]

Just for reference, the ?: ternary operator is called the "conditional operator". K&R has a fairly concise description of it (p. 51, 2nd ed).

"expr1 ? expr2 : expr3" does the following (in order):
1. evaluate eval1.
2. if expr1 is non-zero, evaluate expr2 and use that as the value.
3. otherwise (expr1 is zero), evaluate expr2 and use that as the value.

They point out that "z=max(a,b)" can be implemented in two equivalent ways:

  if (a > b) 
    z = a;
  else
    z = b;

  z = (a > b) ? a : b;

To parse your line of code, you need to know that ?: has a precedence below all other operators (just before assignment expressions).

[roustk]

If you like the ?: expressions, you can do something like this (with lots of whitespace to aid human parsing):

text_out
  ( s_text 
    + s_info
      [ (cp_ptr->spell_book == TV_MAGIC_BOOK) 
        ? spell 
        : (cp_ptr->spell_book == TV_PRAYER_BOOK) 
            ? spell + PY_MAX_SPELLS 
            : spell + PY_MAX_SPELLS*2
      ].text
  );

If you are adding even more classes of books, this is really calling out for a switch:

int s_info_offset;
switch (cp_ptr->spell_book)
{
  case TV_MAGIC_BOOK:
  { 
    s_info_offset = 0;
    break;
  }
  case TV_PRAYER_BOOK:
  { 
    s_info_offset = 1;
    break;
  }
  case TV_NATURE_BOOK:
  { 
    s_info_offset = 2;
    break;
  }
  case TV_WARRIOR_BOOK:
  { 
    s_info_offset = 3;
    break;
  }
}
text_out(s_text + s_info[spell + PY_MAX_SPELLS*s_info_offset].text);

Kevin

[Pete Mack]

Actually, if you define the TV_BOOK stuff in a rational way, you can get by without any conditional code at all

#define TV_MAGIC_BOOK n
#define TV_PRAYER_BOOK n+1
#define TV_NATURE_BOOK n+2

text_out(s_text +s_info[spell + PY_MAX_SPELLS*(p_ptr->spell_book-TV_MAGIC_BOOK)].text

This is the way I would usually do it, but I don't know if you are able to set all the TV_xxx_BOOK as adjacent values.

[roustk]

Actually, if you define the TV_BOOK stuff in a rational way, you can get by without any conditional code at all

This is the way I would usually do it, but I don't know if you are able to set all the TV_xxx_BOOK as adjacent values.

If DaJ is close enough to Vanilla, there is quite a bit of space in the high tvals (MAGIC is 90, PRAYER is 91, and the next (and last) tval is TV_GOLD at 100).

As a matter of philosophy, for a many-developer project like Angband, I tend to believe that minimizing assumptions and redundancy is more important than code size or execution speed (within reason). The code I provided, especially the switch, is reasonably efficient (and I assume that it is involved in spellcasting, so not a bottleneck), has no redundant code within cases (for easy maintenance), and does not make a hidden assumption about the #defines. (For maintainability, I'd actually recommend including an else case or assert that deals with spell_book having an invalid value.)

grepping the source, it appears that the ?: operator is also used to select spell/prayer and cast/recite. If enough types are added, it might make sense to build a const array of these sorts of strings. In that case, have a 0-based spell_book equivalent in struct player_class would be valuable (add a byte to active memory, plus the strings, but save a lookup or math operation).

Kevin

[will_asher]

small question about C code

When C calculates an integer from a formula, does it round to the nearest integer or does it always round up (or down) if there's a decimal?

[PowerDiver]

When C calculates an integer from a formula, does it round to the nearest integer or does it always round up (or down) if there's a decimal?

It truncates towards 0. Both 0.8 and -0.8 would convert to 0.

[roustk]

When C calculates an integer from a formula, does it round to the nearest integer or does it always round up (or down) if there's a decimal?

Both integer division (dividing one int by another) and integer casts ('(int)') simply truncate (and effectively round towards zero). See K&R p. 41 and 45.

Kevin

[roustk]

Both integer division (dividing one int by another) and integer casts ('(int)') simply truncate (and effectively round towards zero).

Also note that integer division truncates immediately, so these give different answers:

  int ans;
  ans = 2/3 + 2/3;    /* ans = (int)(0 + 0) = 0 */
  ans = 2./3 + 2/3.;  /* ans = (int)(0.66 + 0.66) = (int)1.33 = 1 */

Kevin

[will_asher]

New questions:

I've been wondering how to add something to a list with this kind of stuff at the end
0x40000000
0x80000000..
If it's too complicated, I probably won't bother.

I was also wondering if there was some not-too-complicated way of putting the 3.0.9b changes into DaJAngband. (I definetly won't try to get everything from 3.1.0 when it comes out, but I'd like to have the changes for 3.0.9b.)

[PowerDiver]

New questions:

I've been wondering how to add something to a list with this kind of stuff at the end
0x40000000
0x80000000..
If it's too complicated, I probably won't bother.

I was also wondering if there was some not-too-complicated way of putting the 3.0.9b changes into DaJAngband. (I definetly won't try to get everything from 3.1.0 when it comes out, but I'd like to have the changes for 3.0.9b.)

You cannot add to the end of the list. That list is full.
There are probably some items that are obviously unused, and you
should change them instead.