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

Luau #2815

Closed
wants to merge 11 commits into from
Closed

Luau #2815

wants to merge 11 commits into from

Conversation

reflectrpteam
Copy link

@reflectrpteam reflectrpteam commented Nov 12, 2022

This is an attempt to add a new Lua version along with the default one. In this PR you can find an implementation of an interface allows to switch between Lua 5.1 and Luau in runtime. Can be considered as stable but due to the complexity can potentially contain issues that are expected to be revealed after the release.

lua5.1 and luau libraries are shared now for all build configurations. But Lua_Server and Lua_Client are static. It makes the system flexible on the one hand and resolves the name conflict on the other.

You can enable Luau for a specific resource for client, server or both.

<meta>	
	<lua client="luau" server="luau" both="luau" />	
</meta>

Due to the flexibility of the interface new versions of Lua can be added in the future.

@CrosRoad95
Copy link
Contributor

One thing from my side: if possible, download vendor files from luau repository to do not bloat mta codebase and make future updates easier,
example how you can do this you can find in mta bulletphysics pr :) it contains example for client and server side download of 3-rd party files
#1246

@reflectrpteam
Copy link
Author

Unfortunately, we cannot use the default Luau code in the future. Some adjustments required in the code.

@CrosRoad95
Copy link
Contributor

CrosRoad95 commented Nov 12, 2022

Unfortunately, we cannot use the default Luau code in the future. Some adjustments required in the code.

you can download luau and make a folder with patched files you copy onto luau files

This pr just show how much the mta team doesn't give a damn about this project

@lopezloo lopezloo added the enhancement New feature or request label Nov 12, 2022
@Xenius97
Copy link
Contributor

Xenius97 commented Nov 12, 2022

Instead of < lua > could be better to set lua version per script.
Just like < script src="server.lua" type="server" lang="luau"/ >
If lang is not set script uses 5.1 as default.

@Lpsd
Copy link
Member

Lpsd commented Nov 12, 2022

Instead of < lua > could be better to set lua version per script.
Just like < script src="server.lua" type="server" lang="luau"/ >
If lang is not set script uses 5.1 as default.

This goes against the current implementation whereby each resource has its own Lua VM (on client and server)

@MegadreamsBE
Copy link
Member

Instead of < lua > could be better to set lua version per script. Just like < script src="server.lua" type="server" lang="luau"/ > If lang is not set script uses 5.1 as default.

Disagree, if you have a need to mix Lua versions you should split it up in multiple resources.

@MysterAnonimi
Copy link

Thanks

@Disinterpreter
Copy link
Member

Disinterpreter commented Nov 18, 2022

IMHO pros and cons:

Pros:

  1. Good syntax and language
  2. It is really popular and developed version or Roblox's lua. Maybe we can use their tools in future
  3. Knowing developers made this (this user account hide people which we work many years)

Cons:

  1. It won't work for our luac.mtasa (but you can cache scripts)
  2. Diffrent VMs, like you can't write on Lua and Lualu in one resource (but i think exports for multiple resources are working)

My opinion - lgtm.

@dmi7ry
Copy link

dmi7ry commented Feb 1, 2023

Not finished, but already usable
https://github.com/dmi7ry/Luau-definition-for-MTA

@dmi7ry

This comment was marked as off-topic.

@CrosRoad95

This comment was marked as off-topic.

@MegadreamsBE

This comment was marked as off-topic.

@Lpsd

This comment was marked as off-topic.

@CrosRoad95

This comment was marked as off-topic.

@tederis

This comment was marked as off-topic.

@Lpsd

This comment was marked as off-topic.

@dmi7ry

This comment was marked as off-topic.

@CrosRoad95

This comment was marked as off-topic.

@dmi7ry

This comment was marked as off-topic.

@CrosRoad95

This comment was marked as off-topic.

@tederis

This comment was marked as off-topic.

@Lpsd

This comment was marked as off-topic.

@dmi7ry

This comment was marked as off-topic.

@Lpsd

This comment was marked as off-topic.

@CrosRoad95

This comment was marked as off-topic.

@PlatinMTA

This comment was marked as off-topic.

@multitheftauto multitheftauto deleted a comment from Lpsd Feb 5, 2023
@multitheftauto multitheftauto deleted a comment from PrototypeX Feb 5, 2023
@Pirulax
Copy link
Contributor

Pirulax commented Feb 16, 2023

Unfortunately, we cannot use the default Luau code in the future. Some adjustments required in the code.

you can download luau and make a folder with patched files you copy onto luau files

This.
Making manual patches all the time is not a great idea.
And I'm sure keeping Luau up-to-date is preferred.
Also, what are the changes made? Are they really necessary?
Maybe we can do some workaround to avoid having to modify it.

I want to admit that we don't really have any "test branch" which mta should have. we could use huge playerbase to test >?
features, make in main menu so kind of information to switch to "testing" version of mta if you want to help mta bla bla bla.

I guess there are nightlies, but even for that the PR has to be tested (and be somewhat stable).


So anyways, let's get the ball rolling.

I'd like this to be merged, unlike my attempt at LuaJIT (Also, Luau is better maintained, so it's superior anyways).
My idea for testing is as simple as taking some opensource mta mod, running it, and testing if it works.
If it does, then we can make a nightly out of it, and encourage a few servers to start using it (I'm pretty sure there'd be people willing to help, there always are, shoutout to them!).

On another note...

Before we do all of that, we need some netcode stuff too for checking if luau scripts are signed, until then compiled scripts are a no-go, executing untrusted bytecode is dangerous (Same reason why currently Lua scripts go thru MTA's site, they get signed too)

@hasitotabla
Copy link

Oh wait, if it's possible to select runtime then where's the problem? If the server detects the script is compiled, then convert the meta.xml line to explicitly using lua5.1. Otherwise just load it as luau code...

@Rilot06
Copy link

Rilot06 commented Oct 7, 2024

Actually not even just on a per-resource basis, you can even use different Lua version for server and client side in the same resource, so yeah, I really don't see where the problem is. Yes, there is some CPU and memory overhead because of the translation layer, but they have been using it for more than a year or so. A nightly test for high player count tests would be really great.

@lynconsix

This comment was marked as off-topic.

@sbx320 sbx320 closed this Oct 8, 2024
@dmi7ry
Copy link

dmi7ry commented Oct 8, 2024

If the server detects the script is compiled, then convert the meta.xml line to explicitly using lua5.1. Otherwise just load it as luau code...

Luau has own compiler, of course.

@dmi7ry
Copy link

dmi7ry commented Oct 8, 2024

I just had a quick question, is it possible to detect the language of each script and then run it in that language?

Theoretically it is possible, but it has many pitfalls, when the definition will not work correctly. In general, it is a very unreliable method. The current version is much simpler and more reliable.

If so there is no problem according to me.

There are no problems here anyway

@tederis
Copy link
Member

tederis commented Oct 8, 2024

Just merge and don't support compiled luau scripts yet.

Im talking about all lua legacy. Without legacy support we can close a lot of projects.

Like I said, this PR is 100% backward compatible. Luac works as before for Lua 5.1. But it no longer matters...

@MegadreamsBE
Copy link
Member

@sbx320 What was the reason for the closure? I understand that you probably have your reasons, but it'd be best to share them as there was a lot of support for this PR.

@CrosRoad95

This comment was marked as off-topic.

@Xenius97
Copy link
Contributor

Xenius97 commented Oct 8, 2024

RIP
MTA will never have alternative programming languages. Someone worked on this 2+ years and closed within a second.

@shadylua
Copy link
Contributor

shadylua commented Oct 8, 2024

For about 2 years many developers for Luau have collaborated and shared their knowledge with each other, but what I don't understand is why this topic is closed.

@Fernando-A-Rocha
Copy link
Contributor

RIP MTA will never have alternative programming languages. Someone worked on this 2+ years and closed within a second.

Two things I don't understand:

  1. Why this PR was closed after 2 years of existence by @sbx320 who did not comment anything
  2. Why you guys say that this will never be merged, it's literally ready to be tested in nightly

@Rilot06
Copy link

Rilot06 commented Oct 8, 2024

  1. Why you guys say that this will never be merged, it's literally ready to be tested in nightly

Because it was open for 2 years, and all the "MTA team" did was cry about backwards compatibility, even when it wasn't an issue. Big new features almost never get merged into MTA.

@Xenius97
Copy link
Contributor

Xenius97 commented Oct 8, 2024

About backward compatibility, I read somewhere, maybe in the comments here, that the version number should be raised to MTA 1.7. This would give time for server developers to update scripts.

@dmi7ry
Copy link

dmi7ry commented Oct 8, 2024

About backward compatibility, I read somewhere, maybe in the comments here, that the version number should be raised to MTA 1.7. This would give time for server developers to update scripts.

There is no a reason for it. It's 100% compartible.
Nobody see difference until he decided to switch resource(s) to Luau (then it brings performance, including native compilation support, string interpolation, optional types support, etc).

@sbx320
Copy link
Member

sbx320 commented Oct 8, 2024

@sbx320 What was the reason for the closure? I understand that you probably have your reasons, but it'd be best to share them as there was a lot of support for this PR.

See #2815 (comment)

As there are apparently no intentions to address the mentioned issues, this won't be merged.

@hwbp
Copy link

hwbp commented Oct 8, 2024

staff team is retarded, they dont want to merge anything that requires them to work more than 5 minutes (they would AT least have to create a secure bytecode loader for luau, just like for lua)

@Fernando-A-Rocha
Copy link
Contributor

@sbx320 What was the reason for the closure? I understand that you probably have your reasons, but it'd be best to share them as there was a lot of support for this PR.

See #2815 (comment)

As there are apparently no intentions to address the mentioned issues, this won't be merged.

Could the mentioned issues (that need to be addressed) be recapitulated? I think this information is lost/unclear after the dozens of comments.

@Rilot06
Copy link

Rilot06 commented Oct 8, 2024

@sbx320 What was the reason for the closure? I understand that you probably have your reasons, but it'd be best to share them as there was a lot of support for this PR.

See #2815 (comment)

As there are apparently no intentions to address the mentioned issues, this won't be merged.

"Issues" xdd It's literally an optional language that you don't have to use, there are no issues if you use 5.1...

@CArg22
Copy link

CArg22 commented Oct 8, 2024

Checkouted this branch and ran my gamemode, couldn't notice any issues

@tederis
Copy link
Member

tederis commented Oct 8, 2024

Checkouted this branch and ran my gamemode, couldn't notice any issues

Although this PR will never be merged it's still viable and can be used in your own forks. Have a fun!

@lynconsix
Copy link

I feel very dissatisfied with the lack of appreciation that the MTA team shows in relation to important PRs like this. It's frustrating to see people who don't dedicate themselves to improving the repository, who simply disappear and only reappear when there are great PRs, only to claim that it wouldn't be feasible and close the PR.

@CrosRoad95
Copy link
Contributor

I understand they don't have time, but they could at least once every year, every update introduce one bigger feature

@lynconsix
Copy link

lynconsix commented Oct 8, 2024

I understand they don't have time, but they could at least once every year, every update introduce one bigger feature

True, in time that a "big" PR was not added, this is very frustrating for developers who want more functions or commitment for MTA

@Dark-Dragon
Copy link
Contributor

Could the mentioned issues (that need to be addressed) be recapitulated?

From these options 2 was not an option because luau isn't deemed to be enough of an upgrade to break obfuscated script file compatibility, 3 was not an option because it's likely not going to work even if someone were to put in ridiculous amounts of work to attempt it and 4 would just scrap the pull request. Later an option 5 was presented, which would keep lua 5.1 on mta 1.6 and below and luau from 1.7 onward, however it would divide the community just for the sake of luau.

This leaves us with option 1 which would keep lua 5.1 alongside luau, however the implementation in this pull request introduces a new translation layer which is explained here and requires noticeable overhead in cpu and memory. While the original comment explaining why this was chosen seems to be gone, quotes left enough context to explain that this was a clever trick that made it easier to implement. Other comments seem to suggest a quote on quote proper implementation would have required direct changes to luau directly through a fork or custom code (probably similarly to how lua is currently slightly modified in mta) which would have then required separate maintenance or collaboration from outside. With some of the context being lost I assume the last suggestion to get rid of the overhead was this comment by sbx. There might have been more conversation internally as to why this either wasn't feasible, or not within the scope of the amount of work the PR author is willing to take on, but either way that's where this particular chain of thoughts seems to have ended.

That's it, at least from what I understand.

@hasitotabla
Copy link

Guys revolutionary idea, support 5.1 for a year or so (so servers have time to do their thing), then make it deprecated and yeet the translation layer out the window. 😎👍

@tederis
Copy link
Member

tederis commented Oct 8, 2024

noticeable overhead in cpu and memory

This is not entirely true. The translation layer obviously implies some overheads of course. But they aren't noticeable. The layer is thin and can be considered(for the simplicity) as an additional dereferencing(or indirection). My results: just a few tens of nanoseconds.

@Rilot06
Copy link

Rilot06 commented Oct 8, 2024

Guys revolutionary idea, support 5.1 for a year or so (so servers have time to do their thing), then make it deprecated and yeet the translation layer out the window. 😎👍

You can support it for 10-20 years, it still won't be enough for the MTA team, they want ancient compiled scripts to run too forever..

@Lpsd
Copy link
Member

Lpsd commented Oct 8, 2024

The summary above by @Dark-Dragon is accurate, for this PR to proceed the following items need to be addressed:

  • Luau implementation to be adjusted to reduce/eliminate the unnecessary overheads (context)

Option 1 from previous discussion should be used; meaning Lua 5.1 and Luau are shipped side-by-side with Lua 5.1 being the default (Luau configurable via meta.xml).

Since Lua 5.1 is the default, existing compiled scripts are not affected. We can add support for Luau script compilation at a later date.

There may be other things to consider, but these are the main items to get this moving.

As the original author(s)/team have stated they don't have the time for these changes (and otherwise may simply not agree on the suggested approach), it would be up to someone else to adjust or re-implement Luau based on that approach.

I'll lock the PR now to avoid any more unnecessary convolution of this thread. These are the facts, use them as you wish.

@multitheftauto multitheftauto locked as too heated and limited conversation to collaborators Oct 8, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.