Discussion Orbiter: Lua 5.4 Upgrade: Community Input

[Kozak]

Hello Orbiter community,

We're at a crossroads regarding Orbiter's Lua language support implementation, and would like to get community input on the best way to proceed.

There is an ongoing push to modernise Orbiter codebase, namely:

• To remove support for obsolete platforms (x86, DX7)
• To achieve building Orbiter under Linux
• To support & improve the "plug & play" nature of Orbiter development - you just need Visual Studio and a codebase checkout to compile

Keeping in mind above 3 priorities, we've arrived at a decision point. Orbiter uses Lua 5.1, and due to various reasons listed below it would be beneficial to consider upgrade to 5.4

Why Consider This Change?​

The primary benefits of upgrading would be infrastructure-related:
1. Linux Compilation Support - This upgrade would remove one of the key blockers for Linux builds
2. Modern Language Support - 5.1 is not supported, upgrade needs to happen at some point
3. Simplified Dependencies - We could leverage vcpkg for more streamlined dependency management

The Challenge​

The main downside this upgrade brings is that - due to lack of backward compatibility between Lua versions - a lot of existing Lua-based add-ons & Orbiter code would break. Any addon using Lua may end up on incompatible list will need some changes to accommodate the upgrade. There is also some community effort which would be required to update Lua scripts used within Orbiter itself

Other factors to consider​

There has been an attempt to use luajit in Orbiter, which also supports 5.1 version of the language. However, using Luajit will lock us out of using vcpkg (due to luafilesystem package requiring Lua 5.3 there), as well as language upgrade in future

What is needed?​

Please share your thoughts on how we should proceed:

• Proceed with the 5.4 upgrade?
• Switch to Luajit, keeping 5.1 language but removing/adapting luafilesystem?
• Keep Lua 5.1 & invest in hacks/workarounds?
• Add more information?

If you're an add-on developer, we'd especially appreciate your perspective on:

• How this would impact your development work
• Would you be willing to update your add-ons
• Any specific concerns about the migration process

This is an exploratory discussion at this stage - no decisions have been made, and your
input will help shape the path forward

 

[Urwumpe]

I also created an issue / feature request in github for this before, glad to see it go ahead.

Upgrade Lua to version 5.4 and make constants really read-only · Issue #391 · orbitersim/orbiter

User Story As script developer, I want to be sure that no other script can overwrite the variables which refer to OrbiterSDK constants and enums, so that its impossible to cause bugs in a different...

github.com

While I am an old C++ guy and less interested in full Lua add-ons, I like the creative option of providing my own Lua interface to scenarios.

 

[Gondos]

Nice idea to bring that up.
My two cents :
The luafilesystem package is only used at compile time by ldoc to generate the documentation, it's not deployed afterwards.
If we can replace ldoc, we can get rid of luafilesystem and Penlight.
For addons that may be doing math heavy stuff in Lua, I think using LuaJIT would provide far better performance compared to Lua 5.4.
Regarding the mentionned issue, do keep in mind that the Lua integration in Orbiter is done by instanciating one lua_State/one thread per Lua module so there is no risk that one module can mess up another module by accident.

 

[Urwumpe]

How is the error handling in luaJIT? similar to the interpreted version? Does the JIT compiler make any "magic" optimizations to the run-time sequence of instructions?

I liked the 5.4 version so far for adding a few features to prevent annoying errors.

 

[Gondos]

How is the error handling in luaJIT? similar to the interpreted version? Does the JIT compiler make any "magic" optimizations to the run-time sequence of instructions?

I liked the 5.4 version so far for adding a few features to prevent annoying errors.

What do you have in mind? I'm not familiar with 5.1+ versions...
It's not related to a particular version or Lua but you can add a "StrictMode = true" option in the vessel config file to have the interpreter generate an error when trying to access an undefined global from a function instead of adding it to the global scope. It's no silver bullet but it can help with typos...

 

[n72.75]

I'm not a heavy user of Lua, so I may not be the best to weight in.

However, part of the motivation in releasing Orbiter 2024, was to create one last, stable amalgamation of all the x86 fixes. Now that we've done that, I think we should move ahead with modernization and features.

 

[Thunder Chicken]

As it is, quite a lot of Lua capability has only recently been built out in Orbiter so backward compatability for Lua add-ons is a rather messy thing right now anyway. I have some scripts that probably wouldn't have been possible even months ago. I think if upgrades and modernization needs to happen, sooner would be much better than later, especially if that allows us to enjoy potentially years of codebase stability alongside a stable Orbiter 2024.

One of the nice features of scripting is that the script must go with the add-on, so if methods need to be updated, the means exists to do so. That's going to be an ongoing process, and the sooner the Lua development environment in Orbiter stabilizes, the saner that process can be.

 

[Thunder Chicken]

It's not related to a particular version or Lua but you can add a "StrictMode = true" option in the vessel config file to have the interpreter generate an error when trying to access an undefined global from a function instead of adding it to the global scope. It's no silver bullet but it can help with typos...

Oh man, I've been looking for something like this forever. This needs to get in the Lua docs.

EDIT: Though, it seems only to scream about assignment to undeclared variables, but the typing in Lua is set by the value it is assigned? Outside of defining certain local variables, how can this be done in Lua? Initialize everything to nil maybe?

 

[Lucy]

If you want to try out the 5.4 implementation, our WIP branch with it in is https://github.com/orbitersim/orbiter/tree/vcpkg-lua-54 - assuming you can build Orbiter, naturally, but that process should be a little more straightforward here also.

I had some pain with the shipped lua scripts as the behaviour of some methods has changed, and some have been replaced entirely, but I think all those have been resolved now.

The main breaking point was that the behaviour of coroutine.running() has changed between 5.1 and 5.4 - it's now effectively the inverse of what it was. In 5.1 this method returns nil if called by the main thread, in 5.4 it returns true in this case. If called by a coroutine the return signature in 5.4 adds a boolean to the response that wasn't returned in 5.1 but you do still get the coroutine back.

 

[jedidia]

I'm not familiar with how things are put together... might it be possible to move the LUA support to a module, graphics client style, so users could still run with the old LUA module if they want to play an older add-on that wasn't updated?
Of course, there'd have to be signifficant enough add-ons for which this would be an anticipated problem, or the effort would be hardly worth it.

 

[Fabri91]

I'm not a heavy user of Lua, so I may not be the best to weight in.

However, part of the motivation in releasing Orbiter 2024, was to create one last, stable amalgamation of all the x86 fixes. Now that we've done that, I think we should move ahead with modernization and features.

In general I agree with the sentiment, especially if like in this case the change helps in making building (and thus support) easier/more robust.

But, I'm not an addon developer, so I don't really have something to "lose" either way.

 

[Gondos]

Food for your thoughts : I did a quick lambert solver MFD in Lua to have something to bench. For 65536 sampling points I get this :
-Lua 5.1 debug : 5.2s
-Lua 5.1 release : 2.4s
-LuaJIT debug : 1.1s
-LuaJIT release : 0.5s
Not sure the solver gives reliable results but it should not impact the comparison...
I'll try it on the 5.4 branch tomorrow to see how it fairs

 

[Gondos]

So far no luck with the 5.4 version, the vcpkg stuff has me stymied
I tried the procedure from here but then I get this when I try to generate the cmake stuff :

1> Command line: "C:\Windows\system32\cmd.exe" /c "%SYSTEMROOT%\System32\chcp.com 65001 >NUL && "C:\PROGRAM FILES (X86)\MICROSOFT VISUAL STUDIO\2019\COMMUNITY\COMMON7\IDE\COMMONEXTENSIONS\MICROSOFT\CMAKE\CMake\bin\cmake.exe"  -G "Ninja"  -DCMAKE_TOOLCHAIN_FILE:STRING="/scripts/buildsystems/vcpkg.cmake" -DCMAKE_C_COMPILER:STRING="cl" -DCMAKE_CXX_COMPILER:STRING="cl" -DORBITER_MAKE_DOC:STRING="OFF" -DCMAKE_BUILD_TYPE:STRING="Release"  -DCMAKE_MAKE_PROGRAM="C:\PROGRAM FILES (X86)\MICROSOFT VISUAL STUDIO\2019\COMMUNITY\COMMON7\IDE\COMMONEXTENSIONS\MICROSOFT\CMAKE\Ninja\ninja.exe" "C:\github\openorbiter" 2>&1"
1> Working directory: C:/github/openorbiter/out/build/windows-x86-release
1> [CMake] CMake Error at C:/Program Files (x86)/Microsoft Visual Studio/2019/Community/Common7/IDE/CommonExtensions/Microsoft/CMake/CMake/share/cmake-3.20/Modules/CMakeDetermineSystem.cmake:129 (message):
1> [CMake]   Could not find toolchain file: /scripts/buildsystems/vcpkg.cmake
1> [CMake] Call Stack (most recent call first):
1> [CMake]   CMakeLists.txt:7 (project)
1> [CMake] -- Configuring incomplete, errors occurred!
1> 'C:\Windows\system32\cmd.exe' '/c "%SYSTEMROOT%\System32\chcp.com 65001 >NUL && "C:\PROGRAM FILES (X86)\MICROSOFT VISUAL STUDIO\2019\COMMUNITY\COMMON7\IDE\COMMONEXTENSIONS\MICROSOFT\CMAKE\CMake\bin\cmake.exe"  -G "Ninja"  -DCMAKE_TOOLCHAIN_FILE:STRING="/scripts/buildsystems/vcpkg.cmake

Isn't it supposed to make things easier? Massively underwhelmed so far

 

[Lucy]

This error suggests a VCPKG environment variable is not set - it's supposed to be the prefix in that path. I know it seems trite but a reboot may resolve that to refresh the environment after the vcpkg installation.

Edit: It looks like a manual configuration might be required, from that guide. I seem to have forgotten how I set it up on my windows install, will check later.

 

[Kozak]

VCPKG component needs to be explicitly installed in Visual Studio. You can install it manually using Visual Studio installer or just import the vsconfig file into it - and it will install all missing components

P.S. It also works from command line: VisualStudioSetup.exe --config P:\orbiter\.vsconfig

 

[jarmonik]

There is a lot of LUA related work done for Orbiter 2024, does that code remain valid after update to 5.4 or does it go to garbage can ?

Also, I don't exactly like that when I got time to do Orbiter development I notice it wont compile. Since I am not a build expert it's a pain to deal with a problems like that. It's easier to forget it and do something else. So, could we do a test run in a separate branch for a build modifications and when approved merge to main. And I would like to receive a notification to e-mail when something needs attention. Also build related documentation should be kept updated.

 

[n72.75]

Also, I don't exactly like that when I got time to do Orbiter development I notice it wont compile. Since I am not a build expert it's a pain to deal with a problems like that. It's easier to forget it and do something else. So, could we do a test run in a separate branch for a build modifications and when approved merge to main. And I would like to receive a notification to e-mail when something needs attention. Also build related documentation should be kept updated.

100% agree.

I can't remember if we have github permissions to do this, but if we do, we should require reviews before merging.

At least one reviewer should checkout and build the changes themselves, before approving and before the code gets merged into main.

Every merge commit to main, should be "release" ready. (non-breaking, with updated documentation. Long-running projects should have regular merges from main to prevent merge conflicts.

 

[Kozak]

What were the build problems you experienced? Every commit in main is built using CI - so if the code didn't compile for you, CI should be improved it so it better reflects local environment. Happy to take a look

There is a lot of LUA related work done for Orbiter 2024, does that code remain valid after update to 5.4 or does it go to garbage can ?

Yes, that is the question posed in the original post. Should we "go fast and break things" in that regard, or is there feeling that too much work has been invested and we should just keep living with 5.1 (optionally luajit)

 

[Urwumpe]

There is a lot of LUA related work done for Orbiter 2024, does that code remain valid after update to 5.4 or does it go to garbage can ?

Also, I don't exactly like that when I got time to do Orbiter development I notice it wont compile. Since I am not a build expert it's a pain to deal with a problems like that. It's easier to forget it and do something else. So, could we do a test run in a separate branch for a build modifications and when approved merge to main. And I would like to receive a notification to e-mail when something needs attention. Also build related documentation should be kept updated.

I agree - I can fix many issues myself, but it isn't really a motivation, if you spend your free evening fixing a build instead of developing.

 

[jarmonik]

Recent build problems were resolved and there is a PR for it to fix it. Looks like nobody tested building Orbiter without XRSound and Documentation. Building the docs requires so-many additional tools that build must work without it.

As for the Reviews, if I want somebody to review my code there's nobody I can ask through the GIT. The current list of valid reviewers are probably not going to review anything. Matthew, Gondos are not on the list. Also, I don't recall ever receiving a request to review anything.

Lua 5.1 is about 20 years old. So, an update is something what should be considered. But then again, DX9 is more than 20 years old and upgrade has been on mind many times but the truth is that DX12 or Vulkan only offers 2 or 3 features that would be useful to us and none of them been important enough to go through it, more important things has taken the priority.

I guess that LUA 5.4 could be developed in a separate branch and when the functionality matches the functionality of 5.1 it could be merged.