View Single Post
07-26-13, 04:55 AM   #9
AnrDaemon
A Chromatic Dragonspawn
AddOn Author - Click to view addons
Join Date: Jul 2008
Posts: 156
Originally Posted by Phanx View Post
If you don't need the quest name immediately (eg. you can wait for OnTooltipSetQuest to happen) then that will work, though I'd suggest cleaning it up some. In no particular order:
Yes, I largely don't need it immediately. Certainly not in the same frame - I have no intention to get WoW stuck on every other window opening.
Though, that previous example with metatable will be a little better for when I need to store and handle a list of quest names during a single session.

* Making a table in your slash command handler like that is pretty horrifying
You refer to
Code:
local argv = { strsplit(" ", strtrim(msg)) }
?
Then how do you suggest I prepare it? I have no idea, how many words will be in parameters.

* There's no need to set and un-set the script, since your code is the only code that's going to be setting hyperlinks to that tooltip. If you're worried about it, check the self.message value instead.
I'm unsetting it to prevent repeated access to handler, where I don't want it. also to save some processing. Of course, I could just pupulate some other variable with necessary data, and use it from some other function... Isn't that "a bit" too much of overhead?

* "self" isn't a very good thing to call the second arg passed to your slash handler, since that arg refers to the editbox the user typed the command in, and not to any "self" as used anywhere else in your addon.
"self" Is always the first argument passed to class function. Where do you see it as second?

* You can cut the number of calls to string.format in half.
Yes, I can. I just left it this way to reduce amount of unrelated stuff referenced in far-reaching places.

* You should :Hide() your tooltip after getting the text.
I never Show() it in first place. However, I can set OnShow = Hide, if you insist.

Edit: You posted a revision while I was posting mine, but most of the above is (probably) still applicable. Also, either your new code should be throwing errors left and right because you're using argv in a scope where it's not defined, or that isn't your real code.
That is a part of real code, a part of real slashcommand handler. (There's lots more of unrelated stuff handled, I just extracted the relevant block for demonstration.)

Last edited by AnrDaemon : 07-26-13 at 05:01 AM.
  Reply With Quote