Implementing the restructure changes

[PowerWyrm]

About improve_attack_modifier(), the previous remark is even more true if RF_HURT_FIRE and RF_HURT_COLD are finally implemented as they should be (double damage in melee if using a BRAND_FIRE/BRAND_COLD weapon). Passing and returning the multiplier would allow using a multiplier of 4 for weak brands and 6 for normal brands instead of 2 and 3.

[PowerWyrm]

Just looking at improve_attack_modifier(), it seems that only the multiplier is used, so using those fancy pointers seem useless and it would probably be better to pass and return the multiplier as a pointer:

		int j;
		const struct brand *b = NULL;
		const struct slay *s = NULL;

		my_strcpy(verb, "hit", sizeof(verb));

		/* Get the best attack from all slays or
		 * brands on all non-launcher equipment */
		for (j = 2; j < player->body.count; j++) {
			struct object *obj = slot_object(player, j);
			if (obj)
				improve_attack_modifier(obj, mon, &b, &s, verb, FALSE, TRUE,
										FALSE);
		}

		improve_attack_modifier(obj, mon, &b, &s, verb, FALSE, TRUE, FALSE);

		dmg = melee_damage(obj, b, s);

becomes

		int j, mult = 1;

		my_strcpy(verb, "hit", sizeof(verb));

		/* Get the best attack from all slays or
		 * brands on all non-launcher equipment */
		for (j = 2; j < player->body.count; j++) {
			struct object *obj = slot_object(player, j);
			if (obj)
				improve_attack_modifier(obj, mon, &mult, verb, FALSE, TRUE,
										FALSE);
		}

		improve_attack_modifier(obj, mon, &mult, verb, FALSE, TRUE, FALSE);

		dmg = melee_damage(obj, mult);

and modify melee_damage() accordingly.

[PowerWyrm]

Brands and slays are broken since 78cf8d.

When making an attack, the best brand/slay is given by calling improve_attack_modifier() on each equipment slot.

pointer best_slay;
for (slots) improve_attack_modifier(best_slay);
multiplier = best_slay->multiplier

Before the restructure, the function was doing:

improve_attack_modifier(pointer best_slay)
{
for (slays) {if (slay->multiplier > best_slay->multiplier) best_slay = slay;}
}

Now it's doing:

improve_attack_modifier(pointer best_slay)
{
int multiplier = 1;
for (slays) {if (slay->multiplier > multiplier) multiplier = slay->multiplier; best_slay = slay;}
}

Basically each call resets the best brand/slay. The result is that the last call wins (in this case the weapon) and off-weapon brands/slays are ignored.

Fix: in improve_attack_modifier(), line 481, the code should be

int best_mult = 1;

if (*brand_used) best_mult = brand_used->multiplier;
else if (*slay_used) best_mult = slay_used->multiplier;

[PowerWyrm]

A copy/paste error in mod_slot_mult(): before the restructure, off-weapon multiplier for SHOTS was 4, it was changed to 3 in f49f44c.

[PowerWyrm]

Digger description leaks info ("it can clear rubble...") when the digger is not identified (or worn).

Fix: in obj_known_digging(), add a check vs object_this_mod_is_visible(OBJ_MOD_TUNNEL).

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

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

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

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

[Nick]

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

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

[PowerWyrm]

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

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

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