New Lua API. #5190

pchote · 2014-04-25T07:38:48Z

This introduces a new scripting API built on Eluant and the native lua library. The main focus of this implementation was building a self-consistent and self-documenting API (generated using the --lua-docs utility command, and available on the wiki). Secondary benefits are that it is faster than Kopilua and doesn't leak memory everywhere.

This is going to take a while to properly review, so i'm filing this now. There ~~are two~~ is one main TODOs left to finish:

~~Packaging changes: we can use the system lua for linux platforms, but will need to ship lua on Windows and OSX. We should also credit the new thirdparty bits in AUTHORS.~~
~~Eluant.dll includes a bunch of now-redundant changes. I need to create a public fork, revert the changes we don't need, and file upstream prs for the ones that we do.~~

This implements enough of the API for the shellmaps, but no more. A future PR will introduce a ScriptPlayerProperties implementation that exposes mission objectives and victory conditions.

DanAE111 · 2014-04-25T08:02:14Z

That's a lot of code, i think you deserve a beer!

Mailaender · 2014-04-25T08:28:06Z

You will need to change the package-all.sh, Makefile install section and the NSIS file for the Windows installer.

ghost · 2014-04-25T11:05:22Z

🍻

pavlos256 · 2014-04-25T20:19:01Z

OpenRA.Game/OpenRA.Game.csproj

@@ -74,6 +74,9 @@
      <Package>mono.nat</Package>
      <HintPath>..\thirdparty\Mono.Nat.dll</HintPath>
    </Reference>
+    <Reference Include="Eluant">
+      <HintPath>..\Eluant.dll</HintPath>


This should be "..\thirdparty\Eluant.dll"

pchote · 2014-04-26T01:43:32Z

Thanks for the detailed review @pavlos256! I'll start working through these over the next couple of days.

pchote · 2014-04-26T02:09:41Z

OpenRA.Mods.RA/Scripting/Global/MapGlobal.cs

+		}
+
+		[Desc("Calls a function after a specified delay.")]
+		public void RunAfterDelay(int delay, LuaFunction function)


This should move to Trigger.AfterDelay(int ticks, LuaFunction func).

pchote · 2014-04-26T03:42:01Z

Figured it was easier to get this out of the way now. Fixed nits, and sorted out the Eluant changes. See https://github.com/pchote/Eluant/ for the custom version included in this pr.

ScottNZ · 2014-04-26T04:06:42Z

OpenRA.Game/Actor.cs

@@ -240,5 +243,40 @@ public void Kill(Actor attacker)

 			health.Value.InflictDamage(this, attacker, health.Value.MaxHP, null, true);
 		}
+
+		#region Scripting interface


Maybe make this and the other Lua-augmented classes partial, with all the Lua-specific bits in their own file?

pchote · 2014-04-26T04:52:07Z

Updated with default parameter support, addressing @obrakmann's concern.

Mailaender · 2014-04-26T07:30:17Z

OpenRA.Game/WVec.cs


 namespace OpenRA
 {
-	/// <summary>
-	/// 3d World vector for describing offsets and distances - 1024 units = 1 cell.


Why remove the XmlDoc here? It integrates into IDE tooltips and is kinda helpful.

Consistency. They aren't used anywhere else in the codebase.

Okay, we should document this somehow though #4372.

pavlos256 · 2014-04-26T20:32:47Z

I think Eluant.dll should be added to the Makefile's COMMON_LIBS

pchote · 2014-04-26T23:09:32Z

I think Eluant.dll should be added to the Makefile's COMMON_LIBS

It already is?

pavlos256 · 2014-04-26T23:15:07Z

You're right, it's there. Sorry, I was probably looking at the wrong branch.

Smilex · 2014-04-27T03:00:40Z

Overall I'd recommend you break lines more often.

pchote · 2014-04-27T05:56:47Z

Updated with the more fixes.

Mailaender · 2014-05-02T11:26:58Z

packaging/linux/buildpackage.sh

 # Copy files for OpenRA.Game.exe and OpenRA.Editor.exe as well as all dependencies.
 make install-all prefix="/usr" DESTDIR="$PWD/packaging/linux/$ROOTDIR"

+# Native library dependencies
+cp "$DEPSDIR"/* "$PWD/packaging/linux/$ROOTDIR/usr/lib/openra/" || exit 3


Doing this here instead of in the Makefile will probably break all thirdparty package installation upon update.

On the other hand they probably don't want our native binaries bundled so I guess it is okay.

These are OS-specific deps, so we don't want to do this in the (theoretically) platform-agnostic makefile.
In the best case, we would then need to go and delete these files again in the OSX packaging script, which is ugly.

New Lua API.

Mailaender · 2014-05-02T11:29:58Z

This was difficult labour. Thanks.

Mailaender · 2014-05-02T11:34:08Z

Pinging downstream maintainers @cerebrum, @ckorn and @hasufell as we made a tough decision to bundle native libraries, because of http://www.mono-project.com/Config_DllMap limitations and new versions not being available in LTS distributions. This will require some additional sed fixes to use system libraries in your packaging scripts.

Mailaender · 2014-05-02T11:35:30Z

Also pinging @xaionaro as this might disrupt the Hetzner auto-update scripts as make dependencies && make install won't be enough.

hasufell · 2014-05-02T12:36:38Z

I decided to fix this upstream https://build.opensuse.org/request/show/232302 instead.

That does not look right. These kind of things should never happen. Instead, someone needs to make debian stop this crap. Otherwise it should be fixed on related consumer packages, not library level... but I don't know about openSUSE policies. Also mind that the pkgconfig files of lua are hacked as well.

If there is something actually wrong, then it can be fixed upstream at lua. Instead debian uses wrong approaches to ship lua and ends up in their current mess.

If we don't follow common interfaces and standards and add random hacks like debian does, then we encourage people to do the same and help spreading wrong usage of distro-specific pkgconfig files, uncommon library SONAMES and so on.

This isn't the first time happening, there was way more serious breakage (like skype linking to non-existing SONAMES and modified libraries only present in debian). Make people aware that debian is bad for development and don't do as they do just because it's the easiest workaround.

Mailaender · 2014-05-02T12:47:31Z

I agree and hate Debian/Ubuntu for it, but sadly they are the distribution the majority uses or bases their own ones upon. Also note how you just received a WONTFIX 1 year ago without further comment. This does not convince me that we can start to reason with them. In the end you are always fucked, because you get treated with "It works on Ubuntu, not on SUSE, you suck!" and everyone claims that Linux is not a sophisticated work platform because no one adheres to standards.

I will probably experiment with http://software.opensuse.org/package/openra and invent a make native-dependencies or share the sed strings required to have this use system dependencies instead. Maybe even for the platform-independent CLI DLLs in thirdparty. We will see.

obrakmann · 2014-05-02T12:49:16Z

FWIW, the naming convention used by Debian's lua packages is in accordance with their policy manual. Good luck persuading them to change that.

hasufell · 2014-05-02T13:36:30Z

FWIW, the naming convention used by Debian's lua packages is in accordance with their policy manual. Good luck persuading them to change that.

It wouldn't be so bad if they provide a proper abstraction layer that prevents people from using these library names directly. FYI... gentoo does a lot of very terrible hackery to make python, ruby etc work the way we want. But that is hidden from the developer/user. In contrast... changing names of pkgconfig files is so terrible, that I don't know what to say.

I will probably experiment with http://software.opensuse.org/package/openra and invent a make native-dependencies or share the sed strings required to have this use system dependencies instead. Maybe even for the platform-independent CLI DLLs in thirdparty. We will see.

Can you not just fix that in the code, so that it tries to load different libs in that order: liblua.so.5.1, liblua.so.5, liblua.so, liblua5.1.so?

Also mind that lua upstream doesn't even support shared libs in the first place (as in, no target in the Makefile). They were unable to get it working on all supported platforms without libtool (they didn't explain what's wrong with libtool). Not sure why that is any reason, but anyway. Just saying that the "5.1" etc are pretty much guesswork. That however isn't a problem as long as you use the pkg-config files which obviously isn't possible here. So technically... this is already an unsupported (but reasonable) hack, since static-only libs are a security problem.

Mailaender · 2014-05-02T13:52:06Z

In IRC I was told that @xamarin uses *.dll.config.in files that get configured during the build to workaround the issue that it always uses the last entry, not the first matching. We can investigate that.

xaionaro · 2014-05-02T15:26:38Z

@Mailaender:

Also pinging @xaionaro as this might disrupt the Hetzner auto-update scripts as make dependencies && make install won't be enough.

Does current bleed servers work right? (on Hetzner)

Mailaender · 2014-05-02T15:40:10Z

@xaionaro Yes, it does work. Looks like the dedicated servers don't require Lua at all.

xaionaro · 2014-05-02T15:48:46Z

@Mailaender, ok. Anyway notify me when the release/playtest will be, please :)

Mailaender · 2014-05-02T15:50:14Z

We are also trying to persuade @lua to properly support shared libraries.

hasufell · 2014-05-04T14:49:27Z

On an almost unrelated note: is it possible to use the static lib with C#? I don't know enough about that language and mono.

Mailaender · 2014-05-04T14:55:24Z

You always use some kind of wrapper. See https://github.com/cdhowie/Eluant/blob/master/Eluant/LuaApi.cs on how the bindings interact with the Lua DLL.

hasufell · 2014-05-04T14:59:47Z

You always use some kind of wrapper. See https://github.com/cdhowie/Eluant/blob/master/Eluant/LuaApi.cs on how the bindings interact with the Lua DLL.

That doesn't tell me if you can achieve the same with a static lib.

Unit158 · 2014-05-04T15:04:37Z

I think you would need to compile the static lib into mono, which is not desired.

hasufell · 2014-05-04T15:08:21Z

I think you would need to compile the static lib into mono, which is not desired.

Ouch, yeah.

Mailaender · 2014-05-22T10:18:12Z

I addressed Linux packaging in #5365 and #5391 to ease the process and prefer system dependencies again. Please contribute your distributions lsb_release -is output and .so name policy.

Mailaender · 2014-11-01T16:34:23Z

I brought back the jets in https://github.com/Mailaender/OpenRA/compare/migs-lua-desert-shellmap but I am not sure if it is worth bringing them back as I don't really see them on my small laptop screen.

pavlos256 reviewed Apr 25, 2014
View reviewed changes

pchote reviewed Apr 26, 2014
View reviewed changes

ScottNZ reviewed Apr 26, 2014
View reviewed changes

Mailaender reviewed Apr 26, 2014
View reviewed changes

Mailaender reviewed May 2, 2014
View reviewed changes

Mailaender added a commit that referenced this pull request May 2, 2014

Merge pull request #5190 from pchote/new-lua

1a432e9

New Lua API.

Mailaender merged commit 1a432e9 into OpenRA:bleed May 2, 2014

Mailaender mentioned this pull request May 3, 2014

Added an in-game map editor #4496

Closed

pchote mentioned this pull request May 3, 2014

Remove X11 dependency from OSX. #5244

Merged

pchote deleted the new-lua branch May 16, 2014 09:38

Mailaender mentioned this pull request May 18, 2014

Installing from CD fails because of file name case (Linux) #3892

Closed

This was referenced May 25, 2014

Fixed the OpenRA.Game.exe start menu link #5449

Merged

Added support for SDL2 and Lua 5.1 system libraries #5460

Merged

Port missions to the new Lua API #5490

Closed

pchote referenced this pull request in Mailaender/OpenRA Jun 30, 2014

add new mission win state properties

88c713d

New Lua API. #5190

New Lua API. #5190

Conversation

pchote commented Apr 25, 2014

DanAE111 commented Apr 25, 2014

Mailaender commented Apr 25, 2014

ghost commented Apr 25, 2014

Choose a reason for hiding this comment

pchote commented Apr 26, 2014

Choose a reason for hiding this comment

pchote commented Apr 26, 2014

Choose a reason for hiding this comment

pchote commented Apr 26, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pavlos256 commented Apr 26, 2014

pchote commented Apr 26, 2014

pavlos256 commented Apr 26, 2014

Smilex commented Apr 27, 2014

pchote commented Apr 27, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Mailaender commented May 2, 2014

Mailaender commented May 2, 2014

Mailaender commented May 2, 2014

hasufell commented May 2, 2014

Mailaender commented May 2, 2014

obrakmann commented May 2, 2014

hasufell commented May 2, 2014

Mailaender commented May 2, 2014

xaionaro commented May 2, 2014

Mailaender commented May 2, 2014

xaionaro commented May 2, 2014

Mailaender commented May 2, 2014

hasufell commented May 4, 2014

Mailaender commented May 4, 2014

hasufell commented May 4, 2014

Unit158 commented May 4, 2014

hasufell commented May 4, 2014

Mailaender commented May 22, 2014

Mailaender commented Nov 1, 2014