AddonLoader

20 - X-LoadOn-Combat: false

Some addons do Very Bad Things if you try to load them during combat, eg when reconnecting after a dc on a boss fight.

Would like X-LoadOn-Combat to accept an argument of  "false" to mean that regardless of other conditions, loading should be delayed until after exiting combat.

User When Change
mikk Mar 17, 2013 at 16:25 UTC Changed assigned to from Ammo to mikk
mikk Feb 26, 2013 at 18:33 UTC
oscarucb Jul 22, 2011 at 06:23 UTC Create

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

  • Avatar of oscarucb oscarucb Mar 17, 2013 at 21:24 UTC - 0 likes

    I've committed a more general fix in r127:

    Delay all addon loading from "automatic" conditions that trigger during combat until after combat, unless the addon is specifically flagged for X-LoadOn-Combat. The user can still initiate "manual" loading in combat via X-LoadOn-LDB-Launcher or X-LoadOn-Slash.

    Lemme know what you think.

  • Avatar of oscarucb oscarucb Mar 17, 2013 at 20:50 UTC - 0 likes

    Thanks for looking into this.

    You've "fixed" it for the narrow case of addons loading via "X-LoadOn-Always: delayed", but it still fails in the same way for addons that load via any other condition that happens to fire in combat. For example, if you enter combat then join a group (as often happens if you load into LFR/LFG in progress), the X-LoadOn-Group addons will try to load and hit the time limit. I've just verified this happens with the latest r126, with the exact same stack as my Feb 23 comment. The same is true of many other load conditions that can fire in combat.

  • Avatar of mikk mikk Mar 17, 2013 at 16:25 UTC - 0 likes

    Fixed in R126.

  • Avatar of oscarucb oscarucb Feb 26, 2013 at 22:16 UTC - 0 likes

    No, swatter loads statically. The LoadAddon() was invoked by AddonLoader to load a big addon and was terminated by the limit, triggering swatter - which then is aborted on the next time slice while trying to scan the stack.

    I captured that particular error when zoning into the valentine's boss with several addons flagged X-LoadOn-Group. This year's valentines boss put you directly into combat upon zoning and group formation, and AddonLoader choked on it every time.

    I can get the same error by entering combat and then clicking an X-LoadOn-LDB-Launcher icon.

  • Avatar of mikk mikk Feb 26, 2013 at 17:53 UTC - 0 likes

    Is that Swatter taking too long to load? oO

    Edit: Hm or maybe some other addons before it took too long to load on the same event..

    Last edited Feb 26, 2013 by mikk
  • Avatar of oscarucb oscarucb Feb 23, 2013 at 13:02 UTC - 1 like

    With the advent of hard time limits on event handlers, this issue has become significantly more important. Attempting to load large addons while in combat (eg relogging after a dc during a boss fight, or queuing into an LFD/LFR already in combat) frequently causes an error like the one below.

    This error has nothing to do with the "quality" of the addon being loaded, and there's nothing the target addon can do to improve the situation. It's caused by AddonLoader's "dangerous" attempt to call a long-running LoadAddOn() from inside an OnEvent handler while in combat lockdown. The problem is exacerbated by the fact that AddonLoader tries to load ALL addons (and their dependencies) matching a given condition event (eg X-LoadOn-Group -> GROUP_ROSTER_UPDATE) within ONE event handler; however even if that was split up into individually-delayed handlers as it probably should be, the danger still remains with loading a single, large addon inside a handler during combat.

    Mikk's suggestion to wait on loading "delayed" addons until combat exit is a step in the right direction, but this problem can still easily occur with addons not marked delayed. With these real-time limits, it's really not 100% safe for insecure code to load ANY addon in combat, no matter what its annotations might say. There should probably be a global option to "never load addons in combat", possibly with a per-addon override (X-LoadOn-Combat: true?) for crucial addons the user wants to risk trying to load in combat.

    Message: ..\AddOns\!Swatter\Swatter.lua line 169:
       script ran too long
    Debug:
       !Swatter\Swatter.lua:169
          !Swatter\Swatter.lua:157
       !Swatter\Swatter.lua:227
          !Swatter\Swatter.lua:225
       [C]: debuglocals()
       !Swatter\Swatter.lua:169
          !Swatter\Swatter.lua:157
       !Swatter\Swatter.lua:227
          !Swatter\Swatter.lua:225
       [C]: LoadAddOn()
       AddonLoader\AddonLoader.lua:125 LoadAddOn()
       AddonLoader\AddonLoader.lua:146
          AddonLoader\AddonLoader.lua:135

  • Avatar of oscarucb oscarucb Oct 14, 2011 at 16:55 UTC - 0 likes

    As I mentioned in my previous comment, it doesn't really matter to me what the syntax is for specifying this new behavior to suppress loading in-combat. In most cases I'll be adding it manually in my user settings so the spelling is irrelevant - so do whatever you think makes the most sense.

    It seems reasonable for delayed addons to simply be "delayed" until combat ends (unless they explicitly request in-combat loading), since combat exit is the safest possible time to load things when relogging/reloading in combat.

  • Avatar of mikk mikk Oct 12, 2011 at 00:36 UTC - 0 likes

    Then again something else I've been pondering is simply having "delayed" mean "not in combat" and if someone wants stuff to be loaded in combat they can just go X-LoadOn-Combat: true.

    But I don't know if that's too big a change at this late point. But it's tempting! =)

  • Avatar of mikk mikk Oct 02, 2011 at 19:28 UTC - 0 likes

    Having delayed addons not load in combat is something I've been wanting myself. I was thinking something like X-LoadOn-Always: delayed nocombat

    This is backwards compatible and becomes simply "delayed" with old addonloaders.

  • Avatar of oscarucb oscarucb Sep 04, 2011 at 08:54 UTC - 0 likes

    Yes, in an ideal world every addon should be capable of loading under all conditions, even InCombatLockdown(). But the truth is many addons do not, especially those that have nothing to do with combat (auction helpers, etc). And since we can't force the world to fix all their addons, its nice for an intelligent addon loader to provide me the option to just delay loading them.

    There's another reason I want this - some of those very same addons tend to be large and slow-loading, and I really DONT want them to load (regardless of bugs) if I'm in a OMG I JUST DCD ON THE BOSS FIGHT scenario. Ie I want the ability to block loading of anything nonessential in the rare cases where I'm loading my UI in combat.

    And incidentally, I don't really care what the syntax is for telling AddonLoader to have this behavior, overloading X-LoadOn-Combat was just a suggestion and might not be the best idea for backward compatibility. I'd be equally happy with a new directive like: X-DontLoadOn-Combat

    Last edited Sep 04, 2011 by oscarucb

Facts

Last updated
Mar 17, 2013
Reported
Jul 22, 2011
Status
Accepted - Problem reproduced / Need acknowledged.
Type
Enhancement - A change which is intended to better the project in some way
Priority
Medium - Normal priority.
Votes
2

Reported by

Possible assignees