Bug - Fixed Hatter daily deed should check for ability to equip a hat

[Xyzzyn]

I was using r10748 and my character had ca. 10 moxie and a DRINK ME! potion and a filthy knitted dread sack in his inventory. The daily deeds panel offered the Dances with Tweedles buff in the Hatter dropdown list. I tried to get this buff and mafia: tried to equip the FKDS; failed and noted the error; visited the Tea Party anyway, giving me a different buff.

I think a better way would be not to offer unusable hats in the first place, and to stop if a requested hat cannot be equipped.

The attached patch against r10748 is untested. The most I can say for it is that it seems to compile.

 

[roippi]

The only problem that I can think of is that the deed won't update its display once a previously-unusable hat becomes usable. We'd have to attach some sort of listener to stat gains, or something. As you've coded, the display wouldn't update until another hat was gained or lost. Probably wouldn't affect too many people, but it's a consideration.

I do appreciate the spirit of the report and agree that there should be some sort of protection against hat equip failures.

(also, holy geez, today is Submit Patches For Daily Deeds Day, apparently. heheh)

 

[Veracity]

People like and appreciate Daily Deeds. Heck, even I, who resisted them when they were only in the cramped little dropdown on the main Adventure panel, use them now that we have the spiffy Daily Deeds Frame.

 

[roippi]

As you've coded, the display wouldn't update until another hat was gained or lost.

I take that back after looking at the patch in more detail. You added a line to fire the (hats) listener after stat gains. That's not bad, actually, but we could rather fire it when we gain a stat (rather than substat) point. I'll just shuffle that line to elsewhere in ResultProcessor.

People like and appreciate Daily Deeds. Heck, even I, who resisted them when they were only in the cramped little dropdown on the main Adventure panel, use them now that we have the spiffy Daily Deeds Frame.

that heartens me. I am heartened.

 

[Xyzzyn]

Thanks for the review. You’re right, the stat update can happen elsewhere and less often. (In processGainLoss()? But then you have to check for stat loss messages, as well as the stat gain strings already there.)

Also, the Daily Deeds have become a major feature of mafia for me. I’d not be able to keep track of all that stuff otherwise, and it makes a big difference when playing.

P.S. I’ve thought about this a bit more and now think there’s some value in showing the unusable hats, provided that no attempt is made actually to get the buff with one of them, in the sense that one might like to differentiate between buffs which are unavailable because the equipment is missing and buffs which are unavailable temporarily due to stats. If this was implemented, it’d require a change to the hat selection algorithm, to prioritise wearable hats over unwearable ones, and would do away with some of the checks in my patch. It would also benefit from some indication for listed hats/buffs which currently cannot be equipped/obtained. Considering that there are three places (Daily Deeds, hatter(?) command in the CLI and the relay browser) where Tea Party hats/buffs are listed, it might be a rather bigger change than what I’ve initially suggested.

I don’t actually care too much about this as long as getHatBuff() properly aborts, though.

P.P.S. I forgot to change RabbitHoleManager.decorateRabbitHole(), but it’s not critical since it’s up to the player to click through to the party there.

 

[roippi]

r10768. changed a couple of small things, but overall good stuff.