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

WIP: Added YueScript support #22

Closed
wants to merge 1 commit into from
Closed

WIP: Added YueScript support #22

wants to merge 1 commit into from

Conversation

bram-dingelstad
Copy link

@bram-dingelstad bram-dingelstad commented Apr 17, 2022

It still needs a lot of work, but I wanted to update you on this. We've been talking on DM for a while, but this is the fruit of my labour thusfar.

Not that many changes, but it does allow the addition of a transpiled language as an addition to the project.

Some things I still need to do in order to be complete enough for merging:

  • Make an automatic builder for YueScript shared library
  • In case we don't want YueScript as a dependency: detect presence of yue.so and enable feature accordingly
  • Throw appropriate errors for when there are syntax errors
  • Use YueScript's native class / extend declaration to register as class_name and extends property
  • Fix some tabbing issues

I'll leave some notes at specified code locations for more elaborate explanations.

Copy link
Author

@bram-dingelstad bram-dingelstad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lemme know what you think!

@@ -351,6 +351,51 @@ static godot_pluginscript_language_desc lps_language_desc = {
},
};

static godot_pluginscript_language_desc lps_yue_language_desc = {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made a specific lps_yue_language_desc for custom names, extension and reserved keywords.
Turns out you can just register multiple language in one GDNative binding! Super cool :)

Since all of the functions are the same, it means changes / improvements on the Lua side will automatically be available for YueScript.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Turns out you can just register multiple language in one GDNative binding!

Yeah! Not only that, if we had any NativeScript classes or anything else we could add in the same library as well =]

src/language_gdnative.c Show resolved Hide resolved
src/language_gdnative.c Show resolved Hide resolved
@@ -97,6 +98,14 @@ pluginscript_callbacks.script_init = wrap_callback(function(manifest, path, sour
manifest = ffi_cast('godot_pluginscript_script_manifest *', manifest)
path = tostring(ffi_cast('godot_string *', path))
source = tostring(ffi_cast('godot_string *', source))
local is_yue = path:sub(-4) == '.yue'
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have to fix tabbing on this as well (or most of the code rather).
I basically just check if the file that's being loaded ends with .yue, i'll use this property later

Copy link
Owner

@gilzoide gilzoide Apr 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of adding specific extra code for Yuescript in script_init, maybe you could create a new callback yuescript_init that does the translation, that then calls script_init for going through the actual import process. This would need a new C function hooked in the Yuescript PluginScript, but this shouldn't be a problem. Also the source and possibly path parameters could need to support both Lua strings or godot_strings. Maybe always receive Lua strings and create a lua_script_init for forwarding the godot_string already transformed, idk.


if is_yue then
local yue = require('yue')
local codes, err, globals = yue.to_lua(source)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the file is a .yue file it loads the YueScript shared library (which we can compile for our user or ask our user to obtain themselves).
It then compiles the source to be Lua code. The rest of the code should be compatible from this point

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'd better compile and/or distribute the Yuescript shared library to end users, since it will be needed in runtime for the game/app. If the code would be compiled ahead-of-time as part of the build process, for example, than it would be more ok (although still somewhat annoying) to ask devs for obtaining it by themselves.

if type(script) ~= 'table' then
api.godot_print_error('Script must return a table', path, path, -1)
return
end

if is_yue then
if script.__base ~= nil then
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In case we detect a .yue file, we check if it exports a YueScript class instead of a table.
When it does we change the script to script.__base so it can detect the methods and variables of the class.

Copy link
Owner

@gilzoide gilzoide Apr 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't quite like these if is_yue branches, I think it's better to separate the code that parses Lua code and Yuescript code and get the script definition table. The import code below could still be the same for both languages, extracted to a new function that receives the input table and script manifest to fill.
Yuescript would then pass script.__base to it and Lua would just forward its script.

local known_properties = {}
for k, v in pairs(script) do
if k == 'class_name' then
if is_yue and (
k == '__class' or
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a simple ignore for the values of __class, __index and __base. Let me know if there is a better way to go about this.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, maybe ignoring anything that starts with __, even for Lua, makes sense? They are usually metatable stuff and is possibly not important for Godot. We'd need to warn users about this behavior, of course, but seems ok to me.

@@ -266,6 +266,9 @@ function export(metadata)
return prop
end

-- Another alias for `export` since it's a reserved keyword in
-- YueScript
exp = export
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a small alias for export since it's a reserved keyword for YueScript. There might be a better name for this alias, but couldn't find one for now 😅

I could also add an macro for this in YueScript so I can just use the property method instead.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exp sounds more like an exponential function of math libraries. Maybe exported or exported_property is a better name. Idk, exp just doesn't sound like a good name to me.

.type = "YueScript",
.extension = "yue",
.recognized_extensions = (const char *[]){ "yue", NULL },
.init = &lps_language_init,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Godot will call this init function twice, once for each language, so that 2 lua_States will be created and fully initialized.
In the end, only the second one will be used, since there is the lps_L global variable that is used by almost all callbacks.
There will probably be no leaks, since the first one will still get closed in lps_language_finish, but it's better not having this duplicated state.

@@ -365,6 +410,7 @@ GDN_EXPORT void PREFIX_SYMBOL(gdnative_init)(godot_gdnative_init_options *option
in_editor = options->in_editor;
if (in_editor) {
lps_register_in_editor_callbacks(&lps_language_desc);
lps_register_in_editor_callbacks(&lps_yue_language_desc);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The editor callbacks implemented ("validate", "get_template_source_code" and "make_function") should probably have a specific implementation for Yuescript, since the language is quite different.

local known_properties = {}
for k, v in pairs(script) do
if k == 'class_name' then
if is_yue and (
k == '__class' or
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, maybe ignoring anything that starts with __, even for Lua, makes sense? They are usually metatable stuff and is possibly not important for Godot. We'd need to warn users about this behavior, of course, but seems ok to me.


if is_yue then
local yue = require('yue')
local codes, err, globals = yue.to_lua(source)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'd better compile and/or distribute the Yuescript shared library to end users, since it will be needed in runtime for the game/app. If the code would be compiled ahead-of-time as part of the build process, for example, than it would be more ok (although still somewhat annoying) to ask devs for obtaining it by themselves.

@@ -97,6 +98,14 @@ pluginscript_callbacks.script_init = wrap_callback(function(manifest, path, sour
manifest = ffi_cast('godot_pluginscript_script_manifest *', manifest)
path = tostring(ffi_cast('godot_string *', path))
source = tostring(ffi_cast('godot_string *', source))
local is_yue = path:sub(-4) == '.yue'
Copy link
Owner

@gilzoide gilzoide Apr 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of adding specific extra code for Yuescript in script_init, maybe you could create a new callback yuescript_init that does the translation, that then calls script_init for going through the actual import process. This would need a new C function hooked in the Yuescript PluginScript, but this shouldn't be a problem. Also the source and possibly path parameters could need to support both Lua strings or godot_strings. Maybe always receive Lua strings and create a lua_script_init for forwarding the godot_string already transformed, idk.

if type(script) ~= 'table' then
api.godot_print_error('Script must return a table', path, path, -1)
return
end

if is_yue then
if script.__base ~= nil then
Copy link
Owner

@gilzoide gilzoide Apr 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't quite like these if is_yue branches, I think it's better to separate the code that parses Lua code and Yuescript code and get the script definition table. The import code below could still be the same for both languages, extracted to a new function that receives the input table and script manifest to fill.
Yuescript would then pass script.__base to it and Lua would just forward its script.

@@ -266,6 +266,9 @@ function export(metadata)
return prop
end

-- Another alias for `export` since it's a reserved keyword in
-- YueScript
exp = export
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exp sounds more like an exponential function of math libraries. Maybe exported or exported_property is a better name. Idk, exp just doesn't sound like a good name to me.

@@ -351,6 +351,51 @@ static godot_pluginscript_language_desc lps_language_desc = {
},
};

static godot_pluginscript_language_desc lps_yue_language_desc = {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Turns out you can just register multiple language in one GDNative binding!

Yeah! Not only that, if we had any NativeScript classes or anything else we could add in the same library as well =]

Comment on lines +373 to +375
#if LUA_VERSION_NUM >= 502
"_ENV",
#endif
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is _ENV a thing in Yuescript? If not, this could be removed.

@bram-dingelstad bram-dingelstad closed this by deleting the head repository Feb 3, 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

2 participants