Integrate Luau scripting into the viewer. #785

nat-goodspeed · 2024-02-09T21:48:20Z

Some features were added in secondlife/viewer-private#20, supplement generated release notes with those.

nat-goodspeed · 2024-02-27T17:44:09Z

recheck

Nicky-D · 2024-04-03T16:48:08Z

indra/llcommon/lua_function.cpp

+            popper.disarm();
+            // Table keys are all integers: are they reasonable integers?
+            // Arbitrary max: may bite us, but more likely to protect us
+            size_t array_max{ 10000 };


I believe you an constexpr or const that. This will make the intent of it being some constant upper limit much clearer

Nicky-D · 2024-04-03T16:53:52Z

indra/llcommon/lua_function.cpp

+            }
+            // right away expand the result array to the size we'll need
+            LLSD result{ LLSD::emptyArray() };
+            result[highkey - 1] = LLSD();


Is this really safe, again my lack of LUA-FU is obivious, but what if I construct a table with just one entry.
Lower bound will be equal to higher bound

This -1 is to adjust for the difference between Lua 1-relative indexing and C++ 0-relative indexing. If the high key of a Lua array is 10, there are 10 entries. To immediately reserve that much space in an LLSD array, we assign to result[9].

This is correct, buy my idle musing had been about a 1 length table, it would still not crash I assume. But highkey would be equal to lowkey and thus in result the first item would be overwritten with a LLSD()

As long as that's possible, coming here with a table of size 1

I stipulate that this is a lot of code to wade through, though I find myself doubting that breaking out subsections would make it easier to follow. The basic idea, though, is that we tackle arrays in two passes. The first pass just validates and collects keys. Then we validate that there aren't huge gaps (to avoid having to allocate a ridiculously large contiguous LLSD array) and find the highkey. All this is before storing any values.

Finally we grow the LLSD result array with a trivial assignment and then make a whole second pass to populate its values.

All this is because Lua refuses to guarantee the order in which lua_next() retrieves key/value pairs.

I can attest that you did a very good in (in the code comments) to describe what happens and how such a array is "inflated" post construction.

But my worry is still not really addressed (I think). Which is being able to hand craft some LUA code that would exploit this logic.
Which means I cannot let go yet, sorry for that :)

What would help if you got an lua example table (LUA_TTABLE) that cam be converted.
I think it would look something like this?

local tbl {} tbl[1] = valx tbl[20] = valy

Which then would increase the final C++ object from 2-19 with empty values.

Now my concern (valid or not, I do not know) is this

local tbl {} tbl[1] = valx

Table with one element, consequently after parsing it would just overwrite valx with an empty LLSD

I don't yet understand the problem situation, sorry. If either the first tbl or the second tbl is passed to lua_tollsd(), I think it will work correctly. But those two tables would be two distinct calls to lua_tollsd(). The table passed into a single call to lua_tollsd() wouldn't change shape during the function, because it's a synchronous call from the Lua VM.

There might be in fact no issue at all. bur rather me having a misconception how the LUA VM does interact with the C++ code base.

I will have to step through that with a debugger at least once, this should give me some clarity. Meanwhile it is probably best to assume "there is no bug" (okay there is always a bug, but likely not right here)

Nicky-D · 2024-04-03T16:55:27Z

indra/llcommon/lua_function.cpp

+        // thread, userdata) are not convertible to LLSD, indicating a coding
+        // error in the caller.
+        return lluau::error(L, "Cannot convert type %s to LLSD", luaL_typename(L, index));
+    }


A bit of refactoring (pulling case construct into their own functions) would help a great to deal make this code more reader friendly

Nicky-D · 2024-04-03T16:58:04Z

indra/llcommon/lua_function.cpp

+        // if we got a LuaListener instance, destroy it
+        auto lptr{ listener.get() };
+        listener.reset();
+        delete lptr;


This looks like some back magic, I suppose listener is a smart pointer rather than a doof-pointer. Why is either reset or assigning it a nullptr not enough to diospose it?

I guess it is magic: LLInstanceTracker magic. Arguably there should be an LLInstanceTracker::destroy() method or some such to encapsulate.

LLInstanceTracker stores a map of neutered std::shared_ptrs with no-op deleters. The point is that LLInstanceTracker is only to find subclass objects, not to manage their lifespan. That's what necessitates the logic in question.

Got it. How do I have to bribe you, to add a bit of documentation about this behavior into the code?

The best approach would be to add the aforementioned LLInstanceTracker::destroy() method, so that its internal comments explain its implementation. I'll do that.

Nicky-D · 2024-04-03T17:00:29Z

indra/llcommon/lua_function.cpp

+                // its internal error status. Any subsequent lua_pcall() calls
+                // with this lua_State will report error regardless of whether the
+                // chunk runs successfully. Get a new lua_State().
+                initLuaState();


I'd argue for some LL_WARNS here and down there (533)

Great minds! I added one here and down there with my most recent PR.

Nicky-D · 2024-04-03T17:02:37Z

indra/llcommon/lua_function.cpp

+    LuaListener::ptr_t listener;
+    // Does this lua_State already have a LuaListener stored in the registry?
+    auto keytype{ lua_getfield(L, LUA_REGISTRYINDEX, "event.listener") };
+    llassert(keytype == LUA_TNIL || keytype == LUA_TNUMBER);


It would be better to either use llassert_always or handle it gracefully. The latter being more enduser friendly

This is a "should never happen" condition: either a coder error, or the Lua registry got badly trashed. It's me being cautious about venturing into new turf.

Nicky-D · 2024-04-03T17:03:25Z

indra/llcommon/lua_function.cpp

+LuaFunction::LuaFunction(const std::string_view& name, lua_CFunction function,
+                         const std::string_view& helptext)
+{
+    const auto& [registry, lookup] = getState();


I see this syntax, it makes me happy

Nicky-D · 2024-04-03T17:05:08Z

indra/llcommon/lua_function.cpp

+}
+
+/*****************************************************************************
+*   lua_what


lua? WHAT?!?

Nicky-D · 2024-04-03T17:08:04Z

indra/llcommon/lua_function.h

+    // 'return error(...)'. Wrap luaL_error() in an int function.
+#if __clang__
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wformat-security"


clang isn't all wrong about this. Can a user controlled string enter here or will all of this be sanitized?

No, users would directly call Luau's error() function. This is for our internal use.

What I would like to know is what magic attribute to associate with the const char* format argument to (a) direct the compiler's static validation to callers of this error() function, and (b) pass through the statically-validated format to the underlying function.

If there is no "user" (user in this case could also be resources loaded from XML) supplied char const* (and I understood there is no such thing), then it is indeed strange that clang gets so lippy.

For some warnings compiler will try track arguments during the function calls and then only warn if they can determine the error. As this function is a template there is a good change clang will do it just like that (this would be visible in the way clang emits the warning).

A plausible explanation could then be attributed to the lua precompiled libs somehow in the call stack, then there can be no verification > error.

For b) then compiling lua inside the viewer source tree could fix it (not a super good solution).

Long story short: I think the pragma is the most pragmatic way then. (Except it will not help with GCC)

nat-goodspeed · 2024-08-16T15:41:21Z

For testing staging release:

build = https://github.com/secondlife/viewer/releases/tag/Second_Life_Test%23f8c70f4
cohort = Second Life Project Lua
desired = 10

#2483: Fix label typo in PBR terrain texture transforms tab

When asked to retrieve a slice starting at an `index > 0`, `getSliceStart()` was returning an LLSD array whose first `index` entries were `isUndefined()`, followed by the desired data. Fix to omit those undefined entries.

Fix visualizing luminance buffer and small cleanup

Change `result_view()` from a simple function to a callable table so we can add conventional/default functions to it: `result_view.fetch()` is a generic `fetch()` function suitable for use with `result_view()`, and `result_view.close()` is a variadic function that closes result sets for whichever keys are passed. This arises from the fact that any `LL::ResultSet` subclass is accessed generically through its base class, therefore we don't need distinct "getSlice" and "closeResult" operations for different `LLEventAPI` listeners. (It might make sense to relocate those operations to a new generic listener, but for now "LLInventory" works.) That lets `result_view()`'s caller omit the `fetch` parameter unless it requires special behavior. Omitting it uses the generic `result_view.fetch()` function. Moreover, every view returned by `result_view()` now contains a close() function that closes that view's result set. The table returned by LLInventory.lua's `result()` function has a `close()` method; that method can now call `result_view.close()` with the two keys of interest. That table's `__index()` metamethod can now leverage `result_view()`'s default `fetch` function.

This is the query that produced so many results that, before we lifted the infinite-loop interrupt limit, inspect(result) hit the limit and terminated.

…er in unpredictable manner

to put the biggest performance hits at the top of the list.

…ch frame (#2640)

…ctive

Now the location to which to teleport and the camera focus point can both be specified by the caller, in this case the frame_profile bash script.

Reverse the sort order for profile_cmp.py

Add ability to pass command-line arguments to a Lua script.

# Conflicts: # indra/newview/llfeaturemanager.cpp # indra/newview/llviewertexturelist.cpp # indra/newview/llvoicewebrtc.cpp

develop → Maint B sync

Maintenance B merges into develop

Merge develop into release/luau-scripting

nat-goodspeed · 2024-10-29T14:27:47Z

Since the long-running Lua project branch has been merged into develop, this PR has become moot.

nat-goodspeed added team:viewer release labels Feb 9, 2024

github-actions bot added c/cpp cmake llcommon llprimitive llui labels Feb 9, 2024

github-actions bot added python llfilesystem labels Feb 23, 2024

maxim-productengine added this to the Lua milestone Mar 26, 2024

Nicky-D reviewed Apr 3, 2024

View reviewed changes

github-actions bot added the llwindow label Apr 3, 2024

github-actions bot added llimage llmessage labels Apr 19, 2024

github-actions bot added the llxml label May 2, 2024

github-actions bot removed the llimage label May 15, 2024

github-actions bot added llmath and removed llprimitive labels Jun 4, 2024

github-actions bot added the llinventory label Aug 27, 2024

cosmic-linden and others added 9 commits September 3, 2024 13:56

Merge pull request #2484 from secondlife/v-2483

ceed015

#2483: Fix label typo in PBR terrain texture transforms tab

Fix a bug in ResultSet::getSliceStart().

517163c

When asked to retrieve a slice starting at an `index > 0`, `getSliceStart()` was returning an LLSD array whose first `index` entries were `isUndefined()`, followed by the desired data. Fix to omit those undefined entries.

Fix visualizing luminance buffer and small cleanup

dfae0d9

Merge pull request #2489 from RyeMutt/fix-buffervis

36423bd

Fix visualizing luminance buffer and small cleanup

Use Lua result-set logic for "LLFloaterReg"s "getFloaterNames" op.

a8dd713

This is the query that produced so many results that, before we lifted the infinite-loop interrupt limit, inspect(result) hit the limit and terminated.

#2490: Fix PBR terrain texture transform feature flag handled by view…

0f166a1

…er in unpredictable manner

viewer#2411 LLFontGL::render optimizations

4ae1de1

Remove usused LLButton::mBorderEnabled

2bae8df

nat-goodspeed and others added 25 commits September 20, 2024 16:34

Update test for changed message in error case.

5545505

Fix trailing spaces.

65379a3

Reverse the sort order for profile_cmp.py

4fc8f2e

to put the biggest performance hits at the top of the list.

Improve inventory finder floater performance: Don't parse XUI tree ea…

c0739e0

…ch frame (#2640)

#2618 Roles and Members tab is a mess (fix XML indents for readability)

fa4ea5b

#2618 Roles and Members tab is a mess (show ctrl pos in debug views)

0fc76aa

#2618 Roles and Members tab is a mess

06b6d5e

#2626 fix for incoming IM calls not being rejected when DND mode is a…

60947ee

…ctive

Use Lua command-line args to make frame_profile_quit.lua generic.

8efba1d

Now the location to which to teleport and the camera focus point can both be specified by the caller, in this case the frame_profile bash script.

Make frame_profile bash script find its workarea's viewer build.

dc5c10f

Merge remote branch 'lua-script-args' into lua-script-args

34e89de

Merge pull request #2636 from secondlife/profile-cmp-rev

db013ab

Reverse the sort order for profile_cmp.py

Merge remote branch 'develop'into release/luau-scripting

8bcff54

viewer#2631 Couple small tweaks

0f4b1b8

viewer#2631 Optimize LLWearable::writeToAvatar

3747dd9

Merge pull request #2635 from secondlife/lua-script-args

9289b96

Add ability to pass command-line arguments to a Lua script.

Merge remote branch 'develop' into release/luau-scripting

f6b3b5e

Merge branch 'develop' into marchcat/b-develop

46448f9

# Conflicts: # indra/newview/llfeaturemanager.cpp # indra/newview/llviewertexturelist.cpp # indra/newview/llvoicewebrtc.cpp

Turn off tracy on linux to unbreak the build.

0b7548c

Merge pull request #2657 from secondlife/marchcat/b-develop

38916eb

develop → Maint B sync

Merge pull request #940 from secondlife/release/maint-b

d8e52ec

Maintenance B merges into develop

Merge remote branch 'develop' into release/luau-scripting for Maint B

626920d

include <utility> for Linux: std::forward is not predefined

e290d14

Merge remote 'release/luau-scripting' into release/luau-scripting

74badfc

Merge pull request #2652 from secondlife/lua-merge-dev2

9036e45

Merge develop into release/luau-scripting

github-actions bot added llappearance llimagej2coj labels Sep 23, 2024

nat-goodspeed closed this Oct 29, 2024

github-actions bot locked and limited conversation to collaborators Oct 29, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Integrate Luau scripting into the viewer. #785

Integrate Luau scripting into the viewer. #785

nat-goodspeed commented Feb 9, 2024

nat-goodspeed commented Feb 27, 2024

Nicky-D Apr 3, 2024

Nicky-D Apr 3, 2024

nat-goodspeed Apr 3, 2024

Nicky-D Apr 3, 2024

nat-goodspeed Apr 3, 2024

Nicky-D Apr 3, 2024

nat-goodspeed Apr 3, 2024

Nicky-D Apr 3, 2024

Nicky-D Apr 3, 2024

Nicky-D Apr 3, 2024

nat-goodspeed Apr 3, 2024

Nicky-D Apr 3, 2024

nat-goodspeed Apr 3, 2024

nat-goodspeed Apr 3, 2024

Nicky-D Apr 3, 2024

nat-goodspeed Apr 3, 2024

Nicky-D Apr 3, 2024

nat-goodspeed Apr 3, 2024

Nicky-D Apr 3, 2024

Nicky-D Apr 3, 2024

Nicky-D Apr 3, 2024

nat-goodspeed Apr 3, 2024

Nicky-D Apr 3, 2024

nat-goodspeed commented Aug 16, 2024 •

edited

Loading

nat-goodspeed commented Oct 29, 2024

Integrate Luau scripting into the viewer. #785

Integrate Luau scripting into the viewer. #785

Conversation

nat-goodspeed commented Feb 9, 2024

nat-goodspeed commented Feb 27, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nat-goodspeed commented Aug 16, 2024 • edited Loading

nat-goodspeed commented Oct 29, 2024

nat-goodspeed commented Aug 16, 2024 •

edited

Loading