Implementing the restructure changes

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

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

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

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

[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

[Carnivean]

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

While he finishes doing your job then...

[Nick]

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.

My feeling is that this isn't the case, because the ID_ flags are whether the player knows, not whether the object has. Maybe that's because I am mistaken about how average items were identified before. What more is there to learn after {average}?

[PowerWyrm]

Next problem: ring of resist poison will say "doesn't possess any abilities" until worn.

Obvious: elements are learned on any aware non-artifact jewelry when the item is worn. However, ring of resist poison (and similar) is "easy_know", and therefore appears as "known" when "aware". When displaying info, the "get_known_elements" method only checks against known flags and doesn't check against "easy_know".

Easy fix: replace "obj->el_info[element].flags & EL_INFO_KNOWN" checks by calls to object_element_is_known().

[Carnivean]

My feeling is that this isn't the case, because the ID_ flags are whether the player knows, not whether the object has. Maybe that's because I am mistaken about how average items were identified before. What more is there to learn after {average}?

Previously, from the user perspective, you had to carry an item until the pseudo-id kicked in to recognise that an item was average, at which point you knew all the stats.

I haven't played the new version yet, but from what is being stated, you don't have the wait for pseudo-id step in the new id system, and therefore you're immediately aware of all of the 0 flags from the first turn and the object is identified. Maybe not fully identified from previous bug reports though.

Whether you think that should be the case is another question.

[PowerWyrm]

My feeling is that this isn't the case, because the ID_ flags are whether the player knows, not whether the object has. Maybe that's because I am mistaken about how average items were identified before. What more is there to learn after {average}?

The problem with average items is obvious: there is nothing to learn. That's why they are immediately identified. As I said, the old code was adding a "KNOWN" flag to any identified item, so average items still had to learn that flag by passing the identify check. Now that check isn't even made, since average items are automatically known.

[PowerWyrm]

Next problem: ring of resist poison will say "doesn't possess any abilities" until worn.

Obvious: elements are learned on any aware non-artifact jewelry when the item is worn. However, ring of resist poison (and similar) is "easy_know", and therefore appears as "known" when "aware". When displaying info, the "get_known_elements" method only checks against known flags and doesn't check against "easy_know".

Easy fix: replace "obj->el_info[element].flags & EL_INFO_KNOWN" checks by calls to object_element_is_known().

Hmm this makes me think that this will fix some pricing problems reported against the beta, since "easy_know" items with elements would not report these elements as known in element_power().

[PowerWyrm]

And onto the next problem: ring of the dog shows the -2 to stealth (fixed) and the protection from fear, but also says "you do not know the full extent of this item's power" when everything is obviously known.

Trivial: easy_know is missing on the ring (object.txt).

While at it, the easy_know flag has been set on amulets of inertia, but since they have a variable penalty to speed, the flag should be removed.

[Carnivean]

And onto the next problem: ring of the dog shows the -2 to stealth (fixed) and the protection from fear, but also says "you do not know the full extent of this item's power" when everything is obviously known.

Trivial: easy_know is missing on the ring (object.txt).

While at it, the easy_know flag has been set on amulets of inertia, but since they have a variable penalty to speed, the flag should be removed.

While they make sense, these recommendations are the opposite to 3.5 behaviour.

[PowerWyrm]

While they make sense, these recommendations are the opposite to 3.5 behaviour.

Exactly, that was a bug in 3.5

[PowerWyrm]

Final problem for now: examining an unaware ring of strength says it "affects your strength", while it's obvious you have no knowledge about it.

In the old code, a first step was to collect all known mods, then iterate on the list and display "+x" if the mod is visible and "affects" if the mod is not visible. In the new code, the first step is omitted, so any non visible or non known mod is defaulted to "affects".

Fix: I'm assuming that being not aware is sufficient to mask all the mods, so simply adding a "if not aware return" in describe_stats() should be enough. The behavior for digging will be similar to 3.5, as a ring will not say anything, but an unidentified shovel of digging will.