A question to the dev team regarding new ID-by-use (possible bug)

[PowerWyrm]

Looking at spells1.c, there seems to be some incoherence regarding how object flags are "known" when the character is hit by various elements. For example:
- base elements (GF_FIRE/COLD/ELEC/ACID): immunity, resistance and susceptibility are always noticed in the xxx_dam() functions
- poison (GF_POIS) and other elements: resistance is only noticed if the character actually resists poison (same for other elements)

Some other incoherences of the same type seem to occur for other object flags: for example, sustain flag is always noticed when a character is subject to stat drain; flags are always noticed when triggering a trap from the floor (feather falling, resist poison, ...), but only when the character actually resists the effect when triggering a trap from a chest; drain life is always noticed when a character is attacked in melee by a monster that can drain life, but only when the character actually resists life drain when hit by chaos/nether attacks.

I think the calls to wieldeds_notice_flag() should be more consistent. Any thought?

[PowerDiver]

What version are you using? Have you actually tested and found incorrect learning? I am not saying you are wrong, but it seems strange I would have missed so many. I haven't looked in a long time.

IMO there shouldn't be any tests in the trap code. E.g. there ought to be calls to a function that does poison damage, and that code should do the learning. There should be a call to lower a stat, which should do that learning. Etc. The problem is [IMO] bad code that e.g. tests if you have FA and does not call the paralyzation routine if you have it.

The codebase is haphazard, and things added late are hard to incorporate with good design.

[PowerWyrm]

Just look at the source code from current trunk:

Learning resist poison from a trap

/* Extra spike damage */ 
                                 if (one_in_(2)) 
                                 { 
                                         msg_print("You are impaled on poisonous spikes!"); 
  
                                         dam = dam * 2; 
                                         (void)inc_timed(TMD_CUT, randint1(dam), TRUE); 
  
                                         if (p_ptr->state.resist_pois || p_ptr->timed[TMD_OPP_POIS]) 
                                         { 
                                                 msg_print("The poison does not affect you!"); 
                                         } 
                                         else 
                                         { 
                                                 dam = dam * 2; 
                                                 (void)inc_timed(TMD_POISONED, randint1(dam), TRUE); 
                                         } 
  
                                         wieldeds_notice_flag(OF_RES_POIS); 
                                 }

Learning resist poison from a chest

/* Poison */ 
         if (trap & (CHEST_POISON)) 
         { 
                 msg_print("A puff of green gas surrounds you!"); 
                 if (!(p_ptr->state.resist_pois || p_ptr->timed[TMD_OPP_POIS])) 
                         (void)inc_timed(TMD_POISONED, 10 + randint1(20), TRUE); 
                 else if (p_ptr->state.resist_pois) 
                         wieldeds_notice_flag(OF_RES_POIS); 
         }

All the code is like this: sometimes the actual resistance is tested, sometimes not. In fact, it doesn't matter at all because if you don't resist something... then nothing worn can provide it. So the only thing is that you make a useless call to wieldeds_notice_flag().

[PowerDiver]

The chest code is not buggy as written. The inc_timed on poison ought to do poison learning. If it doesn't, that would be a bug in inc_timed.

The code ought to migrate to no if clause, just call inc_timed without checking resistances, and all checks and damage mods done there. The question is how to migrate slowly, when volunteers don't have the patience to do everything at once. You could take the notice_flag out of the if/else, but then someone might not do some later cleanup. Or maybe it would be easier to clean up. I don't know.