Hooray wrote in Sat Jan 23, 2016 10:03 am:@bugman:
Hi! This is quite a post, I'll see if I can respond. I suggest that for better input into the developments, to have shorter messages that focus on a single point.
Wow, that's quite an accomplishment for someone just recently claiming not to be a C++ programmer
Well, once you know a few languages, learning to speak another is much easier. I have written a lot of code, mainly the software relax consisting of a trunk SLOC count of 141,331 lines of code (with an enforced ~30% comment ratio).
I haven't actually built your branches, but went through your commits/patches and read your comments on the devel list.
You should give it a go. This would enable you to make more valuable contributions
So take my comments with a grain of salt (of course...), I am making them specifically with a focus on 1) the way Nasal scripting currently works in FlightGear (and the challenges resulting from that), and 2) your comments to hopefully provide Python support as a viable alternative for "software prototyping" and adopting HLA via scripting.
I have plenty experience, so I have built up a large reserve of salt!
That being said, here's some feedback:
Despite the salt, all feedback is good. Cheers! Note that there have been a number of private and the official developer Google Hangout discussions about the topics you cover below.
- to bring the Python interface up to par with Nasal, timers and listeners would be useful - because those are currently the main building blocks to trigger scripted code in the FlightGear main loop.
Let me get back to that one. Note - I think timers are a hack.
Over time, this has become a bit problematic in FlightGear - and the core developers have come to the conclusion that RAII is needed for timer/listener management - which is to say that you would want to look at the OOP maketimer() API in FGNasalSys and explore having two similar APIs for wrapping timer/listener functionality in a makelistener() fashion.
The more I look into the Nasal timer code, the more I develop a deep down sick feeling. Note that the FGPythonSys subsystem is not bound by this design, and I do not intend it to be.
One of the issues causing Nasal code to be a real challenge compared to its C++ counterpart is that none of the Nasal code is using the existing SGSubsystem interface - so if your comments on "software prototyping" were/are serious, we would probably want to look at exposing SGSubsystem as a base class which can be inherited from in Python space to register new subsystems that are implemented in Python.
Those comments are serious. The idea of a base class for spawning SGSubsystems and talking to them via HLA has already been discussed "behind the scenes" (not really behind the scenes, but just not visible from this forum).
Like you mentioned on the devel list, many things like the property tree/subsystems would ideally be using some form of IPC (like HLA) to interface with the rest of the simulation - but the logical first step is exposing conventional bindings for the property tree and subsystems.
From my coding experience I can say that this is not how it's done. It's more like this - serve up a concrete problem to solve, with code, data, scripts, etc., and that problem will be solved.
The way your code is currently structured, it seems that you pretty much used FGNasalSys as a template, using copy & paste. As you probably noticed already, there is quite a bit of overlapping code/functionality, so that it /might/ make sense to introduce a common base class in the form of FGScriptingHost that inherit from SGSubsystem, from which FGPythonSys/FGNasalSys could then inherit their own child-classes, so that shared functionality can reside in the parent class (or customized/overridden accordingly).
There was a lot of copy and paste. That's not hidden. But I would like to diverge significantly from where Nasal has gone. Therefore I do not intend on sharing any of this Nasal infrastructure. Remember - this is in no way designed to be a Nasal replacement. And this clean slate development is in no way bound by the Nasal design decisions.
Otherwise, I do applaud your effort to make the whole thing compatible with the ongoing reset/re-init work, because that could be cosidered a key part of the whole HLA thing, and most core developers continue to make commits that are basically incompatible with reset/re-init, i.e. complicating matters for Zakalawe tremendously (on the other hand, his reset/re-init and headless work admittedly didn't even make it to the "updated roadmap" either).
Cheers! After working with this, I can see that this work is still in flux. Modularlisation, isolation, and standardisation of the FG codebase is very much WIP. This is being pushed hard from 2 directions - James with the reinit() and Stuart with HLA. Note my CppUnit comment on the mailing list - if this takes off, then this process will be massively accelerated.
Another issue that FGNasalSys is suffering from is that it is designed to be a singleton - and it seems FGPythonSys is sharing this limitation. You may want to reconsider if that is ultimately what you are aiming for - simply because in FG, the whole thing of a scripting session depends largely on the "context" - and here FGNasalSys has become a rather messy part of FlightGear, because we don't even differentiate between different "types" of scripts (think aircraft vs. scenery vs. GUI vs. core etc).
I do think that this could also help address the security issues that Rebecca briefly touched on the devel list, and which I mentioned a few weeks ago on the forum, too
Had FGNasalSys been designed to support independent instances, we could have multiple Nasal interpreters running independently for different script types, and could also grant them different permissions ($FG_HOME, $FG_AIRCRAFT, $FG_SCENERY)
That is something that would greatly help simplify state management for reset/re-init, including use-cases where people may want to save/resume flights (in fact arbitrary simulator state).
For the time being, this is not possible and causing tons of Nasal related challenges for reset/re-init.
Equally, there is the very long standing issue of the Nasal interpreter not being available earlier, despite it being potentially useful.
As you can see on the wiki ("Initializing Nasal earlier"), James, Melchior and others were once contemplating to change that so that FlightGear scripting would become available sooner.
The main thing that prevents this currently for the Nasal VM is the fact that it has a plethora of SGSubsystem-specific "bindings" (basically subsystem-specific APIs) that are dumped into the globals context, and subsequently assumed to be available "automatically".
This is a fragile/broken assumption in an increasingly multi-threaded environment, especially because of the ongoing reset/re-init work, where subsystems may be shut down/restarted "on demand", i.e. their availability may change dynamically.
Like I mentioned previously in this thread, we can easily work around this issue by using the ::bind() and ::unbind() methods to register/remove certain API symbols that depend on other subsystems.
With Nasal it is kinda "too late" unfortunately, because of the plethora of APIs and subsystems currently exposed - for FGPythonSys, this may be much more straightforward, because it's still in its infancy.
Well, you can only have one instance of the Python interpreter running, and it is bound by its global interpreter lock (GIL). Note though that there is sub-interpreter support. The current FGPythonSys subsystem can be initialised at any point and I intend to keep it that way. If I can set up the CppUnit tests then initialising just this subsystem, with absolutely nothing else, will be locked into stone!
In layman's terms that basically means that an API like maketimer() will depends on the events subsystem (in the case of Nasal), whereas maketimer() would depend on the property tree being around, and navDB related APIs would obviously depend on the navDB.
By the way, when it comes to main loop timers, you may want to consider having your own "python-events" subsystem to ensure that Nasal and Python related timers show up in different subsystems (i.e. in the performance monitor stats).
I do not intend on using timers or maketimer(). I find such a design to be very much non-ideal. There are much better ways of solving the same problem.
You may also want to provide a startup option to entirely disable Python to help ensure that people can troubleshoot issues and determine if they are affected by Python being active or not - i.e. Python being built-in but adding the subsystem is determined conditionally using a fgGetBool() and or logic in $FG_SRC/Main/options.cxx
I am just suggesting that because we have seen countless of posting where Nasal (and its GC) were said to be the culprit, and only when we walked people through removing all Nasal code from the aircraft (think Su15), they finally realized that Nasal was not the issue at all.
Any ideas how?
That is also one of the reasons why I believe that the whole "Singleton" assumption will become problematic over time, because if something like FGPythonSys is not a singleton, we can also much more easily stress-test it - while also being able to deal with scripts differently, i.e. depending on their "context" (scope) being aircraft/scenery or GUI related.
The signleton is already built into the embedded Python interpreter, with the exception of sub-interpreters. It should be possible to work around though, as countless other programs with embedded Python have.
Equally, a FGPythonSys interpreter that is up and running very early could become really helpeful for debugging/troubleshooting purposes, but also help us implement benchmarking support.
True, but an interface to the relevant C++ parts will be needed.
Overall, my suggestion would really be to closely look at where Nasal "failed", and for which reasons - and try to learn from these lessons.
Timers sounds like the biggest of those failures. Do you have any others you can list?
Incremental re-initialization and saving/loading state really is a crucial part of the simulation, and it would go a long way to come up with a new scripting option that does not suffer from certain restrictions, especially in an increasingly multi-threaded and distributed environment like HLA.
Finally, you did an outstanding job there with all the unit testing you implemented, which is another thing where Nasal failed.
Cheers! Now I just need to hardcode this into a proper test suite.
Again, your mileage may vary - so if in doubt, raise any of this with some of the folks on the devel list who are willing to help review/commit your patches - my priorities are obviously biased due to my own experiences with FGNasalSys
We'll see. This is currently just a big experiment. Who knows what will come next
Regards,
Edward