Threat Plates

Threat Plates

30M Downloads

Threat Percentage: Show threat delta

Backupiseasy opened this issue ยท 23 comments

commented
commented

This feature is now available in release 10.3.0 And with that I meant that the bug above, is fixed now

commented

Sorry for not commenting on this in the last few days. Need to fix some bugs currently. I will add this to the 10.3 release. Right now, I think about adding options like raw percentage, scaled percentage and extended (the version above).

commented

@Backupiseasy I have a question about this. So right now, the Threatplates update their threat percentage display values how exactly? According to the API, the only way to get the threat values for a unit is if it has a valid unit ID, right? So currently how does Threatplates do this for NPCs that no one in your group/party/raid is targeting? Pretty much someone has to be targeting something in order to get the threat value for it. Right? or is there another way?

So my point, above you say you think that the above code is not performant enough because it's constantly cycling through all the party/raid member's targets and updating the threat values for them. But isn't that how Threatplates already works? If not, can you explain how it updates the values? Where does it get threat values from NPCs w/o a unit id (untargeted NPCs).

I am really interested in knowing how it works. I think the whole idea about this Threat delta display value would be a HUGELY helpful thing. I would happy to create a PR for this if it's something you would be interested in. But I want to understand the current system first and see if @zekeflynn and/or I could make it perform better.

commented

I've updated my initial comment code with a bunch of fixes for issues from my first draft encountered during raids etc.

Regarding performance, I've noticed no issues at all thus far. Key points to note:

  1. The iteration of the raid only occurs for units that you are the current tanking target of. This is because the native threat API provides enough detail to determine the relative threat difference for mobs you are not tanking via calculation of the standard UnitDetailedThreatSituation response and thus the raid iteration can be avoided entirely.
  2. Threat values are updated on the native event UNIT_THREAT_LIST_UPDATE . This only seems to fire every ~0.5-1 second and seems to be specifically throttled by the WoW API.
  3. Following on from point two, I haven't benchmarked this, but one would assume that the calls to UnitDetailedThreatSituation are fast & efficient lookups from a cached data table that is not constantly updated (but rather throttled as per point 2).
  4. Overall, performance would be of the order of the number of mobs you are the tanking target of (very importantly, NOT the number of unit frames displayed)
commented

@Jakobud the threat values are simply determined by the inbuilt UnitDetailedThreatSituation API

commented

For instance, say the mob is aggro'd onto someone else. If I stun the mob (Hammer of Justice) and proceed to apply and surpass threat, the mob name will turn yellow (because being stunned, it hasn't had the chance to actually switch aggro), and the % threat counter will actually keep climbing up past 100. Presumably because it's still showing in relation to its current target.

@zekeflynn this is because a stunned target doesn't change who it's targeting until after the stun wares off. So, the situation you describe is a quirk of the above as UnitDetailedThreatSituation reports the threat values relative to the mobs active target. Once the stun wares off, provided you are in melee range and at 110%+ (or otherwise 130%+), the mob would then change its target to you, and the UnitDetailedThreatSituation would then report you as 100% threat.

commented

Another solution might be to depend on a threat meter like Details or Skada and get the threat data from there assuming they provide it. Never looked into that.

@Backupiseasy I'm not sure that they would maintain the threat tables of non-targeted mobs. From what I can tell, the ThreatClassic2 lib for example maintains the threat data table for the raid based on the players active target only.

commented

So Threat meter add-ons like Details etc must be iterating over all raid members constantly inorder to get the threat data for everyone right? How would it do that for all targets?

commented

@armstrjare I am having some issues with the code block post posted above.

While as a tank or dps in a group everything is working as intended.
As a dps in a raid everything is working as intended.

However while tanking in a raid it appears that it is not iterating over all raid members? It is only iterating over my primary party members?

I will do more testing to provide better feedback

commented

Looking at the code, I am not sure either. I am currently working on integrating it into the 10.3 branch, but I currently have not enough time to test it.

commented

One minor performance improvement to this: You don't need to check if you are in a raid and determine the number of raid members every single time ThreatPlates are checking for threat do you? Seems like just set those values using other events like when someone leaves the group or when you join a group etc.

commented

@Jakobud this is true, but I don't think it's a material performance impact versus the added code complexity of extracting this out.

@ShafferZee Are you still seeing this issue? Theoretically, it should update on the same cadence as other threat meters and should show the delta of your target vs next highest threat. To be honest, I haven't tested this as a tank in a raid. But, you could verify the behaviour comparing the current target to your threat meter values?

@Backupiseasy having been using this for a while as a tank levelling my Warrior in TBC 5 man's, I'm thinking that it would be helpful to provide some flexibility to how the numbers are displayed/sizing/position to help the tank pick out targets they should execute some abilities on etc. vs ones they should ignore.

I'm more than happy to help get all this as a clean pull request to the main release branch if you need. Let me know.

commented

So, finally I am working on integrating this. Hopefully I can upload an alpha version in the upcoming days. One questing though regarding your calculation which I do not understand:

threatDiff = threatValue - threatValue/(scaledPercentage/100)

scaledPercentage / 100 gives the scaled threat as percentage (like 0.25) as scaledPercanteage is between 0 and 100. Dividing threatValue by this percentage, I do not understand. Shouldn't it be threatValue * (scaledPercentage/100) - to scale threatValue.

Can you just explain how this calculation should work or what it should show?

commented

10000 - 10000*(75/100) = 2500 delta
10000 - 10000/(75/100) = -3333.3 delta

Maybe he got + and - reversed and since the absolute value difference is only 8% of the total he didn't notice.

commented

This calculates the threat delta value to display when the Player is not currently tanking/target of the mob (so we can use the threat API which tells us the %'age we are currently at of the tank's threat)

threatDiff - The amount of threat Player behind (negative) or ahead (positive) of the Current Target/Tank.
threatValue - The raw threat value Player has on the mob.
scaledPercentage - The percentage of threat the Player has of what the Current Target/Tank has (i.e. %'age of way to pulling threat/agro of mob).

Therefore...

threatValueNeededToPull = threatValue/(scaledPercentage/100)
(since.... threatValue = threatValueNeededToPull*(scaledPercentage/100))

Therefore...

threatDifference = threatValueNeededToPull - threatValue
(positive)

Therefore...
threatDiff = threatValue - threatValueNeededToPull
(negative)
i.e. -3000 threat (behind tank)

commented

@armstrjare
Here is a video of the problem I am experiencing. It simply isn't displaying the threatdiff value while in raid and tanking.
https://www.youtube.com/watch?v=1uYpx6-y9YQ

However, while doing additional testing, and not shown in video. If you aren't tanking the current target it will show the threatdiff.
image

Edit:
I think this maybe be because when we are in a party we call the function GetNumSubgroupMembers() which doesn't include the player.

But while in a raid we call the function GetNumGroupMembers() which does include the player.
We need to exclude getting the tValue of the player while performing the for loop.

A quick and dirty way I fixed it was like so...

            if tValue and tValue > topOtherThreat and tValue ~= threatValue then
              topOtherThreat = tValue
              topOtherThreatUnit = format("%s%d", groupType, i)
            end
            if tpValue and tpValue > topOtherThreat and tValue ~= threatValue then
              topOtherThreat = tpValue
              topOtherThreatUnit = format("%spet%d", groupType, i)
            end

Simply do not update the topOtherThreat and topOtherThreatUnit variables if tValue is equal to our own threatValue.

I'll need to do additional testing because I do not know what other ugly flaws might spring up from this bandaid fix.

commented

Here is my current code block I am using with a few of my own personal tastes added.
I'm sure there are various places that could be much better optimized.

  if Settings.ThreatPercentage.Show then
    local isTanking, _, scaledPercentage, _, threatValue = UnitDetailedThreatSituation("player", unit.unitid)

    local DISPLAY_TYPE = "diff"
    local THREAT_DOWNSCALE = 100

    if scaledPercentage then
      if DISPLAY_TYPE == "diff" then
        local threatDiff = 0
        if isTanking then
          -- Determine threat diff by finding the next highest raid/party member threat
          local numGroupMembers = IsInRaid() and GetNumGroupMembers() or GetNumSubgroupMembers()
          local topOtherThreat = 0
          local topOtherThreatUnit = nil
          local groupType = IsInRaid() and "raid" or "party"

          for i = 1, numGroupMembers do
            local _, _, _, _, tValue = UnitDetailedThreatSituation(string_format("%s%d", groupType, i), unit.unitid)
            local _, _, _, _, tpValue = UnitDetailedThreatSituation(string_format("%spet%d", groupType, i), unit.unitid)
            
            if tValue and tValue > topOtherThreat and tValue ~= threatValue then
              topOtherThreat = tValue
              topOtherThreatUnit = format("%s%d", groupType, i)
            end
            if tpValue and tpValue > topOtherThreat and tValue ~= threatValue then
              topOtherThreat = tpValue
              topOtherThreatUnit = format("%spet%d", groupType, i)
            end
          end

          if topOtherThreatUnit then
            threatDiff = threatValue - topOtherThreat
          end
        else
          -- Determine raw threat deficit by scaled % of target threat
          if threatValue == 0 or scaledPercentage == 0 then
            threatDiff = 0
          else
            threatDiff = threatValue - threatValue/(scaledPercentage/100)
          end
        end
        -- Show threat lead
        if threatDiff == 0 then
          threatValue = string_format("%s%d", "+", threatValue/THREAT_DOWNSCALE) or ""
          widget_frame.Percentage:SetText(threatValue)
        else
          local diffText = string_format("%s%d", (threatDiff > 0 and "+" or ""), threatDiff/THREAT_DOWNSCALE) or ""
          widget_frame.Percentage:SetText(diffText)
        end
      else
        widget_frame.Percentage:SetText(string_format("%.0f%%", scaledPercentage))
      end
commented

So, I finally integrated that into the 10.3 alpha version. It's available here for testing: https://github.com/Backupiseasy/ThreatPlates/releases/tag/10.3.0-alpha3.

I changed some code from above (especially how +/- and handled). - now should be shown when tanking (unless the second player on the thread list has more threat than you).

For handling the raid issue, I added a check if the unit queried is the player which seems to be more save than comparing threat values. I also skipped the player's pet(s) in this check, not sure if that is correct or not.

commented

Maybe I'm misinterpreting something, but when using "Threat Value Delta" I don't believe the correct decimal power value is behind displayed correctly?

threat

According to details I have a roughly 4000 threat lead.
Shouldn't the value displayed on the nameplate be -3970?

commented

UnitDetailedThreatSituation returns two threat percentages, raw and scaled. Currently, only scaled is shown. I am working on adding raw as well. That's something that can easily be added. You solution is better, I think, but the for loop, i.e., scanning the whole raid everytime the text is updated (which happens often in combat, for every threat update), might impact performance. Still, it's an interesting idea and it could be added as an option.
Another solution might be to depend on a threat meter like Details or Skada and get the threat data from there assuming they provide it. Never looked into that.

commented

I was looking for a feature like this so have had a play around. Here's what I've come up with so far which replaces the Threat %'age with a + or - delta from the next highest threat (if tanking) or the threat delta required to pull threat (if not tanking).

I will see how this goes performance wise in dungeons and raids, and open to any feedback or suggestions, before tidying up and making an appropriate pull request (unless the addon author already implements the feature).

Capture

Replacing the 3 highlighted lines of code in ThreatWidget.lua with the following:

local _, _, scaledPercentage, _, _ = UnitDetailedThreatSituation("player", unit.unitid)
if scaledPercentage then
widget_frame.Percentage:SetText(string_format("%.0f%%", scaledPercentage))

  if Settings.ThreatPercentage.Show then
    local isTanking, _, scaledPercentage, _, threatValue = UnitDetailedThreatSituation("player", unit.unitid)

    local DISPLAY_TYPE = "diff"
    local THREAT_DOWNSCALE = 1000

    if scaledPercentage then
      if DISPLAY_TYPE == "diff" then
        local threatDiff = 0
        if isTanking then
          -- Determine threat diff by finding the next highest raid/party member threat
          local numGroupMembers = IsInRaid() and GetNumGroupMembers() or GetNumSubgroupMembers()
          local topOtherThreat = 0
          local topOtherThreatUnit = nil
          local groupType = IsInRaid() and "raid" or "party"

          for i = 1, numGroupMembers do
            local _, _, _, _, tValue = UnitDetailedThreatSituation(string_format("%s%d", groupType, i), unit.unitid)
            local _, _, _, _, tpValue = UnitDetailedThreatSituation(string_format("%spet%d", groupType, i), unit.unitid)
            
            if tValue and tValue > topOtherThreat then
              topOtherThreat = tValue
              topOtherThreatUnit = format("%s%d", groupType, i)
            end
            if tpValue and tpValue > topOtherThreat then
              topOtherThreat = tpValue
              topOtherThreatUnit = format("%spet%d", groupType, i)
            end
          end

          if topOtherThreatUnit then
            threatDiff = threatValue - topOtherThreat
          end
        else
          -- Determine raw threat deficit by scaled % of target threat
          if threatValue == 0 or scaledPercentage == 0 then
            threatDiff = 0
          else
            threatDiff = threatValue - threatValue/(scaledPercentage/100)
          end
        end
        -- Show threat delta if non-zero
        local diffText = not (threatDiff == 0) and string_format("%s%d", (threatDiff > 0 and "+" or ""), threatDiff/THREAT_DOWNSCALE) or ""
        widget_frame.Percentage:SetText(diffText)
      else
        widget_frame.Percentage:SetText(string_format("%.0f%%", scaledPercentage))
      end
  -- Remaining existing code to apply color and toggle show/hide of widget frame
  -- ....
commented

If I may add to this, I've noticed moments where a viable solution ( at least how I understand it) is already present within the addon in certain circumstances.

For instance, say the mob is aggro'd onto someone else. If I stun the mob (Hammer of Justice) and proceed to apply and surpass threat, the mob name will turn yellow (because being stunned, it hasn't had the chance to actually switch aggro), and the % threat counter will actually keep climbing up past 100. Presumably because it's still showing in relation to its current target.

I've had situations where I applied large amounts of threat during the stun and saw the % climb to 180%, 200%. But then once the stun ends and the mob switches aggro to me, the bar turns red and the number resets down to 100%.

If it was viable to keep the % counter in relation to the second highest threat target, after the aggro switches to me, that would solve my problem pretty neatly.

commented

I was just asking about this feature request recently. Would be a great addition. Being a tank and it showing you have 100% threat isn't really that helpful. It doesn't really tell you if you need to build more threat on it or if it's okay to build threat on a different NPC, etc.