Feature counter for Mayflower Bouquet Drops?
- Thread starter KingFelix
- Start date
Sorry for the necro. I wasn’t sure whether it was okay to duplicate the thread after a set period of time.
I got my hands on a bouquet recently, so I patched this in. Note that with the new Daily Deeds structure, the counter won’t show up until you add , Mayflower Bouquet Drops to your dailyDeedsOptions setting.
I got my hands on a bouquet recently, so I patched this in. Note that with the new Daily Deeds structure, the counter won’t show up until you add , Mayflower Bouquet Drops to your dailyDeedsOptions setting.
roippi
Developer
Well, the patch looks okay, but a couple of comments:
1) When adding new built-in deeds, I have put in framework for incrementing the "version" of daily deeds and thereby automatically adding the deed to people who upgrade from an older version (currently version 0). Look for the code up top that checks dailyDeedsVersion, and my comments in there.
2) I don't think this needs a new built-in deed. Simple deeds that just track a preference can be implemented by the user with custom deeds.
2.5) If you were implementing this as a built-in, you would want to add an item listener and setVisible( false ) if the user does not have the item. Similarly, a custom item deed is capable of doing this.
3) What you might want to do instead of making it be its own deed: consider adding it to the already-existing DropsDaily deed. Use the above advice to set the visibility if the user has the item.
1) When adding new built-in deeds, I have put in framework for incrementing the "version" of daily deeds and thereby automatically adding the deed to people who upgrade from an older version (currently version 0). Look for the code up top that checks dailyDeedsVersion, and my comments in there.
2) I don't think this needs a new built-in deed. Simple deeds that just track a preference can be implemented by the user with custom deeds.
2.5) If you were implementing this as a built-in, you would want to add an item listener and setVisible( false ) if the user does not have the item. Similarly, a custom item deed is capable of doing this.
3) What you might want to do instead of making it be its own deed: consider adding it to the already-existing DropsDaily deed. Use the above advice to set the visibility if the user has the item.
Oh, I see, that’s how it works. I thought it was just a placeholder, for some reason.
Hrm, I see the sense in this. But where do you draw the line for what should be built in and what shouldn’t, given the existing builtins?
D’oh. I’m starting to think I had my eyes closed when I wrote this.
That was my first approach. Then I figured it might be out of place, given that all the others are directly related to individual familiars rather than specific familiar equipment.
Thank you for all the feedback. I’ve put together an updated patch to scratch the bad code itch—I hope—but I’ll probably end up using the custom deeds instead. Let me know if you spot any more errors!
roippi
Developer
I think the litmus test is that if a custom deed can 100% cover the functionality, it shouldn't require a built-in. Of course, legacy deeds are an exception.
Well, it's different in what causes the drops, but functionally there's very little difference. There's a limited number of drops, and you get them from an IOTM. Sure, it's an item instead of a familiar, but that's no biggie. I'd do something like
boolean hf8 = KoLCharacter.findFamiliar( FamiliarPool.BOOTS ) != null;
boolean hi1 = InventoryManager.hasItem( ItemPool.MAYFLOWER_BOUQUET );
...
if( hi1 ) text = text + Preferences.getInteger( "_mayflowerDrops" ) + " flowers";
adjusting the other branching if() statements accordingly.