View Single Post
07-14-05, 06:03 PM   #22
Legorol
A Cliff Giant
AddOn Author - Click to view addons
Join Date: Jul 2005
Posts: 79
A very nice AddOn

I have just noticed this AddOn thanks to the publicity it received on the official forums. Good job, Iriel :-) It does look very nice and very useful. However, I would like to make a couple of small comments:

1) In make_hook, you create a function that has ellipsis (...) as argument, so that it can receive multiple arguments. You then use unpack(arg) to pass these onto the hooked function. While this is very nice style-wise, the use of the ellipsis itself generates garbage, and therefore might bias your statistics. Consider replacing it with something like say 20 static arguments, such as: "return function(a1, a2, a3, a4, etc.)", and then pass a1, a2 etc. to the hooked function. It will look ugly but eliminates garbage generation.

2) You are using the Class:Method syntax to enter functions to watch that are elements of a table. I suggest (and hope) that you use the Class.Method syntax instead. Although the choice is arbitrary from the entry point of view, it ties in better with the Lua concepts of elements of a table (Also see point 3). I know that usually the way to call most methods of XML objects is via the Class:Method syntax (e.g. GameTooltip:Show()), but in Lua, the colon is really nothing special, it's just a shorthand for Class.Method(Class, ...). In otherwords, "GameTooltip:Show() = GameTooltip.Show(GameTooltip)". Also, if you are trying to hook a function, you would specify GameTooltip.Show as the function to hook and not GameTooltip:Show.

3) Do you think you will support functions that are more than one layer deep in a table? For example Class.Subclass.Function? I don't know if you are familiar with the Sea library, but if you need some inspiration on how to do something like this, Sea has two functions Sea.util.getValue and Sea.util.setValue. Those work like getglobal/setglobal, but allow table elements as their arguments too. Their code might be a useful starting point.

4) I don't know if you noticed this, but unfortunately due to the way Blizzard coded things, you can't enumerate the native methods of tables that are representing XML-defined objects. You only have access to methods if you know their name exactly. In other words the following code snippet,
Code:
 for k, v in GameTooltip do
  -- do something with k and v
 end
only returns those elements of the GameTooltip table that have been added from Lua. So if you are using it to iterate over the methods of GameTooltip, you only see those methods that have been modified from Lua, for example hooked into by some AddOn. This means that regular expressions don't work reliably with methods of XML-defined objects. You can see this for yourself: typing "GameTooltip:.+" into TraceEvent results in very few methods being traced. As far as I know, there is no way at the moment to enumerate the native methods.

Apart from the above comments, let me again say that you have a great AddOn here that I am going to be using from now on!
  Reply With Quote