Implementing the restructure changes

[Nick]

2) Before the restructure, an object having no slays/brands was adding dice power to the overall power (instead of the dice power multiplied by the average multiplier of all slays/brands vs monsters). After the restructure, nothing is added. So in fact, the previous line should be

if ((num_brands + num_slays + num_kills) == 0) return p + dice_pwr;

Did you notice

	dice_pwr = damage_dice_power(obj);
	p += dice_pwr;

earlier in object_power?

It actually seems possible to me that dice power is being incorrectly added twice for objects with slays and brands, but this code is quite dense and I need to read it more thoroughly.

[PowerWyrm]

Did you notice

	dice_pwr = damage_dice_power(obj);
	p += dice_pwr;

earlier in object_power?

It actually seems possible to me that dice power is being incorrectly added twice for objects with slays and brands, but this code is quite dense and I need to read it more thoroughly.

Actually the old code was doing this:

1) Compute dice power (simply does power for xdy, independently of brands and slays)

2) Add power once for all objects (to take into account the dice)

3) Compute an averaged "slay" multiplier: 1 for objects with no slays/brands, something greater than 1 for objects with slays/brands

4) Add power again for all objects (multiplier * dice power)

So the dice power was indeed added twice. Was that a bug or not, no idea... But the current code (part 4) now adds 0 for items without brands and 1+x for items with brands. If you don't want to add the power twice for items with slays/brands, you need to substract 1 to the best multiplier when computing the averaged multiplier for slays.

[PowerWyrm]

Another thing that is puzzling me is the new format of the xxx_stuff() methods.

Before:

void update_stuff(struct player *p)
{
...
calc_hitpoints();
...
}

Now:

void update_stuff(struct player_upkeep *upkeep)
{
...
calc_hitpoints();
...
}

However, if you look at the calc_hitpoints() method...

static void calc_hitpoints(void)
{
...
[B]player[/B]->xxx
...
}

So much for using a parameter in the calling method...

Clearly, this should be:

static void calc_hitpoints(struct player *p)
{
...
p->xxx
...
}

void update_stuff(struct player *p)
{
...
calc_hitpoints(p);
...
}

[PowerWyrm]

Before the "object flag apocalypse", ice effects were checking against cold immunity/resistance/vulnerability for damage. Now damage is handled by the corresponding element... and since there is no RES_ICE, you take full damage from ice attacks. This is probably not intended...

Side effects are handled corrrectly (inventory damage if no cold immunity, cuts if no rShards, stunning if no pStun).

Fix in adjust_dam():

	/* If an actual player exists, get their actual resist */
	if (p && p->race) {
                int res_type = GF_ICE? GF_COLD: type;
		
                resist = p->state.el_info[res_type].res_level;

		/* Notice element stuff */
		equip_notice_element(p, res_type);
	}

[Nick]

Clearly, this should be:

static void calc_hitpoints(struct player *p)
{
...
p->xxx
...
}

void update_stuff(struct player *p)
{
...
calc_hitpoints(p);
...
}

Clearly, and the same goes for a bunch of those player-calcs functions.

[PowerWyrm]

Unless I'm mistaken (because I didn't check the rest of the latest code), many flags are not learned when noticing an ego.

Old code:

/* Learn ego flags */
of_union(o_ptr->known_flags, o_ptr->ego->flags);

/* Learn all flags except random abilities */
of_setall(learned_flags);

/* Don't learn random ego extras */
if (kf_has(o_ptr->ego->kind_flags, KF_RAND_SUSTAIN))
    of_diff(learned_flags, OFT_SUST);
if (kf_has(o_ptr->ego->kind_flags, KF_RAND_POWER))
    of_diff(learned_flags, OFT_MISC, OFT_PROT);
if (kf_has(o_ptr->ego->kind_flags, KF_RAND_HI_RES))
    of_diff(learned_flags, OFT_HRES);

of_union(o_ptr->known_flags, learned_flags);

This means noticing an ego makes the following flags learned:
- all ego flags
- all flags except sustains if there's a random sustain, miscs/prots if there's a random ability and high resists if there's a random high resist.

New code:

/* Learn ego flags */
of_union(obj->known_flags, obj->ego->flags);

/* Learn ego element properties (note random ones aren't learned) */
for (i = 0; i < ELEM_MAX; i++)
    if (obj->ego->el_info[i].res_level != 0)
        obj->el_info[i].flags |= EL_INFO_KNOWN;

/* Learn all flags except random abilities */
of_setall(learned_flags);

/* Don't learn random ego extras */
if (kf_has(obj->ego->kind_flags, KF_RAND_SUSTAIN))
    of_diff(learned_flags, OFT_SUST);
if (kf_has(obj->ego->kind_flags, KF_RAND_POWER))
    of_diff(learned_flags, OFT_MISC, OFT_PROT);

of_union(obj->known_flags, learned_flags);

This means noticing an ego makes the following flags learned:
- all ego flags
- all flags except sustains if there's a random sustain, miscs/prots if there's a random ability
- all ego elements

Compared to the old code, the following object elements (which were flags before) are not learned anymore:
- lows (acid, fire, cold, elec)
- ignores and hates
- high resists if there's no random high resist

Here's the code I would suggest to add in object_notice_ego() to bring back the old behavior:

of_union(obj->known_flags, learned_flags);

    /* Learn all element properties except random high resists */
    for (i = 0; i < ELEM_MAX; i++)
    {
        /* Don't learn random ego high resists */
        if ((i >= ELEM_POIS) && (i <= ELEM_DISEN) && kf_has(obj->ego->kind_flags, KF_RAND_HI_RES))
            continue;

        /* Learn all element properties */
        if (obj->el_info[i].res_level)
            obj->el_info[i].flags |= EL_INFO_KNOWN;
    }

This should take into account lows and highs. Concerning ignores/hates, I have no idea yet how it is handled in the latest code, so I cannot say what to do.

[PowerWyrm]

If I'm not mistaken, object_stackable() and object_absorb_merge() don't take into account elements since they were separated from flags. This means for example that two identical "Elvenkind" shields, one with resist poison and one with resist chaos will stack. And IIRC this has already been reported on the "Angband 4.0 beta" thread.

/* Different flags don't stack */
if (!of_is_equal(o_ptr->flags, j_ptr->flags)) return FALSE;

/* XXX el_info must be equal XXX */

    /* Blend all knowledge */
    of_union(o_ptr->known_flags, j_ptr->known_flags);
    of_union(o_ptr->id_flags, j_ptr->id_flags);
    for (i = 0; i < ELEM_MAX; i++)
    {
        if (j_ptr->el_info[i].flags && EL_INFO_KNOWN)
            o_ptr->el_info[i].flags |= EL_INFO_KNOWN;
    }

They don't take into account brands and slays either, but that's less important, because there are no "random" slays in V (there are in my variant, so I'll have to fix that too).

        /* Require identical ego-item types */
        if (o_ptr->ego != j_ptr->ego) return FALSE;

        /* XXX require identical brands XXX */
        /*if (!brands_are_equal(o_ptr->brands, j_ptr->brands, FALSE)) return FALSE;*/

        /* XXX require identical slays XXX */

[Nick]

This is brilliant, thanks.

[PowerWyrm]

Concerning ignores/hates, I have no idea yet how it is handled in the latest code, so I cannot say what to do.

Seems that the following apply:
- ignores and hates are never learned (unless the item is fully identified, or the corresponding element is IDed-by-use)
- "cannot be harmed" is displayed if hate + ignore flags are both set on the object and known
- "can be destroyed" is displayed if hate flag only is set on the object and known

There are two circumstances where ignore and hate flags would be learned:
- a monster (or something -- like a fire trap) projects an element on the character
- a monster projects an element on something on the floor

The first case is handled correctly:
- items equipped are checked vs the element and the element is learned
- items carried are checked vs the element and randomly destroyed, so the player should not learn about ignore flag

The second case is partially handled:
- items are checked vs the element and destroyed if hate flag is on but not ignore flag
- items that ignore the element will give a "is unaffected" message

I think in this case the player should learn about the ignore flag. Note that this was not the case in 3.5 either, so it's not a new issue.

[PowerWyrm]

Seems like there's a bug when adding a brand to a randart.

Code for slays:

		/* If we get the same race, check the multiplier */
		if (streq(s->name, slay_names[pick].name) &&
			(s->race_flag == slay_names[pick].race_flag)) {
			/* Same multiplier or smaller, fail */
			if (slay_names[pick].multiplier <= s->multiplier)
				return FALSE;

			/* Greater multiplier, increase and accept */
			s->multiplier = slay_names[pick].multiplier;
			*name = s->name;
			return TRUE;
		}

This means that if there's a SLAY_XXX, it becomes a KILL_XXX.

Now for brands:

		/* If we get the same one or a smaller multiplier, fail */
		if (streq(b->name, brand_names[pick].name) &&
			(b->element == pick) && 
			(b->multiplier >= mult))
			return FALSE;

This means that if there's a weak brand, a normal brand is added. This will probably lead to some randarts showing "branded with weak fire, fire".

[PowerWyrm]

Removing IDENT_SENSED_THIS_TURN and pack reordering on ID reintroduced an old bug. Suppose you have two unID daggers in the pack in slots a) and b), identifying the dagger in slot b) will say "In your pack: a dagger (+x,+y) (b)", but as soon as you press "i" to see your inventory, the identified dagger will be in slot a).

[PowerWyrm]

PWMAngband survived the object flag apocalypse... now will it survive the ID flag restructure?

Found a little bug that cannot happen in V because there's only one player. If you identify an amulet like Trickery and drop in on the floor, another player that isn't aware of the flavor will see "an Azure Amulet <+5, +3, +6> {worn}". This is because "object_was_worn" checks for ID_STR, and "object_notice_everything" has set that flag.

	/* A hack, OK for now as ID_STR is only gained on wield or identify - NRM */
	return id_has(o_ptr->id_flags, ID_STR) ? TRUE : FALSE;

But we cannot disintinguish the two...

Apart from that, I see some other bugs:
- average items are immediately identified
- some jewelry with fixed plusses are not identified (=Dog), some with variable plusses are identified ("infravision)
- unaware jewelry will leak info about plusses (an Azure Amulet -- it affects infravision) when it should only if worn (+x infravision) or aware (affects infravision)

[fizzix]

PWMAngband survived the object flag apocalypse... now will it survive the ID flag restructure?

Found a little bug that cannot happen in V because there's only one player. If you identify an amulet like Trickery and drop in on the floor, another player that isn't aware of the flavor will see "an Azure Amulet <+5, +3, +6> {worn}". This is because "object_was_worn" checks for ID_STR, and "object_notice_everything" has set that flag.

	/* A hack, OK for now as ID_STR is only gained on wield or identify - NRM */
	return id_has(o_ptr->id_flags, ID_STR) ? TRUE : FALSE;

But we cannot disintinguish the two...

Apart from that, I see some other bugs:
- average items are immediately identified
- some jewelry with fixed plusses are not identified (=Dog), some with variable plusses are identified ("infravision)
- unaware jewelry will leak info about plusses (an Azure Amulet -- it affects infravision) when it should only if worn (+x infravision) or aware (affects infravision)

Any help you can offer on ID would be greatly appreciated. I was going to try looking at the problems with average items first. But I feel that I've only gained a basic understanding of how all this works.

[PowerWyrm]

Any help you can offer on ID would be greatly appreciated. I was going to try looking at the problems with average items first. But I feel that I've only gained a basic understanding of how all this works.

I'm gonna fix all these for my variant. I'll post what I've done here.

[Nick]

Found a little bug that cannot happen in V because there's only one player. If you identify an amulet like Trickery and drop in on the floor, another player that isn't aware of the flavor will see "an Azure Amulet <+5, +3, +6> {worn}". This is because "object_was_worn" checks for ID_STR, and "object_notice_everything" has set that flag.

The correct way to deal with that is probably for each player to have a copy of what they know about each existing item. I am planning to get to that at some point, but it didn't seem worth it when rune-based ID was slated for 4.1 anyway; in hindsight, that was probably a mistake.