TomTom Integration Incomplete
DFortun81 opened this issue ยท 10 comments
Hello! I'm the author of the AllTheThings addon.
Its come to my attention that your addon overrides the global variable for TomTom, but it doesn't do it completely. TomTom has the ability to add callbacks to their waypoints via the DefaultCallbacks function. This function creates a table that you can assign minimap and world map tooltip update and show calls to when assigning to a waypoint's callbacks field.
Your replacement of the TomTom global makes this function inaccessible. Additionally, it doesn't take these callbacks into consideration when rendering the waypoints. (You can use AllTheThings' Alt+Right Click to plot something in our addon and see how it's supposed to work and then do it again with your addon enabled. Prior to my fix in 3.5.6, this would crash ATT's plotting because the function TomTom.DefaultCallbacks was inaccessible to ATT.)
As a standard practice, if you are going to override a table value such as TomTom, you should at least pass through from your table override to the table being overridden. This will make it so that when an addon like ATT tries to use a function provided by TomTom that your addon doesn't also inject, it'll fall through to TomTom's functionality.
You can do this really easily like so:
local tom = setmetatable({}, { __index = _G.TomTom }); _G.TomTom = tom; tom.AddWaypoint = Nx.TTAddWaypoint; -- .... etc
While I don't necessarily agree with ever completely overriding another addon's global variable, this is how you'd do it without hiding functionality you didn't intend to completely override.
Its not a 100% Emulation because its not needed if someone wants to have 100% then just use TomTom. Its intended just to emulate only those functions that Carbonite needs to check routes and repopulate them on Carbonite Map.
I specifically added the Nx.RealTom
variable to check if its the real TomTom or not. Ill just add a PR to your repo, its not that hard to check Nx and NX.RealTom variable :)
Of course u are also welcome to write PR to my repo to add those things u want :)
The most important thing is to be able to discuss ๐ without hate.
As to PR let me know ๐ I can make this simple PR to your repo if u like ๐
@DFortun81 Thanks for the feedback.
But that's not quite what I'm doing... I'm Emulating TomTom when its not installed and then and only then I'm using _G.TomTom
variable plus emulation of its functions that are needed.
There is variable in Carbonite Nx.RealTom
globally accessible. If its false
then I'm emulating TomTom and u can make an exemption in your code not to use TomTom functions in this case.
Oh, I guess I wasn't given all the details in the bug report that prompted me to look into this then. In any case, if your implementation could at the very least introduce the DefaultCallbacks method / functionality to your version of the TomTom emulation, that'd be great.