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

Put viewer's Lua functionality behind a feature flag, default off. #2523

Merged
merged 8 commits into from
Sep 9, 2024

Conversation

nat-goodspeed
Copy link
Collaborator

When disabled, the Lua machinery simply returns an error message.

Remove Lua floaters from the default menu structure, but add a Lua autorun script to dynamically re-add them if Lua is enabled.

Add tests to verify that llless() correctly handles signed <=> unsigned
comparison, which native "<" does not.
Appending is effected by passing position == getItemCount(). Until now,
insert() disallowed that value, so you could insert before the last existing
entry but not after it.
'pos' is a 0-relative index at which to insert the desired menu item or
separator. If 'pos' is omitted, the item is appended to the menu.
Make central Lua engine functionality conditional on that flag.
Add a menus.lua autorun script that waits until login, then adds the Lua
floaters back into the Develop->Consoles menu where they were originally.

Extend UI.addMenuItem() and addMenuSeparator() to support pos argument.
// Reserve -1 return to mean event has no "pos" key.
S32 get_pos(const LLSD& event, U32 size)
{
return event["pos"].isInteger()? llclamp(event["pos"].asInteger(), 0, size) : -1;
Copy link
Contributor

Choose a reason for hiding this comment

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

0 menu position is used for 'double line' in the top menu (which allows to undock that menu). Should we allow inserting new items before it, basically pushing 'double line' down? It's ok for me if you think we should (though it's somewhat inconsistent), Otherwise, let's clamp to 1.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hmm! I based this llclamp() call on the test in LLMenuGL::insert(). Are you saying that if we allow inserting at 0, tearing off the menu would leave that first (inserted) item attached to the top menu?

If we change this to 1, should we also change the test in LLMenuGL::insert()?

Copy link
Contributor

@maxim-productengine maxim-productengine Sep 9, 2024

Choose a reason for hiding this comment

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

Using test_top_menu.lua: current script vs inserting "Debug console" at 0.
image

I'm not sure about LLMenuGL::insert(). As far as I remember, only top menus have these 'tear-off' items(LLMenuItemTearOffGL). So, it makes sense to allow inserting at 0 in general( maybe, the exception is the top menu).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The menus.lua autorun script I added definitely manipulates the branch menu Develop->Consoles. Granted, it's not trying to insert at the top.

From what you're saying, to clamp this properly I think we would need to query the specified parent menu to discover whether it's a top menu or a branch menu.

@nat-goodspeed nat-goodspeed merged commit 0c451a6 into release/luau-scripting Sep 9, 2024
11 checks passed
@nat-goodspeed nat-goodspeed deleted the lua-feature-flag branch September 9, 2024 21:14
@github-actions github-actions bot locked and limited conversation to collaborators Sep 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants