Rework "should this be shown?"
kemayo opened this issue ยท 7 comments
There are several areas which independently implement quirks of a "should this get shown?" system.
- Announce decides whether to announce a mob
- Handynotes integration decides which nodes to show, and the icons/colors to use for the nodes once shown
- The broker tooltip decides which to show, and exposes a lot of completion state
These have "completion" as an input. Completion is some inconsistent combination of:
- Is there an achievement?
- Was the achievement done on this character?
- Is there a quest?
- Is there loot?
The UI for "should I show X?" should be reworked. It's reasonable that these different areas might want to show different things, so we probably want a reusable test, like a core:ShouldIgnoreMobBasedOnConfig(id, config)
where config would be a table containing a set of behaviors that can be expressed in an also-reusable config-widget.
This rework depends on a few factors AFAIK.
All the options have the same weight and checking for a return is as simple as looping the configs like so
function core:ShouldIgnoreMobBasedOnConfig(id, config)
local returnThis = true
for i, v in ipairs(config) do
returnThis = config[i](id)
end
return returnThis
end
alternatively
function core:ShouldIgnoreMobBasedOnConfig(id, config)
for i, v in ipairs(config) do
if config[i](id) then
return true
end
end
return false
end
In the second example one could go with a immediate return for true or false, ie if something is false then that is the most important vs true being the most important.
EDIT: just realized the above loops would then need a function attached to each pair in config to use config[i].eval(id)
mountcomplete = {
type = "toggle",
name = "Show mount-achieved mobs",
desc = "Whether to show icons for mobs that drop a mount that you've already learned",
order = 25,
eval = ns.Loot.Status.Mount
},
Some options are "more important" and require special handling via subroutines.
This is what is being done now but without the centralized function handling all of the configs.
All the options depend on each other
Not sure if this one would be combinatory or permutationally implemented but either way a lot of code.
I should note that I have some incomplete work on this from back in October off in a branch, which wouldn't work as is: c8fa3f0. It predates all the loot work and the entire overlay splitting-off. It's also just a fairly straight-up port of the existing check in announce into something reusable.
I can't remember why I dropped it then, so I assume something about it isn't working right. :D
Having added a lot of loot data (ac709e6) in the last release, and now added transmog as a completion element (6ea0365), I think I need to get this reworking done before I can do the next tagged release.
Turns out transmog being factored in for completion noticeably changes the number of rare alerts you get, because it greatly increases the number of mobs that count as "has knowable loot, none of which you can learn".
announce.lua
's implementation is the most complex/complete. It's implemented as a series of checks, any one of which can veto the announcement or shortcut the rest of the steps.
- Is the mob dead, and the setting to announce dead mobs is disabled? Don't announce.
- Is the setting to always announce mobs with known loot that we've already got disabled? If so, and there's any known loot remaining, immediately announce.
- Is the setting to announce mobs we've already completed disabled? If so:
- Are we announcing because of a vignette? If so, announce.
- Do we know about a quest? If so, announce / don't announce based solely on quest completion
- Do we know about an achievement? If so:
- If an alt has completed this achievement already and the setting to announce alt-completed achievements is disabled, don't announce.
- Announce / don't announce based on achievement completion
- Always announce if we get this far
This is blurring a few concepts together. The vignette/quest parts are more related to "is this lootable?" than they are to achievement-completion, and should probably be moved to that section now.
There's a few different broad types of rares which complicate this:
- No achievement, no quest, no special loot. Vanilla rares, common and a guaranteed green or maybe a hunter-tame. Cata rares, a blue.
- No achievement, no quest, special loot. Time-Lost Proto-Drake, Warlords mounts.
- Achievement, no quest. BC / Wrath rares.
- Quest, no achievement. BfA warfronts -- lots of loot here, too.
- Achievement, quest. Most of the post-Mists rares.
- Custom mobs, which have none of these but probably should be special-cased as always-announce.
...so we need to not do something unintuitive in any of these cases with reasonable-seeming settings.
Hey there, do you consider to seperate announce and show? What I mean by that is, in some areas (or just some rares) I dont want to see the map overlay icons but still want to get the alert window.
It's already separate, albeit with slightly different criterea, and I have no plans to change that.