From 02b01af19e38d8f73326857ee5bb261452b66590 Mon Sep 17 00:00:00 2001
From: Florent Rougon <f.rougon@free.fr>
Date: Mon, 10 Dec 2018 15:24:03 +0100
Subject: [PATCH] Add-ons: small fixes to Nasal/addons.nas and
 Docs/README.add-ons

+ After discussion with Henning:
    - orig_setlistener() and orig_maketimer() were not intended to be
      public -> prefix the function names with an underscore;
    - clear the add-on's lists of tracked listeners and timers in
      remove() (load() does that too, but it's a bit late).

+ Little rewording in Docs/README.add-ons.
---
 Docs/README.add-ons | 31 +++++++++++++++++--------------
 Nasal/addons.nas    | 20 +++++++++++---------
 2 files changed, 28 insertions(+), 23 deletions(-)

diff --git a/Docs/README.add-ons b/Docs/README.add-ons
index 1596cf0f6..5c0028292 100644
--- a/Docs/README.add-ons
+++ b/Docs/README.add-ons
@@ -840,13 +840,15 @@ Read-only data members (attributes) of addons.Maintainer objects:
 !!! We really don't want users to send bug reports due to reload going wrong.
 
 To make development of add-ons less time consuming, you can reload the
-Nasal part of your add-on without having to restart FlightGear. The
-addons.nas module will track setlistener() and maketimer() calls for
-your add-on and remove listeners and stop timers on reload for you (the
-add-on's own namespace, has setlistener() and maketimer() wrappers that
-shadow and call themselves the standard setlistener() and maketimer()
-functions).
-
+Nasal part of your add-on without having to restart FlightGear. When an
+add-on is loaded, setlistener() and maketimer() wrappers are installed
+in the add-on's own namespace; these wrappers shadow and call themselves
+the standard setlistener() and maketimer() functions. The setlistener()
+and maketimer() wrapper functions keep track of every listener and timer
+they create. When the add-on is removed (e.g., as part of its reload
+sequence), removelistener() is called for each of these listeners, and
+each timer has its stop() method called.
+_
 For the time being, you have to track any other resources outside the
 namespace of your add-on by yourself and clean them up in the unload()
 function, e.g. delete canvas or close any files you opened.
@@ -855,14 +857,15 @@ You can define this unload() function in the addon-main.nas file. When
 your add-on is reloaded, its unload() function, if defined, will be
 called with one argument: the addons.Addon object (a Nasal ghost)
 corresponding to your add-on. unload() is run in the add-on's own
-namespace, therefore it sees the aforementioned setlistener() and
-maketimer() wrapper functions.
+namespace.
 
-The reload is triggered by setting /addon/by-id/yourAddonIDhere/reload
-to true. A listener will react to that property, reset it to false and
-attempt to reload the Nasal file (doing the cleanup before). You can add
-a menu item to trigger the reload easily, but this should be removed
-before publishing your add-on to users.
+The reload is triggered by setting the property
+/addon/by-id/ADDON_ID/reload to true (replace ADDON_ID with your
+particular add-on identifier). A listener will react to that property,
+reset it to false and attempt to reload the addon-main.nas file for your
+add-on (doing the aforementioned cleanup before). You can add a menu
+item to trigger the reload easily, but it should be removed before
+publishing your add-on to users (see the above warning).
 
 Please have a look at the skeleton add-on at
 https://sourceforge.net/p/flightgear/fgaddon/HEAD/tree/trunk/Addons/Skeleton/
diff --git a/Nasal/addons.nas b/Nasal/addons.nas
index ac6b074f5..dc9cf91c0 100644
--- a/Nasal/addons.nas
+++ b/Nasal/addons.nas
@@ -32,8 +32,8 @@
 # hashes to store listeners and timers per addon ID
 var _listeners = {};
 var _timers = {};
-var orig_setlistener = nil;
-var orig_maketimer = nil;
+var _orig_setlistener = nil;
+var _orig_maketimer = nil;
 
 var getNamespaceName = func(a) {
     return "__addon[" ~ a.id ~ "]__";
@@ -61,16 +61,16 @@ var load = func(a) {
                 p = p.getAliasTarget();
             }
         }
-       append(_listeners[a.id], orig_setlistener(p, f, start, runtime));
+       append(_listeners[a.id], _orig_setlistener(p, f, start, runtime));
        print("#listeners for " ~ a.id ~ " " ~ size(_listeners[a.id]));
     }
 
     # redirect maketimer for addon
     globals[namespace].maketimer = func() {
         if (size(arg) == 2) {
-            append(_timers[a.id], orig_maketimer(arg[0], arg[1]));
+            append(_timers[a.id], _orig_maketimer(arg[0], arg[1]));
         } elsif (size(arg) == 3) {
-            append(_timers[a.id], orig_maketimer(arg[0], arg[1], arg[2]));
+            append(_timers[a.id], _orig_maketimer(arg[0], arg[1], arg[2]));
         } else {
             print("Invalid number of arguments to maketimer()");
             return;
@@ -107,17 +107,19 @@ var remove = func(a) {
     print("- Removing ", a.id);
     var namespace = getNamespaceName(a);
     foreach (var id; _listeners[a.id]) {
-        print("  Remove listener " ~ id);
+        print("  Removing listener " ~ id);
         removelistener(id);
     }
+    _listeners[a.id] = [];
 
     print("  Stopping timers ");
     foreach (var t; _timers[a.id]) {
         if (typeof(t.stop) == "func") {
             t.stop();
-            print(".");
+            print("  .");
         }
     }
+    _timers[a.id] = [];
 
     # call clean up method if available
     # addon shall release resources not handled by addon framework
@@ -154,7 +156,7 @@ var init = func {
 
 var id = _setlistener("/sim/signals/fdm-initialized", func {
     removelistener(id);
-    orig_setlistener = setlistener;
-    orig_maketimer = maketimer;
+    _orig_setlistener = setlistener;
+    _orig_maketimer = maketimer;
     addons.init();
 })