Feature - Implemented Add tooltips to Daily Deed Item function

[IronTetsubo]

Hello world!

I've (relatively) recently begun playing KoL, and even more recently was introduced to KoLmafia. In an effort to decipher all of the intricate nooks and crannies of KoLmafia, I dug into the source as there didn't seem to be better documentation out there, and I happened to start with Daily Deeds - which, while it seems to be very helpful in ensuring I don't forget to do the various daily actions, wasn't well documented on what everything did. So, I dug in further and extended "Item"-type daily deeds to include tooltips that now display what the effect is (i.e., legendary beat = +50% item for 20 turns). I'm not a terribly strong java developer and of course this is my first attempt at providing any code, so please forgive me if I've committed any cardinal sins in my attempt here. There are a variety of ways I could have done this, but this seemed to be the least impactful - if, however, there's a better/more preferred way, by all means let me know, I'm happy to rewrite. The patch is off the latest svn (r10748), and of course builds and runs fine. I tested it with the legendary beat, as that was the 1/day item I had. The specifics of what I've done:

- extended parseItemDeed to optionally take a 6th parameter, which then gets set as the tooltip for the button.
- extended ItemDaily to again, take a 6th parameter which gets set as the tooltip.
- extended addComboButton to optionally take a tooltip parameter, which of course then sets the tooltip.
- convert the existing builtin items to have 6 parameters and thus have appropriate tooltips.

So, there you go. Thanks for the amazing work that's gone into KoLmafia so far, it's an great helper for KoL. I look forwarded to learning more about how KoLmafia can help me, as I've barely scratched the surface, and hopefully I can contribute back some as I continue to figure out how it works.

 

[roippi]

Huh. Sure, why not. Just looking at the patch, I don't see anything wrong. I do like the clever bit with how you did

+		public void addComboButton( String command, String displaytext, String tip )
+		{
+			this.addComboButton( command, displaytext ).setToolTipText( tip );
+		}

I'll poke around with this and can probably commit it as coded once I test it out. Of course, now we should extend this functionality to ALL the different deed types - or, at least, to command and skill deeds. Simple deeds can probably sit this one out since they're supposed to be.. simple.

Since you did such a good job on this patch, any chance you want to do that work for me?

 

[IronTetsubo]

Yup, happy to do so - I'll post another patch sometime tomorrow. On a separate note, is it okay for me to add other daily items to the list? the heart of chocolate and the jackass plumber game jump to mind as 1/day items folk might want to include.

 

[roippi]

Yup, happy to do so - I'll post another patch sometime tomorrow. On a separate note, is it okay for me to add other daily items to the list? the heart of chocolate and the jackass plumber game jump to mind as 1/day items folk might want to include.

I'd prefer that we not add built-in deeds for items that can be 100% covered with custom deeds. If an item needs special handling, then it's a good candidate for a built-in.

 

[IronTetsubo]

I'd prefer that we not add built-in deeds for items that can be 100% covered with custom deeds. If an item needs special handling, then it's a good candidate for a built-in.

Er, okay. Just to make sure I'm clear, I'm not suggesting adding special functions, but simply adding in the appropriate item/command/etc. entries to the builtin_deeds array, so other appropriate daily deeds show up when the KoLmafia user has the appropriate item/skill/etc. Certainly, the user could go through and do custom deeds on their own, but why not have them pre-configured in the application? I suspect this is a "philosophy of KoLmafia" thing I don't understand. Or to ask another way, what makes it appropriate to have the legendary beat in the builtin_deeds, but not have the jackass plumber home game? My apologies if I'm missing the obvious here, as I'm very new to KoLmafia and have a lot of learning to do.

Patch attached that extends the tooltip functionality out to Command and Skill (in addition to the existing Item in v1 of the patch) type daily deeds. I also cleaned up some of the comments on the existing functions to make the parameter definitions match up. There seem like a few additional avenues I could purse with this, if it makes sense:
- I could add tooltip functionality to the "COMBO" type. it doesn't show up as a built-in, but folk may want to have tooltips there... Seems less valuable for the custom deeds, since it's a lot more likely the person knows what the deed does.
- I could add tooltip functionality to the remaining special builtins. A good portion of them have them already, but I think a few don't.
- I could extend the preferences dialog to also display the tooltips when configuring the daily deeds panel. I have no idea if this is even reasonable/possible, but seems like it would be very helpful for folk.

Thoughts?

 

[IronTetsubo]

oh, I should add that this patch is now off the latest svn, r10760. I did test the command tooltip addition (on breakfast/daily dungeon), but I don't have the vent rage gland skill yet on my character, so that's untested. It of course does build/run.

 

[roippi]

Or to ask another way, what makes it appropriate to have the legendary beat in the builtin_deeds, but not have the jackass plumber home game?

Those are left in as "legacy" daily deeds. They predated the overhaul of daily deeds to allow custom deeds. If I truly had my way, I'd remove all of those legacy deeds and make every item and skill require custom deeds. But that's just me being tyrannical.

- I could add tooltip functionality to the "COMBO" type. it doesn't show up as a built-in, but folk may want to have tooltips there... Seems less valuable for the custom deeds, since it's a lot more likely the person knows what the deed does.

I think that's cool in theory, but it makes the syntax really messy. Have a look at the check to see if the proper number of parameters were passed, then imagine how we'd do that if each element could have an extra argument.

- I could add tooltip functionality to the remaining special builtins. A good portion of them have them already, but I think a few don't.

I'll see if there's any that stand out as needing it.

- I could extend the preferences dialog to also display the tooltips when configuring the daily deeds panel. I have no idea if this is even reasonable/possible, but seems like it would be very helpful for folk.

Heh, good luck with that. The JDnDBox implementation is not the easiest to work with. I cheer your efforts if you want to take it on though. Heck, if you're looking at that part, people have requested that edge-scrolling be enabled when you're holding an item near the top or bottom of those dialogs.

I'll take a look at your patch and submit it pretty soon. Your original patch was fine so I don't anticipate problems with this one.

 

[IronTetsubo]

Those are left in as "legacy" daily deeds. They predated the overhaul of daily deeds to allow custom deeds. If I truly had my way, I'd remove all of those legacy deeds and make every item and skill require custom deeds. But that's just me being tyrannical.

Well, I certainly don't want to poke at something that's already been hashed to death, but may I ask why? I would think the preference would be to have all the possible "daily deeds" for existing items/skills/etc. configured as built-ins to KoLmafia, vs the opposite of having no (or very few) daily deeds built in, and push the burden of that onto the user. Is the idea that you don't want to have them hardcoded into the java, which then reduces the ability for users to customize them? I would happily take a stab at (and no real ideas behind how to do this yet, need to dig into the code) moving all the non-special deeds into the preferences, effectively turning them into "custom" daily deeds that KoLmafia happens to ship as defaults? And then from there, adding in all the other items/skills/etc. in KoL that are daily-types. Would that be more palatable? Reason I keep pushing, and again, my apologies for doing so - I would prefer to patch KoLmafia to work "out of the box" for me, vs. having to go write the custom calls myself that no one else then benefits from. If I spend a relatively minor additional effort, everyone gets a set of daily deeds that "just works" and happens to include everything out there, which seems like a win-win.

I think that's cool in theory, but it makes the syntax really messy. Have a look at the check to see if the proper number of parameters were passed, then imagine how we'd do that if each element could have an extra argument.

Fair enough - I poked a bit further, and could see including a "master" tooltip at the end for the whole combo function, but yeah, it's uglier. Ah, well - that particular itch isn't terribly nagging, so I'll drop this direction.

I'll see if there's any that stand out as needing it.

Seem to be a few - haven't gone comprehensive yet, but at least from my scanthrough, the nuns, the skate park, the telescope, spading, resting, mojo filters, hot tub, crimbo tree, spice melange, still, tea party (complicated), and then I got bored scanning through. So, if that one makes sense, I'll take a stab at it.

Heh, good luck with that. The JDnDBox implementation is not the easiest to work with. I cheer your efforts if you want to take it on though. Heck, if you're looking at that part, people have requested that edge-scrolling be enabled when you're holding an item near the top or bottom of those dialogs.

I'll take a look at your patch and submit it pretty soon. Your original patch was fine so I don't anticipate problems with this one.

Cool, I'll poke, but that'll drop down a bit lower on my priority list, accordingly.

 

[roippi]

Well, I certainly don't want to poke at something that's already been hashed to death, but may I ask why?

It hasn't really been discussed much; it's pretty much just been my decision, and people let me have my way with the Daily Deeds code

A big issue is the ballooning size of the built-in deeds display on the preference page. If you were to dump every daily-use item into that list, it would become nearly unusable. There is just a short string describing each deed, and deeds of all types are shuffled together. It's very confusing to a new user, and adding all those items would just make it more confusing.

So I very much resist that until something is done to improve the functionality of that display. There should perhaps be individual tabs for item, skill, special, etc deeds. Or maybe radio button filters. Either would work; the problem is again that the JDnDBoxes are not terribly easy to work with. So, if you want to first improve the display of built-in deeds to be tabbed/filterable, I'd say you can add as many items as you want to the item tab.

tea party (complicated)

Hatter has tooltips that tell you what effect each buff gives.

And not everything NEEDS a tooltip. If it's just a tooltip that says "use mojo filter," that's not terribly useful.

 

[IronTetsubo]

It hasn't really been discussed much; it's pretty much just been my decision, and people let me have my way with the Daily Deeds code

A big issue is the ballooning size of the built-in deeds display on the preference page. If you were to dump every daily-use item into that list, it would become nearly unusable. There is just a short string describing each deed, and deeds of all types are shuffled together. It's very confusing to a new user, and adding all those items would just make it more confusing.

So I very much resist that until something is done to improve the functionality of that display. There should perhaps be individual tabs for item, skill, special, etc deeds. Or maybe radio button filters. Either would work; the problem is again that the JDnDBoxes are not terribly easy to work with. So, if you want to first improve the display of built-in deeds to be tabbed/filterable, I'd say you can add as many items as you want to the item tab.


Hatter has tooltips that tell you what effect each buff gives.

And not everything NEEDS a tooltip. If it's just a tooltip that says "use mojo filter," that's not terribly useful.

re: adding more items to the built in deeds - That makes sense, I get why you're concerned about the size. In my continued efforts to make kolmafia work for me, that sounds interesting to hack on. Any more suggestions on what an improved interface would look like? If not, I can throw together some mockups and see what you think. I presume a key is to continue to ensure that the existing builtin/custom deeds are all possible, but redo how the layout and organization works? Any other "design constraints"? Should I open up a new thread about "redesign daily deeds panel" and move the conversation there?

re: needing tooltips, shrug. I can't think of one that doesn't need a tooltip, but I certainly reserve the right to be wrong. For example, the mojo filter - the tooltip isn't "use mojo filter", absolutely that'd be a waste of space - it's "reduce spleen count by 1" for those of us (like me!) who are new(er) to kol and don't know/don't remember what a mojo filter does. And fair enough re hatter, I was just scanning code and didn't look to deeply.

Thanks!

 

[roippi]

patch looks good. r10770

If anyone wants to update the wiki page that'd be swell In fact, it would be mighty useful if someone who is not me would go through and clarify anything they don't understand. I understand everything on the page, but I wrote all the code so that's not terribly surprising.

We could add another field to the AddCustomDeedPanel builder, but I'm not terribly motivated for that. Writing the logic for all those document listeners on that thing is a headache.