Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Major rework into V2 #37

Merged
merged 60 commits into from Apr 29, 2024
Merged

Major rework into V2 #37

merged 60 commits into from Apr 29, 2024

Conversation

haath
Copy link
Member

@haath haath commented Aug 10, 2023

Summary

I am currently preparing a V2 release of this library with wall-to-wall breaking changes.
The main goal is to Haxify the library, to make working with classes more natural and hide certain Lua eccentricities where possible. The main changes are the following:

  • Remove the script properties typedef, and instead use regular class fields to define properties.
  • Rename all API functions so that it's all camelCase on the Haxe side.
  • Rename extern enum fields from THIS_CASE to PascalCase.
  • Implement some automated way for type-safe cross-script interactions. (accessing properties, sending messages)
  • Hide Lua types where possible (e.g Table).
  • Change the versioning scheme.

@nadako I will work on this PR over time to give you or anybody else a chance to give feedback before everything goes into master 😄. I will also treat this PR as a documentation of the changes, so that I can link to it from the README when warning about the breaking changes.

Before

typedef MyScriptProperties =
{
    @property() var editorProperty: Hash;
    @property(42) var editorPropertyWithDefaultValue: Int;
    var nonEditorProperty: WhateverType;
}

class MyScript extends Script<MyScriptProperties>
{
    override function on_reload(self: MyScriptProperties)
    {
        self.editorPropertyWithDefaultValue = 50;
        Go.animate('#', 'editorPropertyWithDefaultValue', PLAYBACK_ONCE_FORWARD, 100);
    }
}

After

class MyScript extends Script
{
    @property var editorProperty: Hash;
    @property var editorPropertyWithDefaultValue: Int = 42;
    var nonEditorProperty: WhateverType;

    override function onReload()
    {
        editorPropertyWithDefaultValue = 50;
        Go.animate('#', MyScriptProperties.editorPropertyWithDefaultValue, OnceForward, 100);
    }
}

Detailed changes

Rework of Script and properties definition.

I decided to do this for the following reasons:

  • Carrying a self variable everywhere felt very unnatural in Haxe. If scripts are classes, then they should have their own class variables.
  • With properties stored in self, it was unclear to the user what happened with actual class variables under the old implementation. In reality only a single instance of the class got created, which effectively made everything static, but in an ambiguous way.
  • If the user wants to extend a Script class, he has to either make it generic or be forced to take over its properties without being able to define any new ones. In the generic case, the boilerplate was getting quite awkward since the new class had to care about the super class' properties.

How does the new way work?

As seen above in the summary, properties are now simply variables of the class. This is made possible with an @:autoBuild macro that gets called for all classes extending one of the Script types. This new build macro takes all of the non-static variables, and replaces them with get/set properties that effectively change all interactions to accesses of the script's self type on the lua side. In the example above, the generated code for the editorProperty variable looks like this:

class MyScript extends Script
{
    @property var editorProperty(get, set): Hash;

    @:noCompletion @:pure
    inline function get_editorProperty(): Hash { 
        return untyped _G.self.editorProperty;
    }

    @:noCompletion
    inline function set_editorProperty(value: Hash): Hash { 
        untyped _G.self.editorProperty = value; 
        return value;
    }
}

In my tests, the Haxe compiler will always properly inline the getter and the setter, so doing it this way introduces no overhead. Now you might be wondering: why the _G.self? The reason is that we want the script's self reference to be in scope in any function that may get called inside the script. The simplest way to do that was to adapt the script generation macro to now produce all callbacks as shown below:

go.property("editorProperty", "")

require "main"

function update(self, dt)
    tmp = _G.self
    _G.self = self
    _hxdefold_.MyScript_update(dt)
    _G.self = tmp
end

And voila, now the script callbacks do not require a self argument anymore, and all script properties can be accessed as if they were class members!

Note that in this example, nonEditorProperty will also have the same getter and setter generated, but because it lacks the @property tag it will not have a go.property statement generated. This is consistent with what we were doing before, where basically you can store whatever you want on the self object when it's not showing up in the editor or as a named property.

New versioning scheme

So far we've been versioning the library by just mirroring the corresponding Defold API version.
However this has been quite limiting. Multiple times I've discovered a bug in hxdefold, but I had to wait for a new Defold version to be released before I could push a fixed version on Haxelib. So starting with v2.0.0, we will follow a standard Major.Minor.Patch scheme.

  • Major number will be incremented for significant reworks that are certainly forward-breaking.
  • Minor number will be incremented when updating to a new Defold API version. These updates may come with forward-breaking changes.
  • Patch version will be incremented for bug fixes and potentially backwards-breaking changes.

Additionally, I plan to add some startup code that will check the Defold engine version and print a warning to the user if it is not supported by his hxdefold version.

Renaming to camelCase

Having the same codebase with both camelCase haxe function calls and snake_case lua API calls looked a bit uncomfortable. So using the @:native tag, now all of the API functions will be camelCase on the Haxe side and generate the correct snake_case calls in Lua.

I will also be renaming the extern enum fields that represent Defold constants. On the Lua side they are in THIS_CASE, and I'll rename them into PascalCase to match the Haxe standard for enum constants.

However, I will not rename table fields and hash values like message names. This is because it would create an inconsistency from the users perspective, since in that case the message GoMessages.setParent would be equal to hash("set_parent") but not hash("setParent").

Automatically generated property hashes

Another new feature is that now for each script, a class with the list of property hashes will be generated automatically.
Again looking at the MyScript example above, the MyScriptProperties class will get defined at the top-level package and will look like this:

class MyScriptProperties
{
    public static final editorProperty: Property<Hash> = new Property("editorProperty");
    public static final editorPropertyWithDefaultValue: Property<Int> = new Property("editorPropertyWithDefaultValue");

    /**
     *  Properties with comments defined in the original script class will also have them here!
     */
    public static final propertyWithComments: Property<Float> = new Property("propertyWithComments");

    /**
     *  Vector and quaternion properties, will also have hashes for their x, y, z, w components generated automatically!
     */
    public static final vec: Property<Vector3> = new Property("vec");
    public static final vecX: Property<Float> = new Property("vec.x");
    public static final vecY: Property<Float> = new Property("vec.y");
    public static final vecZ: Property<Float> = new Property("vec.z");
}

This provides pre-hashed and typed property ids which can be used to interact with the properties from another script.

Go.set(myScriptUrl, MyScriptProperties.editorPropertyWithDefaultValue, 66);

I have also verified that Haxe's dead code elimination will not include unused hashes in the final build!

Type-safe factory object creation

By default when only exposing the defold factory API, the factory.create() function takes a generic Table<String, GoProperty> as the properties argument. The first problem with that, is that this is of course not type-safe and leaves it up to the developer to manually pass the correct property names when creating an object. The second problem was that the most convenient way to use this method, was to employ Table.fromDynamic({ ... }) with some typedef, and this approach uses reflection which is not very performant.

My current (WIP) solution is to have the script builder macro generate a public static method on each script type, which allows the user to create a properties table in a type-safe way and as efficiently as possible. The method generated for the MyScript example would be the following:

public static function create(?editorProperty: Hash, ?editorPropertyWithDefaultValue: Int): lua.Table<String, GoProperty>
{
    return untyped __lua__('{ editorProperty = editorProperty, editorPropertyWithDefaultValue = editorPropertyWithDefaultValue }');
}

Then this object can be created like this:

Factory.create('#factory', null, null, MyScript.create(hash("value"), 72));

Other minor changes

  • The Script, GuiScript, and RenderScript classes are now abstract, and their constructors are empty and final. Instantiating script classes is not allowed, and now it will be properly blocked by the compiler.
  • It will now be possible to have property variables of a type that is an abstract over a property type! (e.g abstract MyGo(Hash))
  • The live update methods were moved out of the Resource class and into their own LiveUpdate extern class. This should have been done some time ago, but I probably missed it.
  • Properties can now be read-only in the context of the script. By using either @readonly or final, the property will not have a setter generated. (note that Go.set and Go.animate would still be possible, so this is just for convenience)
  • The build macro now generates empty luv.lua, bit32.lua and socket.lua files at the project root. This is an automated workaround to a known issue with the Defold builder, and may be what we stick with long-term.

@skial skial mentioned this pull request Aug 10, 2023
1 task
@andrew-git
Copy link

When trying to build in Defold I get

/main.lua
The file '/luv.lua' could not be found.

In generated main.lua I have:

if package.loaded.luv then
  _hx_luv = _G.require("luv");

Looks related to https://forum.defold.com/t/including-a-lua-module-solved/2747/2
I tried to replace with

if false then
  _hx_luv = _G.require("luv");

but for some reason Defold (Lua) tries to process it anyway, even if the condition is not met. Only deleting the line with _hx_luv = _G.require("luv"); helped.

@haath
Copy link
Member Author

haath commented Sep 30, 2023

The file '/luv.lua' could not be found.

TLDR; I pushed a fix on the v2 branch

So this is a known issue for a while, the workaround is to create an empty luv.lua file at the project root.

This is because the Defold builder takes these _G.require("luv") statements and looks for a local file to import.
I realize now this is not documented anywhere in hxdefold though...oops!

I did however also realize that this can and should be addressed by the library - even in a workaround form, - and not be left up to the user. So I pushed a fix with the latest commit, which makes the build macro generate empty files luv.lua, bit32.lua, and socket.lua at the project root. These are the three that I know of so far that can cause this issue, any others we find in the future we can also add.


And one more thing:

I tried to replace with

You should never have to touch generated files 😉

You should be compiling Haxe freely every time you build the game! It can even be done automatically.

@andrew-git
Copy link

andrew-git commented Sep 30, 2023

Working now, thanks

You should never have to touch generated files 😉

Yes, I know, temporary solution just to verify where exactly issue and run project.
Btw maybe point somewhere min Haxe version? v1.5.0 worked with 4.1.4, but v2 requires new features, maybe 4.2.0+

@haath
Copy link
Member Author

haath commented Oct 2, 2023

Btw maybe point somewhere min Haxe version?

Definitely! I think currently it's 4.3.0+ since I've apparently used the new null coalescing operator somewhere.
I can't find any package registry where 4.3 is not available yet, but nonetheless it might be a good idea to downgrade and hold off on it a bit longer.

Will see about adding some simple CI tests with a version matrix, and a badge showing the minimum needed version.

@andrew-git
Copy link

Maybe add something like below to Script class (not sure about naming) for quick access?

inline static var selfGo:String = "."; //gameObject / currentGameObject
inline static var selfComponent:String = "#"; //selfScript

@haath
Copy link
Member Author

haath commented Oct 4, 2023

Maybe add something like below to Script class (not sure about naming) for quick access?

inline static var selfGo:String = "."; //gameObject / currentGameObject
inline static var selfComponent:String = "#"; //selfScript

What would be the benefit of that? The user would then write for example Go.delete(selfGo) instead of Go.delete("."). I don't see how it's quicker.

Unless you meant something else...

@andrew-git
Copy link

andrew-git commented Oct 5, 2023

What would be the benefit of that? The user would then write for example Go.delete(selfGo) instead of Go.delete("."). I don't see how it's quicker.

Unless you meant something else...

I like autocomplete, to avoid typos and not to remember exact values )
Actually if user prefer use "." he can continue do so, it will not limit him.
Just an option for convenience.

@Eugnee
Copy link
Contributor

Eugnee commented Oct 11, 2023

Hi @haath
Thx u for ur work. Great job actually!
I absolutely agree with the most ideas u trying to implement.
And i have 2 pretty general questions, maybe not so related to this MR.

  • Do u use haxe + defold in production? Or, are u going to use?
  • What do u think about transpilation to Lua by Haxe? Is it good enough? I am not a Lua specialist, so, for me it's really hard to understand how optimal is that transpilation.

@haath
Copy link
Member Author

haath commented Oct 12, 2023

Hey @Eugnee, thanks a lot!

* Do u use haxe + defold in production? Or, are u going to use?

I wish... Making games is just a hobby for me so I probably won't be commercially shipping anything soon 😞
I only have this 10-day jam submission made with hxdefold: https://haath.itch.io/valchemy

But I haven't even touched any other game engine since discovering hxdefold 5 years ago, if that counts for anything.

* What do u think about transpilation to Lua by Haxe? Is it good enough?  I am not a Lua specialist, so, for me it's really hard to understand how optimal is that transpilation.

I hardly know any Lua either actually. 😅
But you don't need to be a specialist to see that a for-loop turns into a for-loop.

If your concern is performance, then the best thing to do is run benchmarks. I actually have already rewritten the Bunnymark example in Haxe. You can try out the HTML5 build on the browser:

In my opinion the transpilation is not an issue. The main thing that I perceive as a potential risk for large-scale projects is the way that hxdefold works: your whole codebase ends up in a single Lua file called main.lua, and then ALL generated Defold scripts import that file.
Could this main.lua have an upper size limit before it starts having an impact on performance, or even not compiling at all? Or is the Defold Lua compiler able to only bring into context what it needs and ignore the rest?
I don't have the answer to that unfortunately, but what I do know is that Haxe is flexible enough that even if such a limit was reached, one could always split his codebase into segments and produce multiple mainX.lua files!

@Eugnee
Copy link
Contributor

Eugnee commented Oct 12, 2023

Sounds great!
But, for example, I see topics like https://forum.defold.com/t/lua-and-luajit-best-practices/68405/2
And it looks like Lua has some very specific and annoying actually rules of thumb)
I didn't dive in it yet, but hope they are taken into Haxe transpilation :)

@haath
Copy link
Member Author

haath commented Oct 12, 2023

Sounds great! But, for example, I see topics like https://forum.defold.com/t/lua-and-luajit-best-practices/68405/2 And it looks like Lua has some very specific and annoying actually rules of thumb) I didn't dive in it yet, but hope they are taken into Haxe transpilation :)

I wouldn't be too concerned with that honestly.

A lot of these just boil down to you writing Haxe code that would produce the performant Lua you are looking for, and not so much to the Haxe compiler having to produce that good Lua for you. For every 1 line in Haxe you get 1 line in Lua more or less, so it's still you writing the program, not the compiler.

Now if the compiler did in fact make some performance blunders that somebody could identify, and even if the Haxe team decided not to fix them, you can still get out of it by injecting raw Lua code into your Haxe.

@Eugnee
Copy link
Contributor

Eugnee commented Oct 12, 2023

Okay, I got u.
Thx for the explanations.
I am not a Haxe specialist also :)
But, I like it's syntax much more then Lua)

@haath haath marked this pull request as ready for review February 2, 2024 09:51
@hansagames
Copy link

Hi what is status of this MR ? something still need to be done or it waits for something ?

@haath
Copy link
Member Author

haath commented Apr 29, 2024

Hi what is status of this MR ? something still need to be done or it waits for something ?

Hey!

No, I think I'm finished with everything I wanted to add here.
Mostly I left it open to collect feedback (complaints), since these are big changes.

But I guess half a year is enough, so I'll merge it now.
Thanks for the reminder 😄

@haath haath merged commit 9d2128c into master Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants