blish-hud / Pathing

[Module] The official pathing module which adds marker and trail support to Blish HUD.
https://blishhud.com/docs/markers/
MIT License
12 stars 10 forks source link

Implement contextual icons with clickable actions #62

Open Pluckerpluck opened 2 years ago

Pluckerpluck commented 2 years ago

Continuing on from your existing work, this enables the contextual icons and makes them clickable. Currently the achievement icon is just informative (that the marker is tied to an achivement), while the wiki icon is clickable, and will open the browser to the wiki.

Do check this works, as I run System.Diagnostics.Process.Start(url); to open the web page, which works fine for me, but is one of those weird things that may not for you.

Tooltips are not yet enabled.


Regarding implementation, it turns out onClick doesn't fire when a field is disabled, but to me it made sense that these icons still be clickable. So at least for now I've overriden OnLeftMouseButtonReleased. It also doesn't check the exact bounds, but instead looks for the click within the 18px window that frames the icon (so no gap between the icons) and uses the full height of the menu item.

I also decided to change your empty Actions into null. This allowed for me to implement a check where a null Action indicates that the icon should have no click functionality, and thus just click through to the underlying menu item. This felt better to me than having an icon that clicking it just did... nothing.

For the URL for the search, I just use the achievement named URL encoded into the same format that GW2 Efficiency uses for their search. I assume this is good in 99% of cases. Absolutely no idea how it handles achievements that share the same name (if they exist).


Anyway, tell me if you want any other changes.

Pluckerpluck commented 2 years ago

This pull request has the side effect of enabling the BasicTooltipText

PathingModule.Instance.Gw2ApiManager.Gw2ApiClient.V2.Achievements.GetAsync(achievementId).ContinueWith((achievementTask) => {
    this.BasicTooltipText = $"[Achievement]\r\n\r\n {DrawUtil.WrapText(GameService.Content.DefaultFont14, achievementTask.Result.Description, 300)}\r\n\r\n{DrawUtil.WrapText(GameService.Content.DefaultFont14, achievementTask.Result.Requirement, 300)}";
}, TaskContinuationOptions.NotOnFaulted);

I don't know enough about this system to know when this BasicTooltip is shown vs the full tooltip. It kind of looks like it only shows in situations where the regular tooltip hasn't finished rendering? But I thought I'd point it out. You'll want to comment out those lines if you wish to disable this.

dlamkins commented 2 years ago

This pull request has the side effect of enabling the BasicTooltipText

PathingModule.Instance.Gw2ApiManager.Gw2ApiClient.V2.Achievements.GetAsync(achievementId).ContinueWith((achievementTask) => {
    this.BasicTooltipText = $"[Achievement]\r\n\r\n {DrawUtil.WrapText(GameService.Content.DefaultFont14, achievementTask.Result.Description, 300)}\r\n\r\n{DrawUtil.WrapText(GameService.Content.DefaultFont14, achievementTask.Result.Requirement, 300)}";
}, TaskContinuationOptions.NotOnFaulted);

I don't know enough about this system to know when this BasicTooltip is shown vs the full tooltip. It kind of looks like it only shows in situations where the regular tooltip hasn't finished rendering? But I thought I'd point it out. You'll want to comment out those lines if you wish to disable this.

Yes, we do not want to include this at all. It predates the implementation of the tooltip that we have now and was for testing the concept early on.

Pluckerpluck commented 2 years ago

Ok. Just removed it. Do you have any icon that you'd think of using for "completed achievement"?

dlamkins commented 2 years ago

Likely a combination of the normal icon + a small ✔️ in the bottom right (like a small version of texture 154979) or something.

This looks pretty good. I can look at the textures and see if I can provide something that fits what I was imagining. I'll also give the build a test.

Regarding process.start for the URLs, yes, that's perfectly fine and exactly what we do elsewhere in Blish HUD to ensure the default browser is used.

Pluckerpluck commented 2 years ago

Lol, there so many tick textures... 154979, 157012, 156950 and 156108.

Anyway I wanted to bring up two things:

The achievement icon uses 155062. There is an alternative with 155061 that might be worth looking into as it's a bit bigger and brighter. Depends how it looks among the others, but something I wanted to note down so I don't forget.

Otherwise we could either use 834012 which is the story mastery completed icon. Though it may result in a sizing mismatch. Would need to test it.

Or something like these two designs: 155061+156950 155062+156950

Both use the 156950 tick (which has a nice outline). The former is using the larger achievement icon, the latter is using the one already used in this pull request.

Thoughts? I can try both out and get screenshots maybe later today or tomorrow.


Then with regards to achievement disabling, would we simply be, by default, unchecking completed achivements when Blish launches regardless of the saved value? (Rather than disabling). This icon would then show you why a marker is disabled.

dlamkins commented 2 years ago

The story master completed icon does look nice if it did fit nicely. The prior of the latter two icons also looks nice (a bit brighter - less dull).

I'm still not sure the best option about the disabling vs. unchecking vs. something else. Trying to think through what makes sense intuitively. Especially for a user trying to make a distinction between an actually unchecked category and one that is completed (despite the icon change 🤔).