WowAce

What is the enhancement in mind? How should it look and feel?
Rather than store event hooks based on the spell name, store event hooks based on spellIDs.

Please provide any additional information below.
By storing by spell ID rather than name, we bypass the need to localize event triggers, which is a major task. It also eliminates the potential problems of typos. Spell ranks will no longer be a problem when Cataclysm is released in a few months. By adding a link parser, users can add spell effects they've seen and not seen by typing /ss create [spelllink] or /ss create ID (taking the ID from a database site).

This is a transition that will help, but there are questions that need to be addressed in the design.

I've thought about doing this before, but there are many events that don't have a spell id. It would work fine for UNIT_SPELLCAST_SENT and similar "when I start casting" style spell announcements, however...

• Most buffs and debuffs don't have spell ids (or the spell id is difficult to identify)
• Many buffs and debuffs have the same name as the spell, but the buff isn't the same event as the spell. Consider Arcane Intellect. If you "/ss create [spelllink]" do you mean "When I start casting" or "when I successfully cast" or "when someone buffs me with" ...? I'm not 100% sure whether the buff has the same spell id as the spell. Redefining the event keys from "<eventtype><name>" to "<eventtype><spellid>" would still preserve the event type, but the UIs based on the ID would have to infer an event type or be more sophisticated. This is still a good step in the right direction.
• Lots of non-spell events like talking to a mailbox that will never have a spell ID. If everything else moves to spell IDs, how do we continue supporting those?

We should change the names to spell IDs where possible for starters, and go from there. EventTable[key] would still use a string key like "UNIT_SPELLCAST_SENT65235" instead of "UNIT_SPELLCAST_SENTBRONZE_DRAKE" (made up the number for an example, not sure what the spell ID is for anything)

• Nope, every single buff and debuff in the game has an associated spellID number. Go to Wowhead and search for the craziest buffs you've ever seen. You will find it, I promise. :) I see handling it as the following 3 ways:

1. Speakinspell automatically detects the buff/spell, and saves it under its ID rather than its name
2. The user types /ss create #. Spell is stored under that number.
3. The user types /ss create [spelllink]. Passed into the addon will be a string that looks kind of like this: "\124cff71d5ff\124Hspell:50303\124h[Swine Flu]\124h\124r" The spell ID is the bolded number, located after the colon and before the next backslash.

We can give instructions for obtaining that link/number in the user manual. I think having the option to just paste it would be convenient.

• Rather than creating an actual event, /ss create [spelllink] should add the spell to the list as if Speakinsay had detected it being cast. That's what I meant to say. /bonk
  
• Oh, I didn't mean spell IDs for EVERY SINGLE EVENT - I just meant the ones that HAVE spell IDs. :) Which, really, is a large portion of them.
  

I don't know, maybe we only use numbers for the default events? Maybe add ID numbers as an additional option and still support text IDs?

Just a note so I don't forget - BigWigs has an array of spell ID numbers (presumably for 10, 10H, 25, 25H abilities) for a single event. If we move to spellID support, I think this would be a good idea. We can include it for just the default events, or we can give the user the option to add many spellIDs just like they can add many announcements to a single event.

SS 4.0.3.05 improves handling of spell IDs.

The SpellIdCache has been removed, because it was used for a slow sequential search to find a spell's ID from its name. It was causing lag.

Now we get the spell ID from the API hooks: UNIT_SPELLCAST_START, STOP, SUCCEEDED, etc. This is new from Blizzard in WoW 4.0.1, before which, the spell IDs were never provided by the API hooks.

The patch function we'd have to create for this presents a challenge... how to convert names to IDs to preserve old data? There are 2 basic approaches, and both present challenges.

1. GetSpellInfo(name)

Blizzard kinda fails at this because GetSpellInfo(name) throws a Lua error on unknown spell names (or it used to before cataclysm).

On the one hand, it should be possible to be smart about avoiding that, and only pass it names we think are real spells (events related to UNIT_SPELLCAST_XXX). However, pre-cataclysm data is still floating around the SpeakinSpell community and may include spells that no longer exist, which would easily trick that simple logic to attempt GetSpellInfo on a spell that no longer exists, and then boom Lua error.

If a patch throws a Lua error, as I fear that logic would be prone to do, then the patch never completes, and SpeakinSpell never fully loads properly. It should be retested whether GetSpellInfo(name) still throws a Lua error on unknown spell names, or whether that's handled more gracefully by Blizzard now to return nil values without popping up a Lua error. It might be OK if Blizzard has fixed GetSpellInfo(name) to gracefully return nil for failures, instead of crashing to a Lua error popup. However, even if that's safe, I'm still concerned about getting the correct ID from this which I'll get to below...

2. GetSpellInfo( ID ) for all IDs from 1 to 100k

Alternatively, we would have to use the old method that used to be attached to the SpellIdCache (prior to SS 4.0.3.05), which was a sequential search of all spell IDs from 1 to 100k until we find one with a matching name.

Unfortunately, I've seen that it's not a one-to-one relationship. Blizzard defines multiple spell IDs with the same name. For example when casting Ritual of Refreshment, there are two different spell IDs reported by the API hooks, both called Ritual of Refreshment. Worse: In that specific case, the lower-numbered spell ID is actually not the one we want... so this method would break my mage table announcements because it would convert "Ritual of Refreshment" to the wrong spell ID. I'm not sure if this would apply to any other events, but I assume it probably would.

So unfortunately, I don't see any reliable way to convert the saved data from using names to using the correct spell IDs

3. Make Duplicate Event Triggers

In the past, I did a patch that made multiple copies of event triggers which were ambiguous, to cover all possibilities, which is an approach that may present a solution. Long ago when I updated the data to distinguish between spells and buffs, older data had never made that distinction before, and in some cases, it was impossible to tell whether a given event name indicated a spell or a buff, so I created both in the patch: UNIT_SPELLCAST_SENT<name> and SPELL_AURA_GAINED<name>

So perhaps we would have to do this in the transition to spell IDs, to create a new event key for all matching spell IDs instead of only the first match we find. For my Ritual of Refreshment example above, this would result in 2 event triggers in your SpeakinSpell data, where UNIT_SPELLCAST_SENTRITUAL_OF_REFRESHMENT would become both UNIT_SPELLCAST_SENT87123 and UNIT_SPELLCAST_SENT42123 (numbers made up from fuzzy memory). The one in the 80k range is the desired trigger. The one in the 40k range is a funky additional spell ID. I think this identifies the separation between the ritual (spell ID in the 80k range) and the table itself (spell ID in the 40k range) but I couldn't find adequate documentation on that point.

The fact that Blizzard did stuff like that presents an additional challenge to the end-user to select the correct spell ID for Ritual of Refreshment (and similar spells, if any) which makes it prone to user error in a way that keying on names is not. However, keying on names is ambiguous in a way that may present the same kind of "prone to operator error" scenario from a different angle.

4. Set it up By Hand

The only other way I see is the daunting task of manually creating a conversion table to map all 100k spell names to their IDs, for the appropriate name to ID conversions... This would allow us to be smarter about it and do all the thinking in the way we setup this table, and map Ritual of Refreshment to the ID in the 80k range using a lookup table that we build by hand instead of going to Blizzard with GetSpellInfo().

We could write some Lua code to get the table going to start with, but it would take many hours of work to thoroughly review that table and select the correct IDs where the same spell name comes up more than once. It might take hundreds of hours.

Maybe that task could be reduced by only looking at the subset of known tricky conversions like Ritual of Refreshment... but how to identify the other problem cases, if any exist, and which ones?

In experimental code, we could build the SpellIdCache by looping through all IDs from 1 to 100k. During this process we could analyze the results to identify any name collisions where multiple spell IDs map to the same name. From there we could build a new table of the collision names, which I hope is a somewhat short list.

From there it would be a manual process to review each of those spells and figure out which is the appropriate spell ID that SpeakinSpell wants to use.

Then remove the experimental code that we used to build this table, and put the resulting data table into SpeakinSpell_Patches

I got a bug report from someone that the spells Fury of the Eagle, and Fury of the Illidari basically proc about a hundred times.  If I understand what's happening correctly, Fury of the Eagle "unleashes an unrelenting series of frontal spear strikes" which Blizzard implemented as 100 UNIT_SPELLCAST_SENT events also called "Fury of the Eagle" though presumably with different spell ids.

Here is the whole message I received from a SpeakinSpell user: