Implementing the restructure changes

[Nick]

If it's broken rather badly here, and requires an intensive overhaul to get it to work as it used to, wouldn't it be better to just go straight to the rune-based-id? I know that's not an easy decision, but the double work seems like a waste of time.

Shouldn't be too much time - PowerWyrm's done half the work

[Nick]

I think we have a general problem here of me writing ID code like I was used to it from FA/wanted it to be, rather than to fit the way it worked. Thanks for all the analysis, there's going to be some work needed.

[Carnivean]

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.

If it's broken rather badly here, and requires an intensive overhaul to get it to work as it used to, wouldn't it be better to just go straight to the rune-based-id? I know that's not an easy decision, but the double work seems like a waste of time.

[PowerWyrm]

Next problem: ring of accuracy not identified when attacking something.

The reason: in the old code, the "object_attack_plusses_are_visible" and co. functions were checking for aware jewelry with non-variable plusses and returned true in that case; in the new code, the check has been pushed in "object_flavor_aware"... which is called only once, for the first item identified.

Basically, all the new code in "object_flavor_aware" should be also called on any newly created object which flavor is already known. This means in object_prep(). It would be probably nice to pull out the following code and make a function for it:

	/* Charges or food value (pval) and effect now known */
	id_on(obj->id_flags, ID_PVAL);
	id_on(obj->id_flags, ID_EFFECT);

	/* Jewelry with fixed bonuses gets more info now */
	if (tval_is_jewelry(obj)) {
		if (!randcalc_varies(obj->kind->to_h)) 
			id_on(obj->id_flags, ID_TO_H);
		if (!randcalc_varies(obj->kind->to_d))
			id_on(obj->id_flags, ID_TO_D);
		if (!randcalc_varies(obj->kind->to_a))
			id_on(obj->id_flags, ID_TO_A);
		for (i = 0; i < OBJ_MOD_MAX; i++)
			if (!randcalc_varies(obj->kind->modifiers[i]))
				id_on(obj->id_flags, ID_MOD_MIN + i);
	}

Then call this function in "object_flavor_aware" and in "object_prep" for aware objects.

[PowerWyrm]

1) "Worn" bogus inscription on unaware identified jewelry

I've implemented a crappier hack than the one in place: instead of ID_STR, I use ID_WEIGHT for the check, which is set when the object is created (and not used anywhere else I think); when an item is wielded/worn, I simply remove the flag and check for its absence (instead of presence) in "object_was_worn"; to be sure, I've replaced calls to "object_was_worn" with "object_was_worn || object_is_known" in some places.

2) Average items immediately identified

This is because they have no properties at all! So "object_all_flags_are_known" returns true, "object_all_elements_are_known" returns true, "object_all_brands_and_slays_are_known" returns true and "object_all_miscellaneous_are_known" returns true. In the old code, a specific flag was added when an object was fully known (IDENT_KNOWN) and the check in "object_is_known" was made on that flag. At this point, I don't see how to fix this without reverting what was done for that flag and add ID_KNOWN back to the list of ID flags. All the work is done by "object_check_for_ident" which calls "object_notice_everything" when the player has learned everything about the object, which then sets all ID flags as always. And the only thing to do is add back the check on ID_KNOWN in "object_is_known".

bool object_is_known(const struct object *obj)
{
	if (!object_flavor_is_aware(obj)) return FALSE;
	if (!object_all_but_flavor_is_known(obj)) return FALSE;

        return id_has(obj->id_flags, ID_KNOWN);
}

3) Some rings/amulets with fixed modifiers don't show their modifier, some rings/amulets with variable modifiers show their modifier

Strangely, this was automatically fixed when fixing the previous problem, so it was again a problem with the "known" flag.

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

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

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

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)

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

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]

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.

[Nick]

This is brilliant, thanks.

[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 */

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