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

[Atarlost]

It's not terribly complicated to add TR4 or RF6 or whatever you've run out of space on if you run out of space. That's how we got to TR3 and RF5 in the first place. Just be sure to handle the extra flagset everywhere the others are referenced and note that it breaks savefile compatability.

[will_asher]

Thanks, that helps.

[roustk]

thanks
what's a diff and how to you make/apply it?

On unix systems, 'diff' is a command line utility that identifies the differences between two files or directories. Something similar *should* exist in the tools you are using, but I don't know for sure. diff also exists for svn, where a command line like

svn diff \
[url]http://dev.rephial.org/svn/branches/angband-3.0.8[/url] \
[url]http://dev.rephial.org/svn/branches/angband-3.0.9b[/url]

will generate a list of all changes between 3.0.9 (misnamed) and 3.0.9b.

The output looks like this, for src/defines.h:

Index: src/defines.h
===================================================================
--- src/defines.h       (.../angband-3.0.8)     (revision 836)
+++ src/defines.h       (.../angband-3.0.9b)    (revision 836)
@@ -36,7 +36,7 @@
  * Name of the version/variant and its version string
  */
 #define VERSION_NAME   "Angband"
-#define VERSION_STRING "3.0.9"
+#define VERSION_STRING "3.0.9b"
 
 
 /*
@@ -1411,7 +1411,7 @@
 /*
  * Maximum flow depth when using "MONSTER_FLOW"
  */
-#define MONSTER_FLOW_DEPTH 3
+#define MONSTER_FLOW_DEPTH 32

That is, 3.0.9b has a different VERSION_STRING and a different MONSTER_FLOW_DEPTH. (Lines starting with '-' are the old version, '+' are the replacement version.)

I have attached the output of the 'svn diff' command I noted above. If nothing else, you should be able to read it an manually make the changes indicated to your code.

You probably want to figure out how to use diff (or its equivalent on your system), so that you can more easily put your changes onto 3.1.0 when it comes out. Other variant maintainers have to deal with the problem, so presumably you can find someone to offer more guidance.

Kevin

[will_asher]

thanks
what's a diff and how to you make/apply it?

[roustk]

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.

If it really ends with 0x80000000L, it will likely be difficult. That is the largest flag that can fit in a flag variable. My suggestion: look for an unused value earlier (for example, 0x40000000L is unassigned in the p_ptr->update set, while PU_PANEL is 0x80000000L).

I believe that TR1_* is fully assigned, but there are some free items in TR2_* and TR3_*. Similarly RF1 and RF5 seem to be full, but the other RF flags have some free elements. You should be able to replace an RF#_XXX# flag with your new item, and then tie it into the code like the other items with that same RF number.


Some sequences of definitions end with something other than 0x80000000L -- you should be able to just follow the powers-of-two pattern to add more to the end of those sequences.

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.)

Probably the most reasonable is to generate a diff between your base version (3.0.9, say) and the new base version (3.0.9b), and then try to apply that diff to your patched version. I don't know how much has changed between 3.0.9 and 3.0.9b, but it likely is small enough that you can compensate.

I dont know how you would generate (or apply) the diff on Windows, but you may have already figured that out in distributing your variant.

Kevin

[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.

[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.)

[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

[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

[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.

[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?

[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

[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]

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

[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).