Help me make my new variant! (please!)

[will_asher]

I can't think of anything I did in the code off hand that could likely have caused that kind of problem, but this helps me have an idea what to look for.
(I won't have much chance to work on it in the next couple days though.)

[Julian]

The real problem here is that Angband is written in C, and C is a terrible language made entirely of sharp edges.

In particular, it makes you manage memory yourself, and if you screw up, it just sets seemingly-random bits of the program on fire at runtime.

You'€™re probably going to be happier if you take a little time to learn some C separately, rather than trying to learn it on the fly in the middle of a somewhat complicated codebase.

(I don’t have any recommendations; the way I learned C many moons ago is not advisable for people without a programming background. (And not really advised for those who do.))

[will_asher]

...still working on this same problem...
Trying to randint0() zero or a negative number wouldn't cause memory issues, would it?
(I'm guessing that would be a different kind of problem.)

I would think bugs would be easier to find when I have a diff of those changes to look at, but I put several changes in one commit. (I'll have to avoid doing that in the future...)

[Pete Mack]

No, randint is a pure function--that is, it can never corrupt memory. It is arrays and strings that are usually the problem.

[Julian]

Trying to randint0() zero or a negative number wouldn't cause memory issues, would it?
(I'm guessing that would be a different kind of problem.)

It can’t cause a problem at all; it’ll just give you 0 if you call it on anything <= 0

[will_asher]

No, randint is a pure function--that is, it can never corrupt memory. It is arrays and strings that are usually the problem.

Could it be a string that's too long? How do I know how long a string can be?

[Julian]

Could it be a string that's too long? How do I know how long a string can be?

A string can be pretty much arbitrarily long.

If you’re talking about a constant string: “blah blah blah”, that can be long without any risk of memory problems.

If you’re putting strings into arrays (and that’s likely the final destination of strings in Angband), then your string plus the terminating null character cannot be longer than the array, or Bad Things happen. (Also, when working with unicode characters, things are even more complicated, because some characters are bigger than others; I know angband has left the world of straight ASCII, but I haven’t looked to see what’s being done.)

And many of the standard C string manipulation functions won’t stop you. They’ll just happily run off the end of the allocated memory for the array.

Now, it’s better than it used to be, in that there are standard library functions that are safer (strncpy instead of strcpy, for instance), and I’m pretty sure Angband has its own functions to ensure consistent usage, but it’s still easy to screw up.

Just as an example:
char *src = “blah blah blah”;
char *dest;

dest = malloc(strlen(src));
strcpy(dest, src);

Looks good, right? (Aside from the failure to check that malloc() actually returned valid memory, of course.)

but… strlen() returns the number of characters before the terminating null. strcpy() copies the terminating null.
So I just wrote a byte off the end of the array.

If I used the safer function, strncpy():
strncpy(dest, src, strlen(src));
Now it’s safe, until I try to do something else with the copied string. Since now the terminal null wasn’t copied, most of the string functions will keep reading off the end of the array, into whatever memory lies beyond.

These sorts of traps are why I suggested taking some time to learn C in a separate context.

[will_asher]

So a string that's in an array has a maximum length. How do I know how long it can be?

EDIT: ie could making a desc line too long in blow_methods.txt (or another of those txt files) cause a string to be too long?
I know monster.txt and artifact.txt obviously allow plenty of space for descriptions, but I don't know about the other txt files, and memory and string manipulating functions are parts of C where I don't understand much at all. (obviously)

[Nick]

EDIT: ie could making a desc line too long in blow_methods.txt (or another of those txt files) cause a string to be too long?
I know monster.txt and artifact.txt obviously allow plenty of space for descriptions, but I don't know about the other txt files, and memory and string manipulating functions are parts of C where I don't understand much at all. (obviously)

Anything read in from one of the .txt files should be fine, as they are all run through the string_make() function which allocates as much memory as it needs.

[will_asher]

Git question: How do I revert the code back to how it was before the commit with the bug?
Do I have to re-clone the whole thing from github?

EDIT: I think I figured it out.

[will_asher]

Well, I didn't find the bug, but I reverted it back to before the largest commit, made some of the same changes (I'll save some of the crazier stuff I did for later), and it's working now.

[Pete Mack]

Probably a good way to go about it, if you dont want to use memory validation.
But keep testing regularly so you don't get stuck this way again. This is a huge benefit of source control: only check in code that (mostly) works, and you will always have a reasonably stable place to test additional changes. "Mostly", because some bugs invariably sneak in anyway, or were present in the first place.

[will_asher]

Probably a good way to go about it, if you don't want to use memory validation.
But keep testing regularly so you don't get stuck this way again. This is a huge benefit of source control: only check in code that (mostly) works, and you will always have a reasonably stable place to test additional changes. "Mostly", because some bugs invariably sneak in anyway, or were present in the first place.

Yes, I've learned that lesson now.

[Pete Mack]

Oh yeah, and once you finish with this bit of character creation, *check it in*, for the reason above, and because smaller changes make code review by others much easier.

[will_asher]

Trying to figure out the effect handler code...
Can one effect handler call another effect handler?

For instance, I'm adding a potion of Mediocrity. So I have
bool effect_handler_MEDIOC(effect_handler_context_t *context)
in effects.c
with stuff like:

/* Stats (damage if over 16, raise if lower than 12) */
if (player->state.stat_ind[STAT_STR] > 16); <damage stat>
else if (player->state.stat_ind[STAT_STR] < 12); <raise stat>

Can the <damage stat> part of that use effect_handler_DRAIN_STAT() ?
Hmmm, it doesn't look like it because of the "context" stuff. I guess I'll just have to copy the relevant parts of DRAIN_STAT() into it 5 times...
(There's got to be a better way of doing this...)

EDIT: hmmm, I found this in the code for the LOSE_ALL monster attack:
effect_simple(EF_DRAIN_STAT, source_monster(context->mon->midx), "0", STAT_STR, 0, 0, 0, 0, &context->obvious);
Could I have one handler call another this way?