Conversation
…y language ID or by supplying an inventory item as a template.
Rider-Linden
requested review from
HaroldCindy,
Copilot and
maxim-productengine
There was a problem hiding this comment.
Pull request overview
This PR updates the viewer’s script-creation flow so it can tell the simulator which default script template/language to use (LSL vs Lua) when creating a new script, removing the prior “create then immediately overwrite code” workaround.
Changes:
- Extend the
RezScriptmessage with an optionalNewScriptInfoblock containing script language and optional template item ID. - Update viewer script creation paths (task inventory + agent inventory) to pick Lua defaults when
LuaScriptsEnabledis advertised by simulator features. - Cherry-pick fix to ensure the login connect button is wired/defaulted earlier during
LLPanelLoginconstruction.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
scripts/messages/message_template.msg.sha1 |
Updates message template hash to reflect protocol change. |
scripts/messages/message_template.msg |
Adds NewScriptInfo block to RezScript message definition. |
indra/newview/llviewerobject.h |
Updates LLViewerObject::saveScript() signature to accept language/template parameters; switches header guard to #pragma once. |
indra/newview/llviewerobject.cpp |
Sends NewScriptInfo when creating a brand-new script (null asset UUID). |
indra/newview/llviewerinventory.h |
Introduces LANG_ID_LSL/LANG_ID_LUA constants and moves NO_INV_SUBTYPE to constexpr. |
indra/newview/llviewerinventory.cpp |
Removes Lua “post-create upload hack”; uses subtype to request Lua default script when supported. |
indra/newview/lltooldraganddrop.cpp |
Updates saveScript() call site to new signature. |
indra/newview/llpreviewscript.h |
Switches header guard to #pragma once. |
indra/newview/llpanellogin.cpp |
Moves connect button wiring/default-button setup earlier (cherry-picked fix). |
indra/newview/llpanelcontents.cpp |
Updates “New Script” (task inventory) creation to request correct language/template via saveScript(). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…e coroutine. This would trigger an attempt to get a mutex which will fail in a coro. Instead post the cleanup back to the mainthread.
| LLViewerAssetType::generateDescriptionFor(LLAssetType::AT_LSL_TEXT, desc); | ||
|
|
||
| // -------------------------------------------------------------------------------------------------- | ||
| // Begin hack |
|
|
||
| constexpr U8 NO_INV_SUBTYPE{ 0 }; | ||
| constexpr U8 LANG_ID_LSL = 0; | ||
| constexpr U8 LANG_ID_LUA = 1; |
There was a problem hiding this comment.
If scripts are created as one type, does that constrain the types of compile options you can use for the script once it's created? Is that something the viewer needs to be aware of?
There was a problem hiding this comment.
No. It does not.
There was a problem hiding this comment.
I fixed subtype on the server side so that it is changed to match the last successful compile. I still need the metadata to identify which VM but this should provide a good default.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| msg->nextBlock("NewScriptInfo"); | ||
| msg->addUUID("TemplateID", template_id); |
There was a problem hiding this comment.
NewScriptInfo/TemplateID are sent using string-based nextBlock()/addUUID(), while the rest of this message uses prehashed *Fast() calls. Once the template change is in place, prefer using the corresponding _PREHASH_* identifiers with nextBlockFast()/addUUIDFast() to avoid typos and keep message construction consistent/perf-friendly.
| msg->nextBlock("NewScriptInfo"); | |
| msg->addUUID("TemplateID", template_id); | |
| msg->nextBlockFast(_PREHASH_NewScriptInfo); | |
| msg->addUUIDFast(_PREHASH_TemplateID, template_id); |
| desc, | ||
| LLSaleInfo::DEFAULT, | ||
| LLInventoryItemFlags::II_FLAGS_NONE, | ||
| LLInventoryItemFlags::II_FLAGS_SUBTYPE_MASK & script_language, |
There was a problem hiding this comment.
The flags parameter here is intended to store the script subtype in the low byte; using LLInventoryItemFlags::II_FLAGS_SUBTYPE_MASK & script_language works but is a bit non-idiomatic/readability-hostile (it reads like you’re masking an existing flags value). Consider passing static_cast<U32>(script_language) (or script_language & II_FLAGS_SUBTYPE_MASK) to make the intent explicit.
| LLInventoryItemFlags::II_FLAGS_SUBTYPE_MASK & script_language, | |
| static_cast<U32>(script_language), |
Description
The viewer may now indicate to the server which default script to use when creating a new script in either task or agent inventory.
Optionally the viewer may also provide the ID of a script in the agent's inventory, this script will be used instead of the default script.
Future Work
Default script choice is determined by what the server supports. If it supports SLua it will use the lua default, otherwise it will select the LSL default.
We should provide some sort of user option to select which default to use. There is also no interface at this time for selecting an alternate default template from inventory, this would be useful to provide as well.
Related Issues
Simulator: https://github.com/secondlife/server/pull/2344
Message Template: secondlife/master-message-template#4
Additional Notes
This also cherry picks the
updateLoginButtonsfix.