SilverDragon - Rare Scanner

SilverDragon - Rare Scanner

20M Downloads

Rework "should this be shown?"

kemayo opened this issue ยท 7 comments

commented

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.

commented

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.

commented

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

commented

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".

commented

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.

  1. Is the mob dead, and the setting to announce dead mobs is disabled? Don't announce.
  2. 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.
  3. Is the setting to announce mobs we've already completed disabled? If so:
    1. Are we announcing because of a vignette? If so, announce.
    2. Do we know about a quest? If so, announce / don't announce based solely on quest completion
    3. Do we know about an achievement? If so:
      1. If an alt has completed this achievement already and the setting to announce alt-completed achievements is disabled, don't announce.
      2. Announce / don't announce based on achievement completion
  4. 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:

  1. No achievement, no quest, no special loot. Vanilla rares, common and a guaranteed green or maybe a hunter-tame. Cata rares, a blue.
  2. No achievement, no quest, special loot. Time-Lost Proto-Drake, Warlords mounts.
  3. Achievement, no quest. BC / Wrath rares.
  4. Quest, no achievement. BfA warfronts -- lots of loot here, too.
  5. Achievement, quest. Most of the post-Mists rares.
  6. 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.

commented

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.

commented

It's already separate, albeit with slightly different criterea, and I have no plans to change that.

commented

New loot development: we now have a distinction between knowable and unknowable (ineffable?) loot. The former is what was already present (mount/toy/pet), but the latter is just random notable items that drop.