Implementing the restructure changes

[PowerWyrm]

Have you seen yet that V now has an array of generated levels too? It's called the chunk_list, and so far it only saves the town and restores it, but I imagine you could use it.

I don't think that helps with your current problem, though.

Yeah I'm gonna use that array instead of the cave array, which should simplify things a bit. However at this point in the code (around 03/14), the "cave" variable is still used broadly in V, and this global variable is statically allocated, so V behaves like the old code in my variant. If the "cave" variable was dynamically allocated, all the functions that use it would also crash like in my variant. That's why I'm going to review my code and pass a "cave" pointer every time I can instead of getting the pointer from the player's depth and the cave array. This should not only fix the crashes, but also make the code more readable.

[MattB]

I'm absolutely hooked on this thread - even though there hasn't been a full sentence that I've actually understood. I love Angband.

[PowerWyrm]

Yeah I'm gonna use that array instead of the cave array, which should simplify things a bit. However at this point in the code (around 03/14), the "cave" variable is still used broadly in V, and this global variable is statically allocated, so V behaves like the old code in my variant. If the "cave" variable was dynamically allocated, all the functions that use it would also crash like in my variant. That's why I'm going to review my code and pass a "cave" pointer every time I can instead of getting the pointer from the player's depth and the cave array. This should not only fix the crashes, but also make the code more readable.

In the original code, I've counted over 1000 occurences of "cave_get" calls. After a day of reviewing, I'm down to 383 occurences, and I've still half the code to review. Hopefully, this will help a ton when levels get dynamically allocated.

[PowerWyrm]

In the original code, I've counted over 1000 occurences of "cave_get" calls. After a day of reviewing, I'm down to 383 occurences, and I've still half the code to review. Hopefully, this will help a ton when levels get dynamically allocated.

Down to 197 occurences. I've left aside the spell code, because I don't want to rewrite the whole code (and it's never called during dungeon generation), but this is still impressive.

[PowerWyrm]

Lol... I compile the code, start a caster, play a bit around to see if the cave generation changes work and everything goes fine... until I try to cast a spell, and nothing happens. Seems that the projection code at this point doesn't work... Need to check how this was fixed later on

[PowerWyrm]

Lol... I compile the code, start a caster, play a bit around to see if the cave generation changes work and everything goes fine... until I try to cast a spell, and nothing happens. Seems that the projection code at this point doesn't work... Need to check how this was fixed later on

Wasn't the V code. In my variant, priests can cast useful spells (healing, blessing...) on other players and so spell projections must compute the project path without the "stop" flag so that friendly spells can go past monsters to reach other players. And the modified V code only adds a "grid" when reaching the last square of the projection path... so I was never recording any grid for bolt spells. That's now fixed...

[PowerWyrm]

Currently implementing the new KF flags, I've seen random ability disappear from Cloaks of the Magi. Checking the latest ego_item.txt file, it seems that it has been replaced by random high resistance. Probably not intended.

[PowerWyrm]

Currently moving vault parsing to generate.c, found out that room and vault parsers are never cleaned up, leading to memory leaks.

[PowerWyrm]

Eek! A nasty bug I've found while playing with the dungeon generator (basically generating a ton of levels with the DM character)... In some *rare* circumstances (huge GV + multiple other vaults deep in the dungeon), the object feeling can overflow the u32b it is stored in, leading to incorrect feelings.

My fix: divide the rating by 100.

Two places to correct: place_object()

if (rating > 2500000)
rating = 2500000; /* avoid overflows */
c->obj_rating += (rating / 100) * (rating / 100);

and calc_obj_feeling()

/* Apply a minimum feeling if there's an artifact on the level */
if (c->good_item && x < 641) return 60;

if (x > 160000) return 20;
if (x > 40000) return 30;
if (x > 10000) return 40;
if (x > 2500) return 50;
if (x > 640) return 60;
if (x > 160) return 70;
if (x > 40) return 80;
if (x > 10) return 90;
return 100;

This also has the advantage of aligning more or less object rating with monster rating.

[Nick]

Those are brilliant - in particular, the template cleanup has cut memory leakage by nearly a third.

[PowerWyrm]

Time to fix some typos in object_notice_on_wield():
- randart with negative speed tells "you feel noisier!"
- randart with negative infravision says "your eyes tingle!"

#1: copy/paste error on the flag (OBJ_MOD_SPEED instead of OBJ_MOD_STEALTH) line 807

#2: missing case for negative infravision line 821 ("your eyes ache")

[PowerWyrm]

This has IIRC been reported already, but when a flag is detected over time, the character sheet is not immediately updated.

Fix: in equip_notice_after_time(), add a PR_EQUIP redraw when flag (or its absence) is noticed.

[PowerWyrm]

Found another typo in describe_stats():

for (i = 0; i < N_ELEMENTS(mod_flags); i++)
...
if (object_this_mod_is_visible(obj, i))
...

should be:

for (i = 0; i < N_ELEMENTS(mod_flags); i++)
...
if (object_this_mod_is_visible(obj, mod_flags[i].flag))
...

[AnonymousHero]

Amazing. You are a bug-finding machine!

Coincidentally, I think I've given up on even contemplating "rebasing" Tome 2.x onto the new Vanilla codebase. (Not that I necessarily thought it was sensible to start with -- too much divergence already... and I've gone down the C++ path already, so...)

[PowerWyrm]

Found randart gloves which gave me "You feel mobile!" message when worn, but didn't tell about FA when inspected. The problem is in object_notice_on_wield(), line 827: since OF_FREE_ACT is not OFID_WIELD, it should be detected only if the flag is obvious, that is when you're a mage wielding gloves.

Fix: replace the line with

if (of_has(f, OF_FREE_ACT) && of_has(obvious_mask, OF_FREE_ACT))