You must login to post a comment. Don't have an account? Register to get one!

• 

  • #7

  lilsparky Fri, 02 Oct 2009 17:52:30

  i'm sort of inbetween releases right now.  i do intend to stick this support file in there, but it might be a few days before i can shore up current development...

• 

  • #6

  kolenka Sat, 19 Sep 2009 00:19:48

  While I'd agree with you on principle, I see some technical hurdles here.

  LSW assumes that by the time OnLoad is called, everything is registered. The problems with this are:

  • MrTrader_SkillWindow should be loaded before OnLoad, which means either you have to extend your LoadWith entry anyways, or I get to become an addon loader for LSW.
  • If MrTrader_SkillWindow is loaded before you, but I don't become a loader, then I still need to wait for you to be loaded and register then. Waiting for ADDON_LOADED here may not reach me before it reaches you.
  • Even if I become a loader, I need to run my registration before OnLoad() gets called in LSW, but after you load the scripts. This makes for another interesting race condition, as I have no idea when ADDON_LOADED gets called (and I don't see much tribal knowledge on the subject). I'd be surprised if LoadAddOn() returned before ADDON_LOADED is fired, as it would mean that you could talk to an addon before it was loaded when you load modules.

  Keeping the script with LSW avoids these race conditions, and keeps the inter-addon knowledge one-way, and because of that, has fewer points of failure.

  My two cents on this.

• 

  • #5

  lilsparky Fri, 18 Sep 2009 05:29:44

  actually, while i have no problem with putting this into lsw, there's no reason this file couldn't be loaded from mrtrader's toc file.  i would prefer the latter simply because i didn't write the code, but it's not a major issue.

  let me know if that scheme presents any problems for you or if you'd rather it be packaged into lsw directly.

• 

  • #4

  kolenka Thu, 17 Sep 2009 17:45:23

  Okay, I've got things sorted out, and this version of the support file should be good to go.

  You will need to make sure you add "MrTrader_SkillWindow" to the list of addons you load with for this to work. I do my own delay loading, and you won't be able to rely on the Blizz UI loading as a sign mine is loading (I actually prevent the Blizz UI from loading unless my window is disabled to save RAM).

• 

  • #3

  lilsparky Thu, 17 Sep 2009 09:11:59

  you should look at the way skillet support is done and see if that might be a better way to go.  ignore the sorting functions.

  skillet provides an api for hooking the button draws/hides (it checks a registry of function calls and will fire those off prior to calling show or hide for a recipe button).  it makes having to know the button names and monkeying with show/hide hooks not an issue.

  something like:

  MrTrader:RegisterRecipeButtonOnShowFunction(myFunction)

  (or whatever you wish to call it -- one also for Hide)

  then prior to your :Show()/:Hide() for those buttons, you'd iterate over the list of registered functions and call them passing the button (and additional arguments you think might be nice, but the button is all that's really necessary i think).

  this would mean lsw support would only have to call a few functions to register the necessary functions and you could change your internal workings to whatever names or anything you'd like.

  this could also be extended to the window updates/show so that lsw isn't touching your show/update functions and is instead simply registering itself with mrtrader...

  doesn't really change much functionality, but it makes for a cleaner connection between the two without lsw having to infringe on mrtrader's personal space, so to speak.

  actually, i take that back.  it DOES change one key aspect of functionality in that Show() scripts don't fire for buttons that are already visible.  this might be the root of the biggest issue with the mrtrader support -- buttons that are already visible don't have the lsw functions called for them.  going with the "preshow" hook registry eliminates this quirk.  otherwise you need to add in extra Hide() calls to force the Show() to trigger the script.

• 

  • #2

  kolenka Thu, 17 Sep 2009 05:28:07

  As a note on this, the new version of MrTrader (0.5.1) includes quite a bit of cleanup on the skill window's code so that it would be easier to maintain going forward. I decided to do it sooner rather than later to minimize impact on other addons before they started depending on mine.

  MrTrader_SkillButton should now be MRTSkillButton in the attached file.

  Sorry for any inconvenience this causes.

• 

  • #1

  lilsparky Wed, 16 Sep 2009 06:44:33

  i'll take a look.