search

anzz1 / TacoTip

Better player tooltips - class colors, talents/specialization, GearScore & more - for Classic / TBC / WoTLK
8 stars 18 forks source link

LibClassicInspector attempt to mute CanInspect error speech #7

Closed ForestJ316 closed 1 year ago

ForestJ316 commented 1 year ago

From testing it seems like msg is always first and then sound after, so no race condition.

perinazzoo commented 1 year ago

its working? @ForestJ316

ForestJ316 commented 1 year ago

From what I tested, yes @perinazzoo

perinazzoo commented 1 year ago

@anzz1 any ETA to merge this?

anzz1 commented 1 year ago

I can't test it right now, but I could merge it if you have tested it and there are no issues. @ForestJ316 @perinazzoo

ForestJ316 commented 1 year ago

@anzz1 Been playing for a while and haven't noticed any issue so far. Wintergrasp seemed to be fine also, since I'm seeing a lot of ppl complaining about bgs triggering the error speech on curseforge comments. But take into account that I'm playing english locale. Doesn't seem like it should be an issue tho, unless blizzard decided to use different ID for outofrange vocal error on non-english locales of course.

anzz1 commented 1 year ago

Very probably not, I would think constants such as that are indeed, constant. We'll see in the comments if that isn't the case then. :smile:

What I'm more worried about is introducing taint, addon developers worst nemesis, which is the thing that causes those "Interface action failed because of an Addon" things which break the UI. Those kinds of pre-hooks can be dangerous since it taints the execution path that if an Blizzard function would call it and then do a "secure action", it could be blocked.

But such a thing like playing a error sound would probably happen at very tail-end of function execution and thus would not introduce taint, as was the case with the AddMessage hook.

Since you tested it and have had no problems, I can push it.

I'll get it merged this weekend and push an hotfix update. Thanks for your help !

anzz1 commented 1 year ago

Merged, released in 0.2.8 and pushed to addon channels. Thanks !