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

Prevent used globals from being counted multiple times in :ss #1830

Merged
merged 1 commit into from
Jan 28, 2025

Conversation

Mars7383
Copy link
Contributor

After #1486 introduced the global import optimization update in Loadstring, global variables are being counted towards the 200 local variable limit multiple times. This causes the limit to be depleted very quickly in some cases. The change in #1587 addressed a formatting error in LuaY that concealed this issue in LuaX, but didn't fix the root cause of the error, which is what this change should do.

Me and @ServerNoob found that if you only localize used globals once, the number of local variables created comes down significantly. I'm not sure if this is an actual issue or intentional behavior for optimization purposes, but since the exact same script that worked before #1486 is now causing errors (and also works in a normal Luau environment on Roblox, like the Developer Console), I think this is a bug.

Proof of functionality: https://www.youtube.com/watch?v=1IwQZPIvMvw
In the video, I imported Adonis into a new baseplate, and modified the config + LuaY to print local vars. Then, I ran the same testing script on:

  1. the latest LuaX in Adonis (didn't work)
  2. LuaX after making this change (worked)
  3. LuaX before Added Luau global import optimisation #1486 was introduced (worked)

Steps script followed in pof: https://github.com/user-attachments/files/18431880/Steps.txt

@Dimenpsyonal Dimenpsyonal added ✨ enhancement Enhancing or improving existing functionality 🎏 miscellaneous Miscellaneous content labels Jan 16, 2025
Copy link
Contributor

@ccuser44 ccuser44 left a comment

Choose a reason for hiding this comment

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

I am @ccuser44 and I approve pull request.

Also probably in the future the local variables limit count should probably be abolished all together.

@Dimenpsyonal Dimenpsyonal merged commit 9512b57 into Epix-Incorporated:master Jan 28, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ enhancement Enhancing or improving existing functionality 🎏 miscellaneous Miscellaneous content
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants