SpeakinSpell

119 - Modularization

When you load the entire addon, it takes up a huge amount of memory - 6.5MB to start, and going up from there. By breaking down SpeakinSpell into several modules, the user can load only what is needed and thus reduce the memory footprint. The proposed modules are:

-Core
-GUI
-Sample Speeches
-Communication

More to be added as needed.

User When Change
Duerma Aug 14, 2010 at 00:11 UTC Changed status from Accepted to Started
rismisner Aug 11, 2010 at 00:47 UTC Changed assigned to from None to Duerma
Duerma Aug 05, 2010 at 00:36 UTC Create

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

  • Avatar of rismisner rismisner Nov 20, 2010 at 22:17 UTC - 0 likes

    4.0.3.01 splits out SpeakinSpell_Defaults into another separate module containing all the default speeches and event hooks

  • Avatar of rismisner rismisner Oct 21, 2010 at 04:36 UTC - 0 likes

    I modularized SpeakinSpell_Patches.  This module contains the functions needed to upgrade your saved data for compatibility with a new version of SS, only needed once per character the first time you login with a newer version of SS installed.  EDIT: this module first released in SS 4.0.1.01

    Last edited Oct 21, 2010 by rismisner
  • Avatar of rismisner rismisner Sep 18, 2010 at 20:27 UTC - 0 likes

    I think we should add a checkbox to preload the GUI, and maybe other modules too, allowing the end-user to essentially de-modularize the loading.

    The issue involved here is that if you try to access the SS GUI by going to Escape > Interface Options > Addons ... it won't show up there unless you've already loaded the SS GUI module by executing some related command internal to SS (like "/ss options" or the minimap button).

    Going strictly through the Blizzard interfaces, SS doesn't know the user might be searching for its options.

    Something tells me that the newbie user is going to do that, and in 3.3.5.10, they won't find the options.  The majority of newbies will probably click the minimap button, or /ss something, which works fine in 3.3.5.10.  However, the default for new users should probably be to load the GUI module at startup until they go into a performance tuning area of the GUI to change a checkbox to load the GUI only as-needed, where the tooltip will warn that you have to run a /ss command or click the minimap button or [chat link] to open the GUI.

    On the other hand, not all addons put their options in the blizzard interface options frame, so it's not necessarily a requirement that SS GUI must be accessible in that way... it's a thinker for usability...

    Some other LoD modules won't need anything like that, for example when we spin off default speeches into a LoD module, that's entirely behind the scenes, and never has to be pre-loaded.  (it's won't even be loaded for time-sensitive operations, so there's not even a performance reason to think that the end-user's machine might run better if they could set this to be preloaded)

  • Avatar of rismisner rismisner Aug 19, 2010 at 21:50 UTC - 0 likes

    I've been thinking about a couple of things we might want to pursue to further build on this modularity with the GUI. These are just things to consider. I am not sure what will actually work out well and I haven't made any decisions. So these are just talking points...

    1. Locale Strings
      • About half of the Locale-xxXX.lua file is for the GUI: checkbox labels, tooltips, etc. There's some overlap with locale strings used by the core, but a big chunk of the locale table could be moved into the GUI. The point is: we don't need to load those locale strings unless the GUI is loaded, so this could further shift more RAM from the core to the GUI.
      • The code might get a little tricky to maintain. We'd have to load two locale tables in most of the gui code: the gui's L[] and the core's L[]. This is possible but would require a new naming convention like LCORE[] vs. LGUI[]. It shouldn't be too hard to adapt the buildlocales.py script to cope with something like this to help with maintenance.
    2. Ace Libraries
      • Some of the Ace libraries, notably AceDialog, are only needed by the GUI. We don't need to load that for the core. We could look into moving some libs into the GUI module.
      • This poses a challenge where currently all of the GUI code adds functions into the SpeakinSpell table/namespace, which is instantiated to inherit from various Ace libs. I'm not sure how to "late bind" to AceDialog if that's not listed in the AceAddon:NewAddon() function call. There is a way to do it, but would require some research.
      • This line of thought also ties directly to my next idea...
    3. Consider making SpeakinSpellGUI more thoroughly its own separate addon.
      • The idea here is that instead of loading functions that augment the SpeakinSpell table (namespace) the gui functions would be more thoroughly modularized into a totally separate object called SpeakinSpellGUI, like SpeakinSpellGUI = AceAddon:NewAddon("SpeakinSpellGUI", ...)
      • Everywhere that GUI functions are called, instead of SpeakinSpell:GUI_SomeFunction() we would have SpeakinSpellGUI:SomeFunction() which looks cleaner
      • This kind of change would wreak havoc on all of the use of "self" throughout the GUI code, much of which would have to change to refer to SpeakinSpell:SomeFunction wherever the GUI calls functions in the core.
      • This kind of change could be useful in driving a thorough code review, but would require a lot of work
      • This would allow the GUI to bring in a different set of libraries than the core, allowing the core to stop pulling in those libraries, like AceDialog. However, some libraries would have to be duplicated in both addons, like AceLocale.
        • AFAIK, shared libraries are only loaded once by all of the addons that share them, given the logic defined in LibStub, so it shouldn't matter. So for example if any other addon already loaded AceDialog, then it doesn't matter if we load it from the core or the GUI module, because it's already loaded anyway by some totally unrelated addon.
  • Avatar of rismisner rismisner Aug 17, 2010 at 21:32 UTC - 0 likes

    I was inspired to do a little more work on this today, and committed some related changes to SVN, but I haven't tested any of this yet.

    1. I created a new file called guiwrapper.lua, which lives in the core SpeakinSpell module.
    2. I moved the LoadAddOn call and related logic from the minimap button code into the new guiwrapper, in a new function called LoadGUI.
    3. I moved all of the ShowPage functions from gui.lua into guiwrapper.lua, which is moving them from the SpeakinSpell_GUI module back into the core SpeakinSpell module. This makes them safe to call, without fear of whether the GUI module has been loaded or not. We know that the GUI will have to be loaded for those functions, so...
    4. All of the functions in guiwrapper.lua (which is all of the ShowPage functions, like ShowHelp, ShowColorsGUI, etc) now call LoadGUI and make sure it succeeds before calling the blizzard API to open the interface options to the requested window.
    5. Elsewhere in the code where GUI functions are called to refresh or update the GUI, they now use the global flag SpeakinSpell.IsGUILoaded to check whether they should call some function, instead of checking if that function exists. This style is less risky of mistakes when changing the code, and a bit more readable about showing why a function would exist or not.

    I haven't tested any of this yet, and I hope I didn't step on your code changes in this area and cause you merge conflicts if you've been working on the same thing. I just felt it should be set up this way and had some extra time to do it.

    So this should make all the slash commands and clickable links in chat work now, to load the GUI on demand as needed, as a general case with shared code, rather than specifically tied to the minimap button.

  • Avatar of rismisner rismisner Aug 17, 2010 at 01:12 UTC - 0 likes

    I undid my hacks to SpeakinSpell.toc so I can load SpeakinSpell_GUI as a separate module correctly now. The minimap button is loading it correctly for me, so that's a fair work-around while we iron out the other bugs, like the slash commands.

    To setup my work environment, I did this:

    I copied /addons/SpeakinSpell/gui/ to /addons/gui/ then renamed it to /addons/SpeakinSpell_GUI/ (NOTE: not renamed in SVN, just in my local file system)

    That included the hidden folder at /SpeakinSpell/gui/.svn/ so that my /addons/SpeakinSpell_GUI/ folder points at the same place in SVN as my /addons/SpeakinSpell/gui/ folder (which also co-exists with the rest of this)

    I can make my changes in either folder, commit to svn, then update the other folder from svn to keep them in sync. /addons/SpeakinSpell_GUI/ contains the files I'm running in the game now, while /addons/SpeakinSpell/gui/ contains the files that come up in my AddOn Studio editor. So I usually make my changes in /addons/SpeakinSpell/gui/, then commit to svn, then use svn to update /addons/SpeakinSpell_GUI/ so I can test in-game.

    I think that setup is good, because I usually commit untested changes anyway (with a thorough code review of the diffs every time) but I might change the wowsln file to point directly to the /addons/SpeakinSpell_GUI/ folder. /shrug

    Last edited Aug 17, 2010 by rismisner
  • Avatar of Duerma Duerma Aug 15, 2010 at 21:53 UTC - 0 likes

    Gah. I thought I had cornered all the gui functions but I forgot about the slash functions. I won't be able to fix it today, though - my whole family's kind of illin' and my head feels like it's filled with cotton. >.< The GUI isn't completely gone, though - if you have the SpeakinSpell_GUI folder separated properly, then right clicking the minimap button will open it up.

    Other problems: I was running Carousel and Fortress as my LDB displays. When I right clicked the minimap button to create the GUI, instead of it being properly created, it was created as a subset of options in the Carousel options.

    SO sorry - I'm usually more thorough than this. I wasn't thinking when I committed last night, though I guess this way it gives you a start of something to work on. >.<

  • Avatar of rismisner rismisner Aug 15, 2010 at 20:50 UTC - 0 likes

    I started a little bit on re-adding the GUI as a separate module.

    We need some code that will load the GUI on demand when it's demanded. I did "/ss" a minute ago and got an error because that tries to call a function in gui.lua. I assume there's an API we can call to load a LoadOnDemand module? and unload it later when we're done with it?

    I found out that the TOC file won't complain or throw any error messages if it lists a file that doesn't exist, so I restored the links to all of the /gui/ files. Normal end-users will have those files in a separate folder with its own LoadOnDemand TOC file, loading the same files in a different way. For devs (like us) who are using SnS from an SVN checkout where the .pkgmeta has not been applied, this allows us to load the gui all within a single folder. It simultaneously blocks us from testing the modularity though, so you have to rename or move the /gui/ directory to load it separately. If there's a better way to handle this, let me know. I guess I could do a separate SVN checkout?

    I'm not sure where to go with it from here.

    CreateGUI() isn't being called, so the GUI can't be opened at this time, even if the files are loaded by my toc file hacks...

    EDIT: I added a temporary hack to SpeakinSpell.lua to CreateGUI if the function exists, so I could access the GUI to work on a different issue while we're working through this modularization transition

    Last edited Aug 15, 2010 by rismisner
  • Avatar of Duerma Duerma Aug 14, 2010 at 00:29 UTC - 0 likes

    OK, I have successfully divorced the GUI from everything else, which reduced the memory footprint by 1.1-1.4 MB when it is not loaded. I'll upload the changes sometime tonight.

  • Avatar of rismisner rismisner Aug 11, 2010 at 00:47 UTC - 0 likes

    I'm assigning this to you just because you seem to understand how to do load-on-demand, so I'm going to let you be the owner of the ticket, even though I'm sure we'll have to collaborate on various technical issues that will be involved.

Facts

Last updated
Aug 14, 2010
Reported
Aug 05, 2010
Status
Started - Work on this issue has begun.
Type
Enhancement - A change which is intended to better the project in some way
Priority
Medium - Normal priority.
Votes
0

Reported by

Possible assignees