+ Reply to Thread
Page 1 of 2 1 2 LastLast
Results 1 to 15 of 19

Thread: Event tables: Uniqueness, removing events, and so on

  1. #1
    RIFT Community Ambassador the_real_seebs's Avatar
    Join Date
    Jan 2011
    Posts
    16,859

    Default Event tables: Uniqueness, removing events, and so on

    Okay, general observation:

    The table.insert(Event.foo, { function, addon, description }) idiom has left us with ambiguities as to how to determine whether a given event handler is already registered, and if so, where.

    Question: Does it ever make sense to have the same function registered more than once for a given event? So far as I can tell, the only possible way would be if you were using the addon identifier for further dispatching. Is anyone likely to need to do that? Because any other way of getting context in is likely to be closures, which make distinct functions.
    You can play WoW in any MMO. You don't have to play WoW in RIFT. Oh, and no, RIFT is not a WoW clone. Not having fun any more? Learn to play, noob! I don't speak for Riftui, but I moderate stuff there. Just came back? Welcome back! Here's what's changed. (Updated for 2.5!)

  2. #2
    Plane Walker Imhothar's Avatar
    Join Date
    Feb 2012
    Posts
    439

    Default

    Just for completenes sake and anyone who didn't know about it yet here's the other thread on table.remove in event tables.

    I agree that table.insert naturally introduces an idiom which suggests using table.remove as complement. And technically, that would be fine if nobody relied on the index after table.insert to stay constant. I think many people don't realize how table.remove causes existing indices to shift.

    As long as there are addons out there which rely on storing the index of an event table value, removing entries will break those. And by being broken, they automatically break others. It's a vicious circle.

    On the question about multiple entries: Sometimes it makes sense. When you have multiple frames of the same class, loaded on demand, then it makes sense to hook events in the class constructor. Makes life easier. Yes, it could be done differently with a central dispatcher, but oh well...

  3. #3
    Champion
    Join Date
    Jun 2011
    Posts
    561

    Default

    I observed that even if you try to identify your exact injection point and try to remove your even entry it will break the event for other addons. At least Event.Unit.Available shows that behaviour.

    table.remove on an event table should never ever be done. I really would suggest to implement a wrapper around the table.insert (...) stuff for events. Something like register.event() or whatever so that addon authors who are maybe not that much experienced are not tempted to do a table.remove().

    Cheers
    N.
    Last edited by Naifu; 04-23-2012 at 12:08 PM.

  4. #4
    RIFT Community Ambassador the_real_seebs's Avatar
    Join Date
    Jan 2011
    Posts
    16,859

    Default

    Quote Originally Posted by Naifu View Post
    I observed that even if you try to identify your exact injection point and try to remove your even entry it will break the event for other addons. At least Event.Unit.Available shows that behaviour.
    Details? I've been messing with this on and off and had not encountered it.
    You can play WoW in any MMO. You don't have to play WoW in RIFT. Oh, and no, RIFT is not a WoW clone. Not having fun any more? Learn to play, noob! I don't speak for Riftui, but I moderate stuff there. Just came back? Welcome back! Here's what's changed. (Updated for 2.5!)

  5. #5
    Champion
    Join Date
    Jun 2011
    Posts
    561

    Default

    Quote Originally Posted by the_real_seebs View Post
    Details? I've been messing with this on and off and had not encountered it.
    See my bug report => http://forums.riftgame.com/beta-addo...ml#post3748907

    Try that two or three liner I put in there and load either KA or OS. Might be that these two addons are not properly removing their events and I only spent a quick look. However it looked to me that at least Othersight is doing it somewhat 'properly'.

    Anyway build that test addon and load Othersight and you'll see that the dump will never appear.

    Cheers
    N.

  6. #6
    Plane Walker Imhothar's Avatar
    Join Date
    Feb 2012
    Posts
    439

    Default

    Quote Originally Posted by the_real_seebs View Post
    Details? I've been messing with this on and off and had not encountered it.
    See my thought experiment here: http://forums.riftgame.com/beta-addo...ml#post3658251

    One single addon which stores the index of its entry in an event table can break other addons (and itself) when a different addon removes from event tables.

    It's quite an interesting case: on the one hand the addon author assumes the event table to be immutable (because he assumes the index does not change) but on the other hand he modifies the table by removing elements, meaning it cannot be immutable. It contradicts itself...
    Last edited by Imhothar; 04-23-2012 at 02:23 PM.

  7. #7
    Rift Disciple chuckySTAR's Avatar
    Join Date
    Feb 2011
    Posts
    152

    Default

    Removing:
    Code:
    event[key][1] = function() end

  8. #8
    RIFT Community Ambassador the_real_seebs's Avatar
    Join Date
    Jan 2011
    Posts
    16,859

    Default

    Quote Originally Posted by chuckySTAR View Post
    Removing:
    Code:
    event[key][1] = function() end
    Two problems:
    1. What if the "key" you stored when you made your event is no longer your event?
    2. This prevents the "table is empty" optimization from taking effect.

    ... come to think of it, does that optimization work on an Event table that's been used but is currently empty?
    You can play WoW in any MMO. You don't have to play WoW in RIFT. Oh, and no, RIFT is not a WoW clone. Not having fun any more? Learn to play, noob! I don't speak for Riftui, but I moderate stuff there. Just came back? Welcome back! Here's what's changed. (Updated for 2.5!)

  9. #9
    Rift Disciple chuckySTAR's Avatar
    Join Date
    Feb 2011
    Posts
    152

    Default

    I use this function to unregister my events:
    Code:
    function module.UnregisterEvent(event, func)
    	local dummy = function() end
    	for key, value in pairs(event) do
    		if type(value) == "table" and value[1] and value[1] == func then
    			event[key][1] = dummy
    			return
    		end
    	end
    end
    Insert via:
    Code:
    tinsert(Event.Combat.Damage, {eventsDamage,  "RiftMeter", "Handle Damage"})
    Remove via:
    Code:
    RiftMeterUtil.UnregisterEvent(Event.Combat.Damage, eventsDamage)
    where eventsDamage is my function

  10. #10
    RIFT Community Ambassador the_real_seebs's Avatar
    Join Date
    Jan 2011
    Posts
    16,859

    Default

    So if you do that a bunch of times, you end up with 20 events all of which have an empty function that's being called with the event. And since obviously modifying the event table isn't destructive to other events, that's table copies, not just function calls.

    Seems... not good.
    You can play WoW in any MMO. You don't have to play WoW in RIFT. Oh, and no, RIFT is not a WoW clone. Not having fun any more? Learn to play, noob! I don't speak for Riftui, but I moderate stuff there. Just came back? Welcome back! Here's what's changed. (Updated for 2.5!)

  11. #11
    Rift Disciple chuckySTAR's Avatar
    Join Date
    Feb 2011
    Posts
    152

    Default

    Than make
    Code:
    local dummy = function() end
    global.
    Define a new function RegisterEvent() and before inserting into the event table look through the table for your dummy function and if found replace it

  12. #12
    Champion Lorandii's Avatar
    Join Date
    Jun 2011
    Posts
    516

    Default

    Trying again, just for the sake of it.
    Code:
    local MyAddon = {}
    
    -- event values
    local unitAvail = {MyAddon:UnitAvailable, "MyAddon", "Check if unit is available"}
    local unitUnavail = {MyAddon:UnitUnavailable, "MyAddon", "Check if unit is unavailable"}
    
    -- register events
    table.insert(Event.Unit.Available, unitAvail)
    table.insert(Event.Unit.Unavailable, unitUnavailable)
    
    -- unregister events
    -- first call the function passing an event table name
    MyAddon:UnregisterEvent(Event.Unit.Available)
    MyAddon:UnregisterEvent(Event.Unit.Unavailable)
    
    function MyAddon:UnregisterEvent(event)
        if type(event) ~= "table" or not event then return end
        for index, value in ipairs(event) do
            if value[2] == "MyAddon" then
                table.remove[index]
                break -- so it stops going through the event lists
            end
        end
    end

  13. #13
    RIFT Community Ambassador the_real_seebs's Avatar
    Join Date
    Jan 2011
    Posts
    16,859

    Default

    Hmm, what if I have more than one handler for an event?
    You can play WoW in any MMO. You don't have to play WoW in RIFT. Oh, and no, RIFT is not a WoW clone. Not having fun any more? Learn to play, noob! I don't speak for Riftui, but I moderate stuff there. Just came back? Welcome back! Here's what's changed. (Updated for 2.5!)

  14. #14
    Champion Lorandii's Avatar
    Join Date
    Jun 2011
    Posts
    516

    Default

    Quote Originally Posted by the_real_seebs View Post
    Hmm, what if I have more than one handler for an event?
    I can't see a reason for doing that, but if it must be done, remove the break entry. A better solution is assign one function handler per event, which then could call other functions as required.

    Will this properly remove all event handlers for a specific addon without breaking other addons?
    Code:
    local unitAvail = {UnitAvail, "MyAddon", "funcDesc"}
    local unitUnavail = {UnitUnavail, "MyAddon", "funcDesc"}
    
    -- register events
    table.insert(Event.Unit.Available, unitAvail)
    table.insert.(Event.Unit.Unavailable, unitUnavail)
    
    -- unregister all events
    function MyAddon:UnregisterAllEvents()
        for index, value in pairs(Event) do
            if value[2] == "MyAddon" then
                table.remove[index]
            end
        end
    end

  15. #15
    RIFT Community Ambassador the_real_seebs's Avatar
    Join Date
    Jan 2011
    Posts
    16,859

    Default

    Nope, adding or removing inside a table.

    Pretty sure the cleanest lua idiom is:

    Code:
    function unregister_event(event, addon)
      addon = addon or Inspect.Addon.Current()
      local remove_me = {}
      for i, v in ipairs(event) do
        if v[2] == addon then
          table.insert(remove_me, i)
        end
      end
      while #remove_me > 0 do
        table.remove(event, table.remove(remove_me))
      end
    end
    Of course, that works only if the table is an ipairs-friendly table. If it's not, you can do event[i] = nil, but then you are creating holes and ensuring that it will not be ipairs-friendly later. (I guess you could table.sort() it with a function which provided a meaningful order, after which I believe it would be a 1..n table again.)

    (And I can't immediately see a reason to have more than one handler, but I bet a lot of addons which use libraries will end up with more than one handler for Event.Frame.Begin.)
    Last edited by the_real_seebs; 04-23-2012 at 09:28 PM.
    You can play WoW in any MMO. You don't have to play WoW in RIFT. Oh, and no, RIFT is not a WoW clone. Not having fun any more? Learn to play, noob! I don't speak for Riftui, but I moderate stuff there. Just came back? Welcome back! Here's what's changed. (Updated for 2.5!)

+ Reply to Thread
Page 1 of 2 1 2 LastLast

Bookmarks

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts