$FG_ROOT/Nasal: fixed and customizable loading order?

[Philosopher]

Hi, all! I really think that FlightGear should have a deterministic loading order that selects a module by filename and loads it in the right order. It would be easy to do: just come up with a list, put code in globals.nas, and have FGNasalSys load the globals module which will in turn load the other modules. Everything would be done via Nasal (unless there is any objections), and we could avoid a lot of ugly _setlistener hacks.

The reason I say that globals gets loaded first is because:
• it makes sense, you want the globals namespace to always be there when loading something
• it is the only "special" namespace, because it is recursively mapped into itself and is (AFAIK) the only one that is more or less mentioned in FGNasalSys.cxx
• all of its high-level wrappers are "dynamically linked" (they look up all of their symbols when called, not when defined) and do not have any dependencies
• depending on what form of resolution support we want (i.e. do we want a "require this lib" function), it would make sense to have the symbols in the global namespace.

Presumably there would be a load_module function in globals.nas which would probably be duplicated from io.nas for sanity. There would be a loading order stores somewhere (vector or a separate file) that would name the modules in order. And when globals.nas is loaded/executed by FGNasalSys, it would execute the other modules (in addition to the $FG_HOME modules that it already does). I have not checked all of the modules yet, but this is the order that I propose:
• globals • props • string • io • * (all others) • $FG_HOME/*

Any thoughts, ideas?

[Hooray]

Right, I suggested that a long time ago but hit quite some resistance, basically you are still right: this would solve a bunch of existing problems, especially related to initialization and reinitialization of the Nasal interpreter itself:

Subject: Namespaces

Also, Nasal itself is far more powerful and flexible than most of the code you see in $FG_ROOT - it is a fairly common misconception that Nasal doesn't support "real" dependency-resolution in the form of include/require or import etc.

In fact, the Nasal developer himself has written code (in Nasal!) to implement support for import(): https://github.com/andyross/nasal/blob/ ... driver.nas
It's just that this has never been integrated in FG, and so it's not used at all.

You can find some of the most advanced Nasal code examples in non-FG related repositories, such as here: https://github.com/andyross/nasal/tree/master/lib or here: http://svn.gna.org/viewcvs/algoscore/trunk/lib/

So far, many of these library modules have not been used in FG - but that doesn't mean that it would be impossible or even complicated. There's tons of high quality Nasal code which isn't FG related.
More often than not, the really high-quality Nasal code snippets in FG are to be found in $FG_ROOT/Nasal/*.nas and were usually written at a time when Nasal was still actively maintained by former FG core developer mfranz.

For example, the Nasal sub modules feature implemented in C++ as part of the FGNasalSys class, could have been better implemented in Nasal space. And instead of having the pseudo-fixed loading order, it would be better to explicitly load Nasal modules using Andy's import() helpers, instead of using the settimer/setlistener hacks we are currently using, i.e. real dependency resolution.

The real issue is that very few people are aware of what Nasal is capable of and how it internally works - including the majority of core developers. This has recently started to change because of Tom's cppbind and James' NasalPositioned work.

Also, most people are not familiar with the standalone Nasal interpreter in Andy's github repository and the various Nasal modules/bindings and helpers available there. In addition, there's an increasing trend among "some people" to favor C++ based "solutions" instead of first checking if a native-Nasal solution could turn out to be superior, which is why we get to see certain things added in C++ space, instead of being directly done in Nasal ... More often than not, the GC issues are brought up, too. So it's kinda political, too

Bottom line being, this is unlikely to change until we have someone actively maintaining Nasal in SG/FG.

[Philosopher]

Using import() would work, we just need to make it use the globals namespace not the local one... And we could say «import("foo")» inside of foo and nothing would happen . I actually think I could come up with a working example without going into the source code («if (!contains(globals, "globals_loaded")) return;»), then I just have to lobby from there.

[Hooray]

yes, it would be even possible to provide a wrapper that handles import() via a listener, so that modules can be made available on demand, and even suspended/reloaded from disk.

Running code would merely need to register a "cleanup" callback so that relevant listeners and timers can be suspended, before the code gets unloaded from a namespace, so that the file can be reloaded from disk.

Technically it would not even be very difficult to provide a clean infrastructure for this, it would be more work to port all the existing modules, which are often messy enough already...

Also note that Zakalawe and ThorstenB already started working on fixing reset/reinit: http://wiki.flightgear.org/Reset_%26_re-init

[Philosopher]

I ended up going for a require() directive, so that the modules just clean themselves up...

For the top of globals:

Code: Select all

##
# Module loading
#
var GLOBALS_LOADED = 0; #0: in the process, 1: done, -1: not yet started, other: failed
# container for local variables, so as not to clutter the global namespace
var __ = {};
__.loaded_modules = {};

##
# Require directive, somewhat duplicated from io.load_nasal().
# If force_load is not set, then it will only load the module
# if it does not exist. Modules are defined as:
#   * All file in a directory in $FG_ROOT/Nasal/
#   * A file in $FG_ROOT/Nasal/
# If both are found then the directory is loaded instead of
# the single file
#

var require = func(name, force_load=0) {
    if (find("/", name) != -1) die("bad module name"); #this is unsafe
    if (name == "globals") return nil; #no way are we loading ourself!
    if (!force_load and __.loaded_modules[name] == 1)
        return nil; #we already loaded it...
    var err = [];
    if (!contains(globals, name)
        globals[name] = {};
    # FIXME: is there a listener associated with directories?
    if ((var files = directory(FG_ROOT~"/Nasal/"~name~"/")) == nil)
        var files = [FG_ROOT~"/Nasal/"~name~".nas"];
    foreach (var file; files) {
        var code = call(func(file) {
            if ((var st = io.stat(file)) == nil)
                die("Cannot stat file: " ~ file);
            var sz = st[7];
            var buf = bits.buf(sz);
            io.read(io.open(file), buf, sz);
        }, [FG_ROOT~"/Nasal/"~name~".nas"], err);
        if (size(err)) die(err);
        var code = call(func compile(file), nil, err);
        if (size(err)) die(err);
        call(bind(code, globals), nil, nil, globals[name], err);
        if (size(err)) die(err);
    }
    __.loaded_modules[name] = 1;
}

For the bottom:

Code: Select all

# Put files which need special treatment here:
require("io");
require("props");

# Then require() the others in as necessary
var path = FG_ROOT ~ "/Nasal";
if((var dir = directory(path)) == nil) return;
foreach(var file; sort(dir, cmp))
    if(size(file) > 4 and substr(file, -4) == ".nas")
        require(file);


And two useful, global definitions:

Code: Select all

var FG_ROOT = getprop("/sim/fg-root");
var FG_HOME = getprop("/sim/fg-home");

Untested for now, will clean it up later and hopefully test it...

[Hooray]

# FIXME: is there a listener associated with directories?

These are called Nasal submodules:
http://wiki.flightgear.org/Creating_new ... ub_modules

The signal property is /nasal/my_module/loaded, which is set to true, see: https://gitorious.org/fg/flightgear/blo ... cxx#line71

This uses the helper class at the top of the file: https://gitorious.org/fg/flightgear/blo ... cxx#line55

Once everything is up and running, nasal-dir-initialized is set to true under /sim/signals: https://gitorious.org/fg/flightgear/blo ... xx#line807

[Philosopher]

Random newspaper headline: "FlightGear's Nasal lifts itself up by its bootstraps!"

Some instructions and notes:

All files need this header at the top of the file:
if (globals["GLOBALS_LOADED"] != -1) return;
Any _setlistener("/sim/signals/nasal-dir-initialized", func{}) replaced with the contents of the func{}.
There are a few "forward-declarations" of items (mostly props.Node objects) that are assignments to nil that can be removed, since the listener is now immediate.
I did not need to modify any sub-modules any....

gui.nas and string.nas need to keep their listeners (the former may be fixable by moving it around, though).

Any error in globals.nas will be potentially deadly -- as in modules will re-load when the C++ code calls it (again). Two things:
a) Do *not* load modules from C++ anymore (^.^), except globals of course.
b) Make sure that globals returns without an error. Display a message if it did -- this could be "detrimental" to the user experience, as I've seen .

Items that I've learned the hard way (I've probably missed some, but most modules just use the listener for the props.Node objects and related helpers):
• view.nas: require("geo")
• dynamic_vew.nas: require("view.nas")
• geo.nas: require("math")

Installation:

Add this to the top of globals.nas:

Code: Select all

##
# Module loading
#

var GLOBALS_LOADED = 0; #0: in progress, -1: loading modules, -2: loading home modules, 1: done, nil: not yet started, other: failed

# container for local variables, so as not to clutter the global namespace
var __ = {};
__.loaded_modules = {};

##
# Require directive, somewhat duplicated from io.load_nasal().
# Essentially asks for a module to be loaded if it is not already.
# The force_load argument has four values:
#   * 1: re-load the module even if it has already been loaded
#   * 2: load the (sub-)module even if it is not enabled (this
#        is for directories, aka sub-modules, where the property
#        /sim/module_name/enabled controls loading of it).
#   * 3: combination of the above (aka force everything)
#   * 0: none of the above
# This function will return nil if the module is not loaded.
#
# Module are defined as:
#   * All files in a directory in $FG_ROOT/Nasal/
#   * A file in $FG_ROOT/Nasal/
# If both are found then both are loaded.
#
var require = func(name, force_load=0) {
    if (find("/", name) != -1) die("bad module name"); #this is unsafe
    if (name == "globals") return nil; #no way are we loading ourself!
    elsif (name == "" or name == "." or name == "..") return nil;
    if ((force_load == 0 or force_load == 2) and contains(__.loaded_modules, name))
        return nil; #we already loaded it...
    if (!contains(globals, name))
        globals[name] = {};
    var err = [];
    var have_directory = 1;
    var base_dir = FG_ROOT~"/Nasal/";
    var files = directory(base_dir~name);
    if (files == nil) {
        var have_directory = 0;
        var files = [base_dir~name~".nas"];
        if (io.stat(files[0]) == nil) die("cannot find module "~name);
    } else {
        if (!getprop("/sim/"~name~"/enabled")) {
            if (force_load == 2 or force_load == 3)
                printlog("warn", "warning: loading a diabled module using require()");
            else return nil;
        }
        forindex (var i; files) {
            files[i] = name~"/"~files[i]; #correct their path from base_dir
        }
        if (io.stat(base_dir~name~".nas"))
            append(files, name~".nas");
    }
    __.loaded_modules[name] = 0; #partial load
    foreach (var file; files) {
        var code = call(func {
            if ((var st = io.stat(file)) == nil) #should not happen
                print("Nasal runtime error: Cannot stat file: "~base_dir~file~", this is a bug!");
            var sz = st[7];
            var buf = bits.buf(sz);
            io.read(io.open(file), buf, sz);
            return buf;
        }, nil, err);
        if (!size(err))
            var code = call(func compile(code, file), nil, err);
        if (!size(err) and typeof(code) == 'func')
            call(bind(code, globals), nil, nil, globals[name], err);
        if (size(err)) {
            if (err != 1 and err != 0)
                __.loaded_modules[name] = err; #unsuccesful load
            print("Error on file "~file);
            debug.printerror(err);
        }
    }
    __.loaded_modules[name] = 1; #sucessful load
}

Add this below the constant definitions, or replace all my uses of them with getprops (the former is muchly preferred ):

Code: Select all

# IMO some useful global variables
var FG_ROOT = getprop("/sim/fg-root");
var FG_HOME = getprop("/sim/fg-home");
var MODEL_PATH = getprop("/sim/model/path");

And then add this below the printlog definition, replacing the settimer:

Code: Select all

var GLOBALS_LOADED = -1;

##
# Put modules here that require special attention; ones
# that are used often enough not to warrant a require()
# at the top of every file or contain security code
# (like io.nas). Other modules that are needed by a file
# should be require'd at the top of that file.
#
require("string");
require("debug");
require("props");
require("io"); #uses all 3 of the above modules

##
# Then we require() others in as necessary.
#
__.dir = directory(FG_ROOT~"/Nasal/");
if (__.dir == nil) print("globals.nas: No Nasal directory!");
else {
    foreach (__.file; sort(__.dir, cmp)) {
        if (substr(__.file, -4) == ".nas") {
            #print("loading module "~substr(__.file, 0, size(__.file)-4));
            require(substr(__.file, 0, size(__.file)-4));
        } elsif (directory(FG_ROOT~"/Nasal/"~__.file) != nil and __.file != "globals" and __.file != "io"
                 and __.file != "props" and __.file != "." and __.file != "..") { #very important that we skip . and ..!
            #print("loading sub-module "~__.file);
            require(__.file);
        }
    }
}

var GLOBALS_LOADED = -2;

##
# Load and execute ~/.fgfs/Nasal/*.nas files in alphabetic order
# after all $FG_ROOT/Nasal/*.nas files were loaded. (We set
# GLOBALS_LOADED before this since it essentially is loaded
# and its loading status does not depend upon these modules).
#
__.path = FG_HOME ~ "/Nasal";
if((__.dir = directory(__.path)) == nil) return;
foreach(__.file; sort(__.dir, cmp))
    if(size(__.file) > 4 and substr(__.file, -4) == ".nas")
        io.load_nasal(__.path ~ "/" ~ __.file, substr(__.file, 0, size(__.file) - 4));

# Keep this at the very end of the file!
var GLOBALS_LOADED = 1;

I personally haven't seen any regressions, but I only use a very small feature set of FG so I might have introduced some, but on the whole I believe it to be cleaner and easier, not to mention much less hack-ish (well, besides the fact that I am working around C++ code).

[Hooray]

I only looked very briefly at your code and haven't yet tested it, but this could turn out to be pretty useful. Explicit dependency resolution is a long-standing feature request:

http://www.mail-archive.com/flightgear- ... 03384.html
http://www.mail-archive.com/flightgear- ... 03387.html
http://www.mail-archive.com/flightgear- ... 03390.html
http://www.mail-archive.com/flightgear- ... 26385.html

I would however suggest to simply overload/replace built-in functions instead of "manually patching" them, i.e. the _setlistener() stuff is obviously problematic for all modules that use _setlistener() without storing the handle to later on call removelistener() accordingly, which is a long-standing unfortunate practice, but which we need to address at some point.

Eventually, we want to be able to easily reload Nasal modules from disk, at runtime. Thus, we would need a basic API/framework and use wrappers for all settimer()/setlistener()/threading calls, so that the corresponding signals can be set, for modules to call their own cleanup/reset/reinit code.

And finally, please don't be discouraged due to the lack of feedback: ThorstenB and Zakalawe already outlined their plans on providing better reset/reinit support, see: http://wiki.flightgear.org/Reset_%26_re-init

Obviously, that effort will make it much easier to switch aircraft at runtime. Thus it will be important for Nasal modules to be also reloadable at runtime, and aircraft-specific scripts should be directly addressable, so that their loops, callbacks and listeners/timers can be easily freed after setting a "cleanup" signal to invoke a corresponding callback, so that a new aircraft can be loaded.

It should be possible to inspect such scripts using some Nasal magic.

From a purely stylistic perspective, I hope that some of your proof-of-concept hacks could be implemented differently, i.e. without the variables at the top, and without huge conditional statements. Specifically referring to stuff like (directory(FG_ROOT~"/Nasal/"~__.file) != nil and __.file != "globals" and __.file != "io" and __.file != "props" and __.file != "." and __.file != "..") - I'd suggest to simply use a vector with elements to check for, and then introduce a one-liner helper function ?

Also, as another workaround, you could change the load_nasal() routine to procedurally add the if (globals["GLOBALS_LOADED"] != -1) return; to the top of each require'd file for now, until we have found a better solution

[Philosopher]

I think you misunderstood me about the _setlistener: all that I am doing is taking the code inside it and making it be outside of it; making it immediate instead of delayed for dependency resolution. So:

Code: Select all

_setlistener("/sim/signals/nasal-dir-initialized", func {
   ...
}

Becomes:

Code: Select all

#_setlistener("/sim/signals/nasal-dir-initialized", func {
   ...
#}

(aka just the ...)

One regression I found is that sub-modules are now loaded with regular modules, but I don't see it mattering anymore at all, period, end of discussion (just use require() if necessary).

I need to work on more fancy features, but it works for now. One thing about requiring is that it allows interruption of loading versus simply "loading this file", "loading that file"; you can weave in and out of different files (eventually, doesn't work on fancy cases right now) and generally be a lot more cleaner about it. Like the issue James raised: what if I need a function from math.nas right now?

EDIT: regarding what I think is your proposal to clean up all listeners and timers, wouldn't that be better done using C++? Well first of all, it sounds like we are going to chuck the whole property tree out the door, so that would clear up listeners. And timers are not really accessible from Nasal. It would be much easier to just have the Nasal subsystem delete all of them, right?