wardz / ClassicCastbars

[WoW] Customizable target castbars for Classic Era, TBC, Wrath and Retail.
63 stars 5 forks source link

Wrong cast times with Curse of Tongues, wrongly applied new cast time to the current casting spell #22

Closed millanzarreta closed 4 years ago

millanzarreta commented 4 years ago

I'm using the last ClassicCastbars version atm, v1.1.2, but these bugs also occur in previous versions.

What is the problem? Is there any Lua errors? No Lua errors. The problem are two bugs related with cast times when some cast speed reduction debuff is present. I only test this with "Curse of Tongues (rank 1)" (I don't have the rank 2 yet), what is a 50% cast speed reduction, I don't know if this also happens with rank 2 and other similar spells as rogue poisons. The two bugs:

1.- The new cast time calculated seems to be wrong, the addon calculates a slower casting time than it really is.

2.- The addon wrongly readjust the cast time of the current casting spell when the debuff is applied, this behaviour is wrong, when some is casting a spell and receive a cast speed modifier it doesn't apply to the current casting spell.

I think some short clips are very explanatory:

The addon works well without debuffs: https://gfycat.com/evergreenpastelbarracuda-warthunder

The addon calculating wrongs cast times with Curse of Tongues (rank 1), 50% cast speed reduction: https://gfycat.com/athleticaromaticbrant

The addon readjusting the cast time of current casting spell when receive a Curse of Tongues debuff: https://gfycat.com/unhappyflamboyantdarklingbeetle

What steps will reproduce the problem? Put a cast speed reduction debuff in a player/mob and let it cast something.

Does the issue still occur when all addons except ClassicCastbars are disabled? Yes

Was it working in a previous version? If yes, which was the last good one? No, I have not tried all versions, but it didn't work well in sine other older versions of the addon.

Any additional info? If you play on a non english client, please include your locale: I play in Spanish language, I have not tried if these bugs also happens with a other client/locale languages.

wardz commented 4 years ago

It's calculated always using highest rank, so 60% for Curse of Tongues.

Will look more into it after this weekend.

millanzarreta commented 4 years ago

Hi. I have investigated a little.

I am only a lvl 43, so I can't test the rank 2 of Curse of Tongues (earned at lvl 50). I modified this line in Data.lua temporally only with test purposes:

[GetSpellInfo(1714)] = 60, -- Curse of Tongues

to this:

[GetSpellInfo(1714)] = 50, -- Curse of Tongues

I did a reloadUI and the problem still happen, the new cast time calculated does not seem to be correct, it is still somewhat slower than expected. When I reach level 50 I will try the last rank of Curse of Tongues directly, but there seems to be something wrong.


Seeing the code I have seen another bug, in Data.lua only found one rank of Mind-Numbing Poison:

-- ROGUE
[GetSpellInfo(5760)] = 60,    -- Mind-Numbing Poison

But the different ranks of Mind-Numbing Poisong have different spell names, so it should be:

-- ROGUE
[GetSpellInfo(5760)] = 40,    -- Mind-Numbing Poison
[GetSpellInfo(8692)] = 50,    -- Mind-Numbing Poison II
[GetSpellInfo(11398)] = 60,   -- Mind-Numbing Poison III

I have seen that UnitAura is used to collect debuff information and also the _SPELL_AURAAPPLIED of _COMBAT_LOG_EVENTUNFILTERED. The spellId is not available for combat log, but it is often available for UnitAura. I think we can try to get spellId in UnitAura when available, but keep it working by taking the last rank in case this information (the spellId) is not available. I try this modifications and seems work well:

In Data.lua:

...
-- List of abilities that increases cast time (reduces speed)
-- Value here is the slow percentage, obtained from spellId.
-- TODO: check if these also affect Aimed Shot/Volley + bosses
namespace.castTimeIncreasesSpellId = {
    -- WARLOCK
    [1714]  = 50,   -- Curse of Tongues (rank 1)
    [11719] = 60,   -- Curse of Tongues (rank 2)
    [1098]  = 30,   -- Enslave Demon (rank 1)
    [11725] = 30,   -- Enslave Demon (rank 2)
    [11726] = 30,   -- Enslave Demon (rank 3)

    -- ROGUE
    [5760]  = 40,   -- Mind-Numbing Poison
    [8692]  = 50,   -- Mind-numbing Poison II
    [11398] = 60,   -- Mind-numbing Poison III

    -- ITEMS
    [17331] = 10,   -- Fang of the Crystal Spider

    -- NPCS
    [7127]  = 20,   -- Wavering Will
    [7102]  = 25,   -- Contagion of Rot
    [3603]  = 35,   -- Distracting Pain
    [8140]  = 50,   -- Befuddlement
    [8272]  = 20,   -- Mind Tremor
    [12255] = 15,   -- Curse of Tuten'kash
    [10651] = 20,   -- Curse of the Eye
    [14538] = 35,   -- Aural Shock
    [22247] = 80,   -- Suppression Aura
    [22642] = 50,   -- Brood Power: Bronze
    [23153] = 50,   -- Brood Power: Blue
    [24415] = 50,   -- Slow
    [19365] = 50,   -- Ancient Dread
    [28732] = 25,   -- Widow's Embrace
    [22909] = 50,   -- Eye of Immol'thar
}

-- List of abilities that increases cast time (reduces speed)
-- Value here is the slow percentage, from highest spell rank.
-- TODO: check if these also affect Aimed Shot/Volley + bosses
namespace.castTimeIncreases = {
    -- WARLOCK
    [GetSpellInfo(1714)] = 60,    -- Curse of Tongues
    [GetSpellInfo(1098)] = 30,    -- Enslave Demon

    -- ROGUE
    [GetSpellInfo(5760)] = 40,    -- Mind-Numbing Poison
    [GetSpellInfo(8692)] = 50,    -- Mind-Numbing Poison II
    [GetSpellInfo(11398)] = 60,   -- Mind-Numbing Poison III

    -- ITEMS
    [GetSpellInfo(17331)] = 10,   -- Fang of the Crystal Spider

    -- NPCS
    [GetSpellInfo(7127)] = 20,    -- Wavering Will
    [GetSpellInfo(7102)] = 25,    -- Contagion of Rot
    [GetSpellInfo(3603)] = 35,    -- Distracting Pain
    [GetSpellInfo(8140)] = 50,    -- Befuddlement
    [GetSpellInfo(8272)] = 20,    -- Mind Tremor
    [GetSpellInfo(12255)] = 15,   -- Curse of Tuten'kash
    [GetSpellInfo(10651)] = 20,   -- Curse of the Eye
    [GetSpellInfo(14538)] = 35,   -- Aural Shock
    [GetSpellInfo(22247)] = 80,   -- Suppression Aura
    [GetSpellInfo(22642)] = 50,   -- Brood Power: Bronze
    [GetSpellInfo(23153)] = 50,   -- Brood Power: Blue
    [GetSpellInfo(24415)] = 50,   -- Slow
    [GetSpellInfo(19365)] = 50,   -- Ancient Dread
    [GetSpellInfo(28732)] = 25,   -- Widow's Embrace
    [GetSpellInfo(22909)] = 50,   -- Eye of Immol'thar
}
...

In ClassicCastBar.lua:

...
local bit_band = _G.bit.band
local COMBATLOG_OBJECT_TYPE_PLAYER_OR_PET = _G.COMBATLOG_OBJECT_TYPE_PLAYER + _G.COMBATLOG_OBJECT_TYPE_PET
local castTimeIncreases = namespace.castTimeIncreases
local castTimeIncreasesSpellId = namespace.castTimeIncreasesSpellId

function addon:CheckCastModifier(unitID, unitGUID)
    if not self.db.pushbackDetect then return end

    for i = 1, 16 do
        local name, _, _, _, _, _, _, _, _, spellId = UnitAura(unitID, i, "HARMFUL")
        if not name then return end -- no more debuffs

        local slowPercentage
        if (spellId ~= nil and spellId > 0) then
            slowPercentage = castTimeIncreasesSpellId[spellId]
        end
        if not slowPercentage then
            slowPercentage = castTimeIncreases[name]
        end
        if slowPercentage then
            return self:SetCastDelay(unitGUID, slowPercentage, nil, true)
        end
    end
end
...

No modification are maded in the _SPELL_AURAAPPLIED section because we never have the spellId there.

wardz commented 4 years ago

Thank you for the detailed information, helps a lot.

The addon wrongly readjust the cast time of the current casting spell when the debuff is applied, this behaviour is wrong, when some is casting a spell and receive a cast speed modifier it doesn't apply to the current casting spell.

Is this only the case for players? In PvE im pretty sure it affects the currenct cast, atleast on yourself.

Do you know if the modifier calculates 50-60% slow of the base cast time (e.g 2.5sec for Frostbolt) or if it's 60% of the remaining cast time? I don't have access to a slow spell so hard for me to test.

millanzarreta commented 4 years ago

Mmmm I will have to test these things to be sure. I will test these things tomorrow when I have some time and I'll let you know.

millanzarreta commented 4 years ago

Is this only the case for players? In PvE im pretty sure it affects the currenct cast, atleast on yourself.

Do you know if the modifier calculates 50-60% slow of the base cast time (e.g 2.5sec for Frostbolt) or if it's 60% of the remaining cast time? I don't have access to a slow spell so hard for me to test.

I do not think so, I think cast time reduction debuffs never affects the ongoing casting spell, but the best way to know is to test it. I have made three clips where I compare the normal casting time, the casting time with tongues or similar curses and the casting time with a tongues while casting.

The conclusion is that reduction cast time debuffs never affect an ongoing spell, only the following castings. I have not test what happens to a slowed casting spell affected by tongues debuff if this debuff ends before the cast ends. I suppose that in the same way that a new debuff does not affect an ongoing cast, this would not affect either. Nor have I test how the addon handles this situation.

When making the clips I forgot to remove the changes I had made in the addon files. Therefore, the casting times that are calculated for Curse of Tongues are calculated by the addon as -50% of casting time and not as 60%, since the addon includes the changes I have shown in the previous message.

In summary, I think it is necessary to fix the two bugs that I mentioned in the first message: the bad calculation of cast times with cast speed reduction debuffs (the addon calculates slower cast times than they really are) and that the addon recalculates the cast time when a reduction cast time debuffs is applied over a player/npc while is casting a spell.

wardz commented 4 years ago

Try now in v1.1.4. I made a few changes that should hopefully fix both issues but im not 100% sure if it works. Thanks again for the help.

millanzarreta commented 4 years ago

Mmm seems to work fine, but I noticed some strange behaviour occasionally. Let me test this a some more.

millanzarreta commented 4 years ago

Ok, after some research I discovered some things:


1º [OK] The addon now calculates well the cast times when the target has a cast time reduction debuff like tongues


2º [OK] Now the addon no longer recalculates the cast timers for a ongoing cast if a cast time reduction debuff is applied in middle of the cast.


3º [OK] I test if debuffs like tongues affect to a [Hearthstone] and a [Summon Felsteed]. Indeed, it does affect, and the addon does the calculation of the new casting time well.


4º [--] Some mobs are immune to a reduction cast mechanics, like bosses and some big elites. Some regular mobs like "Fledgling Chillwind" in Winterspring are also immune to this mechanic. However, even if the NPC is immune, debuff can still be applied. So, for these cases, the recalculated times are wrong.

I believe that this is not a big problem, and it is not worth the effort to add a custom NPC list or something similar. We can deal with it.


5º [NOT OK] The addon uses a cache to store NPC cast times. This allows add to the cache the cast time of NPC spells, even if this spell has another preset duration. The problem is the addon ignores cast time reduction debuffs like tongues, and adds to the cache as the "good" cast time the cast time with the tongues debuff. After this happens, the cached cast time is incorrect, and the following castings will have an incorrect time.

Two possible solutions:

  1. Do not add the spell into the cache if a cast time reduction debuff is pressent
  2. Add the spell into the cache, but doing a reverse calculation to get the good cast time

I believe that solution number 1 is the best, because the solution number 2 will calculate bad times if the NPC is immune to cast time reduction mechanics. Is simple and better not to add the spell to the cache if a reduction cast debuff is present.


6º [NOT OK] The code has a bug in Data.lua :

-- Store both spellID and spell name in this table since UnitAura returns spellIDs but combat log doesn't.
-- Order here is important so we only store highest rank for spell names
for spellID, slowPercentage in ipairs(namespace.castTimeIncreases) do
    namespace.castTimeIncreases[GetSpellInfo(spellID)] = slowPercentage
end

You never get into this loop. The ipairs allows you to iterate in a table in an orderly manner. First the index 1, then 2, then 3, and so on until one of these indexes is nil (although the table has more elements). In this case, the indexes in the table are the spellIds. Therefore, an attempt will be made to access element [1] (spellId=1), but since this is nil, the loop ends and no element will be processed.

If the table is not an array with ordered indixes, you should use pairs to iterate the table, but no order is guaranteed. I know the order here is important to add to the table the times of the last rank, but this must be handled differently. I think somethink like this is a good solution:

-- Store both spellID and spell name in this table since UnitAura returns spellIDs but combat log doesn't.
-- Order here is important so we only store highest rank for spell names
for spellID, slowPercentage in pairs(namespace.castTimeIncreases) do
    local spellIDsWithPriority = {
        [11719] = true,
        [11726] = true,
        [11398] = true
    }
    local spellName = GetSpellInfo(spellID)
    if (spellName and (not namespace.castTimeIncreases[spellName] or spellIDsWithPriority[spellID])) then
        namespace.castTimeIncreases[spellName] = slowPercentage
    end
end

Other solution is add a second element to the elements of the castTimeIncreases table that means the priority.


7º [MINOR] I get two new spellIds to add to this castTimeIncreases list:

namespace.castTimeIncreases = {
...
[10653] = 20,   -- Curse of the Eye
[20882] = 30,   -- Enslave Demon
...
}

8º [NOT OK] At the moment, if two cast reduction debuffs are in the same target, the addon select only the slowest debuff. This is not how it works. If more than one debuff is present, a multiplicative time calculation will be made. Although it is true that this situation is extremely rare.

I personally test it with Curse of Tongues (-60%) and Curse of the Eye (-20%) in the same target. Some of the new cast times was:

So, I wondered what happened in PvP with multiple Curse of Tongues and mutiple Mind-Numbing Poison. I only could test one of the three possible cases:

I will try to investigate these other cases. But anyway, from what I've been able to observe so far, it seems that if there is more than one cast reduction debuff present in a target, the casting time is reduced in a multiplicative manner.


As a summary, I believe that points 5 and 6 are important and should be fixed. The point 7 is not important but is easy to implement, it is just adding two lines of code. You can also try to solve point 8 although it is somewhat uncommon. The point 4 is something for which I have not been able to find an easy solution and I think we can deal with it as is.

wardz commented 4 years ago

5º is already implemented actually but I forgot to rename a variable when changing some stuff in latest version, will have it fixed.

Good catch with 6º, I only tested it with pairs and changed to ipairs without retesting it. But order is actually not important anymore, so just doing pairs will be fine here for now.

Will have all of these fixed in next version besides maybe 4º. Thanks again for the help!

Edit: Do you happen to know if Curse of Tongues affect Grenades and Aimed Shot/Volley? Just curious.

millanzarreta commented 4 years ago

I don't think this affects to Volley, since it is a channeled spell.

I don't have a hunter to test Aimed Shot, but I will try to test it. About Grenades I'm engineer, so I will test it soon when I have time.

Edit: I have tested the Grenades cast time (Ez-Thro Dynamite II, Big Bronze Bomb, Big Iron Bomb and a Mithril Frag Bomb specifically), and I can confirm that Curse of Tongues affects to their cast time, I also have tested it with the Curse of the Eye (curse from a NPC) and it also affects to the cast time. The normal cast time of these are 1.0sec, and with Curse of Tongues (-60%) was 1.6sec and with Curse of the Eye (-20%) was 1.2sec.

wardz commented 4 years ago

Uploaded a new version now that should hopefully fix most issues besides 8 and 4. I'll work more on 8 once I figure out how it works on players with i.e tongues and mind numbing both applied.

millanzarreta commented 4 years ago

I will try to find out how it works when Tongues and Mind Numbing both applied when I have some more time, maybe this weekend... (I will try to convince a rogue to help me).

millanzarreta commented 4 years ago

Hi! I still haven't been able to test the Tongues+Mind Numbing, but I have discovered something also useful anyway.

First, I have tested with a hunter if Tongues affects Aimed Shot, and as I suspected, it doesn't do it. Here (https://gfycat.com/sneakywellgroomedcondor) a clip with the test.

Later, while investigating the GetSpellInfo API function, I noticed that the castTime returned by function is affected by these cast speed reduction debuffs (if the player have one), even for other class spells or NPC spells, but not for spells that are not affected by these cast speed reduction debuffs. This is great, because I can perform a masive test of all spellIds included by the addon (in the castSpellIDs variable) and also other some spellIds from other sources.

For example, with any class, the castTime of Polymorph returned by GetSpellInfo is 1500, but if is affected by Tongues (60%) the castTime will be 2400. But if we test the Aimed Shot spell, the castTime will allways be 3000 even with Tongues.

Ok, then I test this and these was the results of what spellIds are unnafected by cast speed reduction debuffs:

unaffectedCastModsSpells = {
    -- Player Spells
    1464,   -- Slam
    8820,   -- Slam
    11604,  -- Slam
    11605,  -- Slam
    11202,  -- Crippling Poison
    3421,   -- Crippling Poison II
    13227,  -- Wound Poison
    13228,  -- Wound Poison II
    13229,  -- Wound Poison III
    13230,  -- Wound Poison IV
    5761,   -- Mind-numbing Poison
    8693,   -- Mind-numbing Poison II
    11399,  -- Mind-numbing Poison III
    8681,   -- Instant Poison
    8686,   -- Instant Poison II
    8688,   -- Instant Poison III
    11338,  -- Instant Poison IV
    11339,  -- Instant Poison V
    11343,  -- Instant Poison VI
    2835,   -- Deadly Poison
    2837,   -- Deadly Poison II
    11356,  -- Deadly Poison IV
    11355,  -- Deadly Poison III
    25347,  -- Deadly Poison V
    6651,   -- Instant Toxin
    1842,   -- Disarm Trap
    1804,   -- Pick Lock
    6461,   -- Pick Lock
    6463,   -- Pick Lock
    19434,  -- Aimed Shot
    20900,  -- Aimed Shot
    20901,  -- Aimed Shot
    20902,  -- Aimed Shot
    20903,  -- Aimed Shot
    20904,  -- Aimed Shot
    2641,   -- Dismiss Pet
    2480,   -- Shoot Bow
    7918,   -- Shoot Gun
    20549,  -- War Stomp
    20589,  -- Escape Artist
    -- NPCs and Others
    3583,   -- Deadly Poison
    13582,  -- Deadly Poison
    21787,  -- Deadly Poison
    3131,   -- Frost Breath
    16094,  -- Frost Breath
    16099,  -- Frost Breath
    16340,  -- Frost Breath
    21099,  -- Frost Breath
    22479,  -- Frost Breath
    6917,   -- Venom Spit
    16552,  -- Venom Spit
    15664,  -- Venom Spit
    16866,  -- Venom Spit
    17158,  -- Venom Spit
    23862,  -- Venom Spit
    24011,  -- Venom Spit
    7068,   -- Veil of Shadow
    17820,  -- Veil of Shadow
    23224,  -- Veil of Shadow
    6909,   -- Curse of Thorns
    16247,  -- Curse of Thorns
    13608,  -- Hooked Net
    14030,  -- Hooked Net
    15609,  -- Hooked Net
    20716,  -- Sand Breath
    20717,  -- Sand Breath
    8806,   -- Poisoned Shot
    8275,   -- Poisoned Shot
    1980,   -- Bombard
    3015,   -- Bombard II
    1536,   -- Longshot II
    3007,   -- Longshot III
    1540,   -- Volley
    3013,   -- Volley II
    7355,   -- Stuck
    4164,   -- Throw Rock
    4165,   -- Throw Rock II
    3537,   -- Minions of Malathrom
    5567,   -- Miring Mud
    28352,  -- Breath of Sargeras
    7106,   -- Dark Restore
    4075,   -- Large Seaforium Charge
    17709,  -- Summon Zergling
    6619,   -- Cowardly Flight Potion
    12719,  -- Explosive Arrow
    10679,  -- Summon White Kitten
    30021,  -- Crystal Infused Bandage
    10687,  -- Summon White Plymouth Rock
    10695,  -- Summon Dark Whelpling
    10699,  -- Summon Bronze Whelpling
    13258,  -- Summon Goblin Bomb
    5106,   -- Crystal Flash
    10711,  -- Summon Snowshoe Rabbit
    10715,  -- Summon Blue Racer
    10719,  -- Summon Ribbon Snake
    22979,  -- Shadow Flame
    23530,  -- Summon Tiny Red Dragon
    24576,  -- Chromatic Mount
    16967,  -- Inlaid Thorium Hammer
    16983,  -- Serenity
    5669,   -- Peon Disguise
    23650,  -- Ebon Hand
    24696,  -- Summon Murky
    3611,   -- Minion of Morganth
    27794,  -- Cleave
    25247,  -- Longsight
    5208,   -- Poisoned Harpoon
    28505,  -- Summon Poley
    14532,  -- Creeper Venom
    3132,   -- Chilling Breath
    3650,   -- Sling Mud
    3651,   -- Shield of Reflection
    3143,   -- Glacial Roar
    23012,  -- Summon Orphan
    26086,  -- Felcloth Bag
    5264,   -- South Seas Pirate Disguise
    5266,   -- Syndicate Disguise
    5268,   -- Dark Iron Dwarf Disguise
    6296,   -- Enchant: Fiery Blaze
    16984,  -- Volcanic Hammer
    16992,  -- Frostguard
    24194,  -- Uther's Tribute
    17567,  -- Summon Blood Parrot
    21160,  -- Eye of Sulfuras
    24266,  -- Gurubashi Mojo Madness
    7364,   -- Light Torch
    12684,  -- Kadrak's Flag
    23811,  -- Summon Jubling
    22797,  -- Force Reactive Disk
    12720,  -- Goblin "Boom" Box
    16729,  -- Lionheart Helm
    10684,  -- Summon Senegal
    10688,  -- Summon Cockroach
    10696,  -- Summon Azure Whelpling
    10700,  -- Summon Faeling
    10704,  -- Summon Tree Frog
    23428,  -- Summon Albino Snapjaw
    7919,   -- Shoot Crossbow
    10716,  -- Summon Brown Snake
    22430,  -- Refined Scale of Onyxia
    21935,  -- SnowMaster 9000
    6907,   -- Diseased Slime
    3204,   -- Sapper Explode
    26234,  -- Submerge Visual
    26063,  -- Ouro Submerge Visual
    6925,   -- Gift of the Xavian
    7951,   -- Toxic Spit
    10788,  -- Leopard
    16058,  -- Primal Leopard
    16450,  -- Summon Smolderweb
    28738,  -- Summon Speedy
    16993,  -- Masterwork Stormhammer
    23652,  -- Blackguard
    24195,  -- Grom's Tribute
    16554,  -- Toxic Bolt
    12900,  -- Mobile Alarm
    12904,  -- Gnomish Ham Radio
    6470,   -- Tiny Bronze Key
    15495,  -- Explosive Shot
    17169,  -- Summon Carrion Scarab
    471,    -- Palamino Stallion
    19772,  -- Summon Lifelike Toad
    16746,  -- Invulnerable Mail
    15048,  -- Summon Bomb
    16082,  -- Palomino Stallion
    25018,  -- Summon Murki
    6530,   -- Sling Dirt
    30152,  -- Summon White Tiger Cub
    26072,  -- Dust Cloud
    5514,   -- Darken Vision
    11016,  -- Soul Bite
    22567,  -- Summon Ar'lia
    21050,  -- Melodious Rapture
    17481,  -- Deathcharger
    16978,  -- Blazing Rapier
    25162,  -- Summon Disgusting Oozeling
    4520,   -- Wide Sweep
    23653,  -- Nightfall
    4526,   -- Mass Dispell
    6576,   -- Intimidating Growl
    20627,  -- Lightning Breath
    28324,  -- Forming Frame of Atiesh
    22719,  -- Black Battlestrider
    25793,  -- Demon Summoning Torch
    23254,  -- Redeeming the Soul
    25849,  -- Summon Baby Shark
    18711,  -- Forging
    12198,  -- Marksman Hit
    8153,   -- Owl Form
    10673,  -- Summon Bombay
    10677,  -- Summon Siamese
    6626,   -- Set NG-5 Charge (Blue)
    6630,   -- Set NG-5 Charge (Red)
    10685,  -- Summon Ancona
    10697,  -- Summon Crimson Whelpling
    10701,  -- Summon Dart Frog
    10705,  -- Summon Eagle Owl
    23430,  -- Summon Olive Snapjaw
    30081,  -- Retching Plague
    10717,  -- Summon Crimson Snake
    10721,  -- Summon Elven Wisp
    6648,   -- Chestnut Mare
    22967,  -- Smelt Elementium
    6656,   -- Remote Detonate
    23510,  -- Stormpike Battle Charger
    10254,  -- Stone Dwarf Awaken Visual
    16987,  -- Darkspear
    16995,  -- Heartseeker
    2840,   -- Creeping Anguish
    2841,   -- Creeping Torment
    5169,   -- Defias Disguise
    8286,   -- Summon Boar Spirit
    3359,   -- Drink Potion
    22704,  -- Field Repair Bot 74A
    3363,   -- Summon Riding Gryphon
    17618,  -- Summon Risen Lackey
    15999,  -- Summon Worg Pup
    11923,  -- Repair the Blade of Heroes
    17235,  -- Raise Undead Scarab
    16724,  -- Whitesoul Helm
    8386,   -- Attacking
    19814,  -- Masterwork Target Dummy
    15049,  -- Summon Robot
    23431,  -- Summon Leatherback Snapjaw
    28311,  -- Slime Bolt
    26010,  -- Summon Tranquil Mechanical Yeti
    1698,   -- Shockwave
    15067,  -- Summon Sprite Darter Hatchling
    13548,  -- Summon Farm Chicken
    23008,  -- Powerful Seaforium Charge
    10703,  -- Summon Wood Frog
    6951,   -- Decayed Strength
    5265,   -- Stonesplinter Trogg Disguise
    5267,   -- Dalaran Wizard Disguise
    28732,  -- Widow's Embrace
    15648,  -- Summon Corrupted Kitten
    16980,  -- Rune Edge
    16988,  -- Hammer of the Titans
    23639,  -- Blackfury
    28487,  -- Summon Terky
    28995,  -- Stoneskin
    25146,  -- Transmute: Elemental Fire
    24706,  -- Toss Stink Bomb
    6257,   -- Torch Toss
    27829,  -- Titanic Leggings
    10682,  -- Summon Hyacinth Macaw
    7359,   -- Bright Campfire
    16590,  -- Summon Zombie
    9612,   -- Ink Spray
    28740,  -- Summon Whiskers
    23531,  -- Summon Tiny Green Dragon
    3436,   -- Wandering Plague
    9636,   -- Summon Swamp Spirit
    17707,  -- Summon Panda
    17204,  -- Summon Skeleton
    7896,   -- Exploding Shot
    10712,  -- Summon Spotted Rabbit
    10674,  -- Summon Cornish Rex
    12722,  -- Goblin Radio
    16741,  -- Stronghold Gauntlets
    23392,  -- Boulder
    10708,  -- Summon Snowy Owl
    10698,  -- Summon Emerald Whelpling
    10702,  -- Summon Island Frog
    10706,  -- Summon Hawk Owl
    10710,  -- Summon Cottontail Rabbit
    7920,   -- Mebok Smart Drink
    10718,  -- Summon Green Water Snake
    27830,  -- Persuader
    8682,   -- Fake Shot
    10678,  -- Summon Silver Tabby
    23638,  -- Black Amnesty
    10720,  -- Summon Scarlet Snake
    28614,  -- Pointy Spike
    8016,   -- Spirit Decay
    10709,  -- Summon Prairie Dog
    10686,  -- Summon Prairie Chicken
    2838,   -- Creeping Pain
    26102,  -- Sand Blast
    3477,   -- Spirit Steal
    5395,   -- Death Capsule
    5668,   -- Peasant Disguise
    5159,   -- Melt Ore
    12243,  -- Summon Mechanical Chicken
    5403,   -- Crash of Waves
    17460,  -- Frost Ram
    16965,  -- Bleakwood Hew
    16973,  -- Enchanted Battlehammer
    8256,   -- Lethal Toxin
    23632,  -- Girdle of the Dawn
    16970,  -- Dawn's Edge
    28739,  -- Summon Mr. Wiggles
    6441,   -- Explosive Shells
    27241,  -- Summon Gurky
    16986,  -- Blood Talon
    10675,  -- Summon Maine Coon
    23429,  -- Summon Loggerhead Snapjaw
    10850,  -- Powerful Smelling Salts
    3488,   -- Felstrom Resurrection
    30047,  -- Crystal Throat Lozenge
    27832,  -- Sageblade
    10346,  -- Machine Gun
    12740,  -- Summon Infernal Servant
    6469,   -- Skeletal Miner Explode
    6471,   -- Tiny Iron Key
    11397,  -- Diseased Shot
    30156,  -- Summon Hippogryph Hatchling
    27589,  -- Black Grasp of the Destroyer
    10683,  -- Summon Green Wing Macaw
    23432,  -- Summon Hawksbill Snapjaw
    4950,   -- Summon Helcular's Puppets
    17708,  -- Summon Diablo
    8363,   -- Parasite
    10707,  -- Summon Great Horned Owl
    16531,  -- Summon Frail Skeleton
    22027,  -- Remove Insignia
    10681,  -- Summon Cockatoo
    26443,  -- Firework Cluster Launcher
    6510,   -- Blinding Powder
    16991,  -- Annihilator
    10713,  -- Summon Albino Snake
    16072,  -- Purify and Place Food
    20629,  -- Corrosive Venom Spit
    10680,  -- Summon Cockatiel
    10676,  -- Summon Orange Tabby
    10714,  -- Summon Black Kingsnake
    28615,  -- Spike Volley
    19566,  -- Salt Shaker
    7901,   -- Decayed Agility
    7054,   -- Forsaken Skills
    24189,  -- Force Punch
},

With this table should be easy to check if the spellId is in the exclusion list and never modify the casting time for these spells even though the unit has Tongues or similar debuffs.


While testing this, I noticed a minor "bug", in the variable castSpellIDs this spell is included:

local castSpellIDs = {
  ...
  19799, -- Dark Iron Bomb
  ...
}

The 19799 is the spellId of craft Dark Iron Bombs, not the spellId of throw one (that is 19784), and the spellName is the same for the two spells (but castTime is not the same!), so I think is better to remove this and add the spellId of throw the Drak Iron Bomb:

local castSpellIDs = {
  ...
  19784, -- Dark Iron Bomb
  ...
}

Now, one new problem appears with the discover that castTime returned by GetSpellInfo is affected by Tongues or similar debuffs (in the player). When in the addon this function is called to know the castTime of spells (twice in function _addon:COMBAT_LOG_EVENTUNFILTERED()), this information will be wrong if the player is affected by these debuffs. I think this should be fixed with something like this (although I have not test the code directly):

...
local math_floor = math.floor
...
function addon:adjustCastTime(spellId, castTime)
  if spellId == nil or castTime == nil then
    return nil
  end
  if (unaffectedCastModsSpells[spellId]) then  -- check if the spellId is in the exclusion list
    return castTime
  else
    local _, _, _, hCastTime = GetSpellInfo(8690)  -- Hearthstone spell (normal hCastTime should be 10000)
    if (hCastTime and hCastTime ~= 0 and hCastTime ~= 10000) then
      return math_floor(castTime * 10000 / hCastTime)
    else
      return castTime
    end
  end
end

function addon:COMBAT_LOG_EVENT_UNFILTERED()
    local _, eventType, _, srcGUID, srcName, srcFlags, _, dstGUID,  _, dstFlags, _, _, spellName = CombatLogGetCurrentEventInfo()

    if eventType == "SPELL_CAST_START" then
        local spellID = castedSpells[spellName]
        if not spellID then return end

        local _, _, icon, castTime = GetSpellInfo(spellID)
        if not castTime then return end
        castTime = self:adjustCastTime(spellID, castTime)

        -- is player or player pet or mind controlled
        local isPlayer = bit_band(srcFlags, COMBATLOG_OBJECT_CONTROL_PLAYER) > 0

        if srcGUID ~= self.PLAYER_GUID then
            if isPlayer then
                -- Use talent reduced cast time for certain player spells
                local reducedTime = castTimeTalentDecreases[spellName]
                if reducedTime then
                    castTime = reducedTime
                end
            else
                local cachedTime = npc
                Cache[srcName .. spellName]
                if cachedTime then
                    -- Use cached time stored from earlier sightings for NPCs.
                    -- This is because mobs have various cast times, e.g a lvl 20 mob casting Frostbolt might have
                    -- 3.5 cast time but another lvl 40 mob might have 2.5 cast time instead for Frostbolt.
                    castTime = cachedTime
                else
                    npcCastTimeCacheStart[srcGUID] = GetTime()
                end
            end
        else
            local _, _, _, startTime, endTime = CastingInfo()
            if endTime and startTime then
                castTime = endTime - startTime
            end
        end

        -- Note: using return here will make the next function (StoreCast) reuse the current stack frame which is slightly more performant
        return self:StoreCast(srcGUID, spellName, icon, castTime, isPlayer)
    elseif eventType == "SPELL_CAST_SUCCESS" then
        local channelCast = channeledSpells[spellName]
        local spellID = castedSpells[spellName]
        if not channelCast and not spellID then return end

        local isPlayer = bit_band(srcFlags, COMBATLOG_OBJECT_CONTROL_PLAYER) > 0

        -- Auto correct cast times for mobs
        if not isPlayer and not channelCast then
          if not strfind(srcGUID, "Player-") then -- incase player is mind controlled by NPC
            local cachedTime = npcCastTimeCache[srcName .. spellName]
            if not cachedTime then
                local cast = activeTimers[srcGUID]
                if not cast or (cast and not cast.hasCastSlowModified and not cast.hasBarkskinModifier and not cast.hasFocusedCastingModifier and not cast.hasNaturesGraceModifier) then
                    local restoredStartTime = npcCastTimeCacheStart[srcGUID]
                    if restoredStartTime then
                        local castTime = (GetTime() - restoredStartTime) * 1000
                        local origCastTime = 0
                        if spellID then
                            local _, _, _, cTime = GetSpellInfo(spellID)
                            origCastTime = cTime or 0
                            origCastTime = self:adjustCastTime(spellID, origCastTime)
                        end

                        local castTimeDiff = abs(castTime - origCastTime)
                        if castTimeDiff <= 4000 and castTimeDiff > 250 then -- heavy lag might affect this so only store time if the diff isn't too big
                            npcCastTimeCache[srcName .. spellName] = castTime
                        end
                    end
                end
            end
          end
        end
        ...
    elseif eventType == "SPELL_AURA_APPLIED" then
        ...
    elseif eventType == "SPELL_AURA_REMOVED" then
        ...
    elseif eventType == "SPELL_CAST_FAILED" then
        ...
    elseif eventType == "PARTY_KILL" or eventType == "UNIT_DIED" or eventType == "SPELL_INTERRUPT" then
        ...
    elseif eventType == "SWING_DAMAGE" or eventType == "ENVIRONMENTAL_DAMAGE" or eventType == "RANGE_DAMAGE" or eventType == "SPELL_DAMAGE" then
        ...
    end
end
...
wardz commented 4 years ago

Heya, very interesting. I've changed the Dark Iron Bomb spellID and implemented a fix for the GetSpellInfo issue now, (untested) but im still hesitant about adding another large data table as the addon is already bloated with data as it is. I'll have to think about it, it might be possible to instead just modify the cast time auto correction feature to detect if a unit's cast are able to be slowed or not.

millanzarreta commented 4 years ago

We are in 2019 (soon 2020!), I believe that adding a one more static table of 331 elements should not be a problem. Other addons uses several tables of more than 4000 elements and there is no problem.

it might be possible to instead just modify the cast time auto correction feature to detect if a unit's cast are able to be slowed or not.

It is obvious that a method that can detect if a spell casted by a unit can be slowed or not is better than a fixed static table, but I have no idea how to detect if a unit's cast are able to be slowed or not, since the GetSpellInfo function only return the castTime value from your point of view (and depend only on your debuffs).

Think about it, but I believe that this table is necessary anyway (unless we discover another method for this), since it is necessary to know what spells can be slowed and which not, to be able to take as valid/invalid the castTime information returned by GetSpellInfo.

EDIT: I have seen the commit submited on this issue and I think I have not explained myself well:

local IsSpellKnown = _G.IsSpellKnown
local function GetSpellCastTime(spellID)
    local _, _, icon, castTime = GetSpellInfo(spellID)
    if not castTime then return end

    if IsSpellKnown(spellID) then
        local _, _, _, hCastTime = GetSpellInfo(8690) -- Hearthstone, normal cast time 10s
        if hCastTime and hCastTime ~= 10000 then -- If current cast time is not 10s it means the player has a casting speed modifier debuff applied on himself.
            -- Since the return values by GetSpellInfo() from spells that the player has learned in their spellbook are affected by the slow,
            -- we need to remove so it doesn't give slow modified casttimes for other peoples casts.
            return floor(castTime * (hCastTime / 10000)), icon
        end
    end

    return castTime, icon
end

There are several things wrong with this code:

            --Since the return values by GetSpellInfo() from spells that the player has learned in their spellbook are affected by the slow,

The GetSpellInfo() function return a modified castTime for ALL spells in the game, not only the spells in your spellbook, so the IsSpellKnown(spellID) is not necessary. For exampe, with a any class, if you don't have Tongues or similar debuff the result returned by this code is:

local spellList = {
    20904,    -- Aimed Shot (hunter)
    11605,    -- Slam (warrior)
    12826,    -- Polymorph (mage)
    17923,    -- Searing Pain (warlock)
    10955,    -- Shackle Undead (priest)
    5106,     -- Crystal Flash (NPC spell)
    184,      -- Fire Shield II (NPC spell)
    18431,    -- Bellowing Roar (NPC spell)
}
for _, v in ipairs(spellList) do
    local _, _, _, castTime = GetSpellInfo(v)
    print(v,"->",castTime)
end

will be:

20904 -> 3000
11605 -> 1500
12826 -> 1500
17923 -> 1500
10955 -> 1500
5106 -> 2000
184 -> 1000
18431 -> 1500

If you have Tongues (60%) the result of execute the code below is:

20904 -> 3000
11605 -> 1500
12826 -> 2400
17923 -> 2400
10955 -> 2400
5106 -> 2000
184 -> 1600
18431 -> 2400

As you can see, the castTime for Aimed Shot (hunter) and Slam (warrior) and also for the NPC spell Crystal Flash remain the same because they are not slowable spells, but the cast time varies for Polymorph (mage), Searing Pain (warlock), Shackle Undead (priest), Fire Shield II (NPC spell) and Bellowing Roar (NPC spell) because these are slowable spells.

This is how I have managed to datamine the non slowable spells. I have a huge array of spellIds. Without any debuff, I get the castTime for all of them calling to GetSpellInfo() and save it in Table A. Now I put the Tongues debuff on the test character and I get the castTime for the entire spellIds list again and save it in Table B. Now there is only compare Table A with Table B. Those spells whose castTime is the same in both cases means that they are not affected by debuffs.

return floor(castTime * (hCastTime / 10000)), icon

This should be:

return floor(castTime * 10000 / hCastTime), icon

Now a example. You are a mage with tongues (60%), and your enemy is a warlock without any cast reduction time debuff. You warlock start to cast Searing Pain (spellId=17923). In your code the COMBAT_LOG_EVENT_UNFILTERED this code is executed, when we obtain for the first time the castTime from the GetSpellInfo(17923) the castTime is 2400 (normal is 1500, but with Tongues in you the result is 2400). This is wrong because the warlock is not affected by tongues, only you, so the addon should correct this. In the function, the hCastTime returned by GetSpellInfo(8690) is 16000. So, with your code:

return floor(castTime * (hCastTime / 10000)), icon
return floor(2400 * (16000/ 10000)), icon
return floor(3840), icon

3840 is obviously wrong, with the suggested code:

return floor(castTime * 10000 / hCastTime), icon
return floor(2400 * 10000 / 16000), icon
return floor(1500), icon

1500 is the good value for the Searing Pain castTime value. And if the warlock also had a tongues effect? It does not matter, this function would also return the original value of 1500, and the castTime arrangement for the Searing Pain spell of warlock with tongues will be corrected later in the same way that it was being performed until now.

return floor(castTime * 10000 / hCastTime), icon
return floor(3000 * 10000 / 16000), icon
return floor(1875), icon

But 1875 is obviously wrong. This is one of the reasons we need a table or something similar to know if a spell is slowable spell or not, and if this spell is non slowable no correction is necessary.

wardz commented 4 years ago

We are in 2019 (soon 2020!), I believe that adding a one more static table of 331 elements should not be a problem. Other addons uses several tables of more than 4000 elements and there is no problem.

These things adds up really fast, especially if more than 1 addon are doing it. Its 2019, but many people still dont even use SSDs.

Maybe I misunderstood. If you're slowed, does GetSpellInfo()'s return values also affect other spells, i.e NPC spells? I thought it only affected spells if you also happened to have them in your own spellbook. For player spells we could use a small table though.

millanzarreta commented 4 years ago

Sorry I edit my previous comment to clarify some things, but yes, GetSpellInfo()'s return the value for all spells in the game as if you could cast them.

wardz commented 4 years ago

Ahh that explains things. I thought it worked like talent reductions where it only affects spells in your own spellbook and NPC spells that happen to have the exact same name for some reason. I'll most likely end up adding a table of spell ids then.

millanzarreta commented 4 years ago

I guess I didn't explain myself well before, it's really a bit weird ... but it's how it works. I will try to take a look the next commit and test it in the game to verify that it works well. Although I still think that adding the table is not a problem, even for NPC spells! It also provides us a double function: it allows us to skip the castTime correction in the GetSpellCastTime function for non-slowable spells and allows us not to modify the cast time in the function CheckCastModifier for spells that are no-slowable. If you still want to reduce the table, we can purge some spells, since most spellIds in the table are pet/mount summons and item craft spells.

I have also tested whether Troll's racial affects in this way and does. You can create a Troll level 1 and use your racial to check all these things of GetSpellInfo behaviors. The racial troll grants 10% of haste with 100% of hp and up to 30% of haste to the less hp you have (I think 30% was achieved with a 40% hp). The same code that is used to correct the castTime of spells when character has a debuff like Tongues should work without modifications for the Troll haste buff. I comment this so you can perform quick checks if you wish.

Meanwhile, I will continue trying to find a collaborative rogue to help me in the other matter :)

millanzarreta commented 4 years ago

Ok, here a reduced table with 177 elements (instead of 331). I remove the profession craft spells, summon mount spells and summon (non-combat)pets spells:

unaffectedCastModsSpells = {
    -- Player Spells
    1464,   -- Slam
    8820,   -- Slam
    11604,  -- Slam
    11605,  -- Slam
    6510,   -- Blinding Powder
    11202,  -- Crippling Poison
    3421,   -- Crippling Poison II
    13227,  -- Wound Poison
    13228,  -- Wound Poison II
    13229,  -- Wound Poison III
    13230,  -- Wound Poison IV
    5761,   -- Mind-numbing Poison
    8693,   -- Mind-numbing Poison II
    11399,  -- Mind-numbing Poison III
    8681,   -- Instant Poison
    8686,   -- Instant Poison II
    8688,   -- Instant Poison III
    11338,  -- Instant Poison IV
    11339,  -- Instant Poison V
    11343,  -- Instant Poison VI
    2835,   -- Deadly Poison
    2837,   -- Deadly Poison II
    11356,  -- Deadly Poison IV
    11355,  -- Deadly Poison III
    25347,  -- Deadly Poison V
    6651,   -- Instant Toxin
    1842,   -- Disarm Trap
    1804,   -- Pick Lock
    6461,   -- Pick Lock
    6463,   -- Pick Lock
    19434,  -- Aimed Shot
    20900,  -- Aimed Shot
    20901,  -- Aimed Shot
    20902,  -- Aimed Shot
    20903,  -- Aimed Shot
    20904,  -- Aimed Shot
    2641,   -- Dismiss Pet
    2480,   -- Shoot Bow
    7918,   -- Shoot Gun
    20549,  -- War Stomp
    20589,  -- Escape Artist
    22027,  -- Remove Insignia
    7355,   -- Stuck
    -- NPCs and Others
    3583,   -- Deadly Poison
    13582,  -- Deadly Poison
    21787,  -- Deadly Poison
    3131,   -- Frost Breath
    16094,  -- Frost Breath
    16099,  -- Frost Breath
    16340,  -- Frost Breath
    21099,  -- Frost Breath
    22479,  -- Frost Breath
    6917,   -- Venom Spit
    16552,  -- Venom Spit
    15664,  -- Venom Spit
    16866,  -- Venom Spit
    17158,  -- Venom Spit
    23862,  -- Venom Spit
    24011,  -- Venom Spit
    7068,   -- Veil of Shadow
    17820,  -- Veil of Shadow
    23224,  -- Veil of Shadow
    6909,   -- Curse of Thorns
    16247,  -- Curse of Thorns
    13608,  -- Hooked Net
    14030,  -- Hooked Net
    15609,  -- Hooked Net
    20716,  -- Sand Breath
    20717,  -- Sand Breath
    8806,   -- Poisoned Shot
    8275,   -- Poisoned Shot
    1980,   -- Bombard
    3015,   -- Bombard II
    1536,   -- Longshot II
    3007,   -- Longshot III
    1540,   -- Volley
    3013,   -- Volley II
    4164,   -- Throw Rock
    4165,   -- Throw Rock II
    3537,   -- Minions of Malathrom
    5567,   -- Miring Mud
    28352,  -- Breath of Sargeras
    7106,   -- Dark Restore
    4075,   -- Large Seaforium Charge
    5106,   -- Crystal Flash
    22979,  -- Shadow Flame
    3611,   -- Minion of Morganth
    27794,  -- Cleave
    25247,  -- Longsight
    5208,   -- Poisoned Harpoon
    14532,  -- Creeper Venom
    3132,   -- Chilling Breath
    3650,   -- Sling Mud
    3651,   -- Shield of Reflection
    3143,   -- Glacial Roar
    6296,   -- Enchant: Fiery Blaze
    24194,  -- Uther's Tribute
    7364,   -- Light Torch
    12684,  -- Kadrak's Flag
    7919,   -- Shoot Crossbow
    6907,   -- Diseased Slime
    3204,   -- Sapper Explode
    26234,  -- Submerge Visual
    26063,  -- Ouro Submerge Visual
    6925,   -- Gift of the Xavian
    7951,   -- Toxic Spit
    24195,  -- Grom's Tribute
    16554,  -- Toxic Bolt
    15495,  -- Explosive Shot
    6530,   -- Sling Dirt
    26072,  -- Dust Cloud
    5514,   -- Darken Vision
    11016,  -- Soul Bite
    21050,  -- Melodious Rapture
    4520,   -- Wide Sweep
    4526,   -- Mass Dispell
    6576,   -- Intimidating Growl
    20627,  -- Lightning Breath
    25793,  -- Demon Summoning Torch
    23254,  -- Redeeming the Soul
    18711,  -- Forging
    12198,  -- Marksman Hit
    8153,   -- Owl Form
    6626,   -- Set NG-5 Charge (Blue)
    6630,   -- Set NG-5 Charge (Red)
    30081,  -- Retching Plague
    6656,   -- Remote Detonate
    10254,  -- Stone Dwarf Awaken Visual
    3359,   -- Drink Potion
    17618,  -- Summon Risen Lackey
    17235,  -- Raise Undead Scarab
    8386,   -- Attacking
    28311,  -- Slime Bolt
    1698,   -- Shockwave
    23008,  -- Powerful Seaforium Charge
    6951,   -- Decayed Strength
    28732,  -- Widow's Embrace
    28995,  -- Stoneskin
    24706,  -- Toss Stink Bomb
    6257,   -- Torch Toss
    7359,   -- Bright Campfire
    16590,  -- Summon Zombie
    9612,   -- Ink Spray
    3436,   -- Wandering Plague
    9636,   -- Summon Swamp Spirit
    17204,  -- Summon Skeleton
    7896,   -- Exploding Shot
    23392,  -- Boulder
    7920,   -- Mebok Smart Drink
    8682,   -- Fake Shot
    28614,  -- Pointy Spike
    8016,   -- Spirit Decay
    26102,  -- Sand Blast
    3477,   -- Spirit Steal
    5395,   -- Death Capsule
    5159,   -- Melt Ore
    5403,   -- Crash of Waves
    8256,   -- Lethal Toxin
    6441,   -- Explosive Shells
    10850,  -- Powerful Smelling Salts
    3488,   -- Felstrom Resurrection
    10346,  -- Machine Gun
    12740,  -- Summon Infernal Servant
    6469,   -- Skeletal Miner Explode
    11397,  -- Diseased Shot
    4950,   -- Summon Helcular's Puppets
    8363,   -- Parasite
    16531,  -- Summon Frail Skeleton
    16072,  -- Purify and Place Food
    20629,  -- Corrosive Venom Spit
    28615,  -- Spike Volley
    19566,  -- Salt Shaker
    7901,   -- Decayed Agility
    7054,   -- Forsaken Skills
    24189,  -- Force Punch
},
millanzarreta commented 4 years ago

Ok, I have tested the changes and seems to work like a charm.

Taking a look at the code I suggest only a very small modification, in the function GetSpellCastInfo:

local function GetSpellCastInfo(spellID)
    ...
        if hCastTime and hCastTime ~= 10000 then -- If current cast time is not 10s it means the player has a casting speed modifier debuff applied on himself.
            -- Since the return values by GetSpellInfo() are affected by the modifier, we need to remove so it doesn't give modified casttimes for other peoples casts.
            return floor(castTime * 10000 / hCastTime), icon
        end
    ...
end

In this function I think is good to check that hCastTime is not 0 to prevent zero division error in the strange case that hCastTime is 0. It's just for be safe, I don't know anything that makes the Hearthstone instant cast:

local function GetSpellCastInfo(spellID)
    ...
        if hCastTime and hCastTime ~= 10000 and hCastTime ~= 0 then -- If current cast time is not 10s it means the player has a casting speed modifier debuff applied on himself.
            -- Since the return values by GetSpellInfo() are affected by the modifier, we need to remove so it doesn't give modified casttimes for other peoples casts.
            return floor(castTime * 10000 / hCastTime), icon
        end
    ...
end

I think the only thing that would remain pending is discover how Tongues+Mind Numbing works... I will keep trying to test it...

wardz commented 4 years ago

Found some interesting patch notes:

Patch 2.0.3 (09-Jan-2007): "Mind-Numbing Poison" will no longer stack with both "Curse of Tongues" and "Slow".

Since Classic runs on patch 1.12.1 im assuming they still stack so I have added a fix for this now. (Still not 100% sure about the calculations, especially if something like Berserking is active at the same time you have Curse+Mind Numbing Poision but I think it should work good enough.)