Repository navigation
Script Studio QA round five: function timerHandler, a find's place in SLua's count, blank lines kept, a brace-spacing setting, math.modf bare, converted SLua stable under format on save - #439
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Summary
Merge Risk: ⚪ Minimal · up to No concrete issue is identified that prevents merging after normal checks. Pre-merge checks |
  |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 24.69% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 81 functions across 8 files. (2 skipped: … | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
-
Check name Status Explanation Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Title check The title accurately summarizes the main Script Studio QA changes, including timer handling, SLua conversion fixes, blank-line preservation, brace spacing, and format-on-save stability. It is lengthy,… Description check The description provides detailed context, change-specific sections, testing results, known limitations, and implementation details. It does not reproduce the template headings, issue link, checklist …
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 24.69% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 81 functions across 8 files. (2 skipped: 2 unsupported.)
- Fix all pre-merge checks with AI
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
A rabbit checks each curly brace,
Then hops through lines in tidy space.
Comments find their proper place,
Old blank lines leave a measured trace,
And scripts emerge with clearer face.
Comment @coderabbitai help to get the list of available commands.
A script with one state, its timer on LLTimers, had its timer handler written `timerHandler = function() ... end`, assigning the local that the timers preamble declares above setTimer. `function timerHandler()` assigns the same local, since a function statement of a local's name makes no global, and is how Lua writes it. With several states the handler is still a field of the state's table. allsltoslua 68, typed too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…t is in The converter's setState left the state it was in, running its state_exit and taking its handlers off, and then iterated states[name] as it was. A name with no state stopped the script with "attempt to iterate over a nil value", inside setState, with no handlers left on. LSL's compiler refused such a name, so converted code never does it, but an edit afterwards may. The state entered is now looked up first, and a name with none stops the script with "unknown state: <name>", said at the call that named it, before anything is left. allsltoslua 4, typed too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
SLua's ll.SubStringIndex, and table.find for one item looked for in a list, count from 1 and answer nil for none. The converter makes that LSL's count, `(x or 0) - 1`, wherever the answer is used, and tests only whether anything was found against nil. A place compared with a number written out kept the count's arithmetic: llSubStringIndex(item, P) == 0 came out as (ll.SubStringIndex(item, P) or 0) - 1 == 0. Such a comparison is now made in SLua's count, the number moved on by one: ll.SubStringIndex(item, P) == 1, ~= 3, (x or 0) > 1 for an order, with the place on the left where the number was written first. It is the same test whatever the function answers. An == or ~= with a number below -1 is left as written, and a place read as a number keeps LSL's count. allsltoslua 86. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The converter kept a blank line written over a comment, but dropped one between two statements with no comment between them. So the author's grouping survived only where a comment marked it: a function's `if (...) return FALSE;`, a blank line, and `return TRUE;` came out with the two lines together. The same went for globals. A statement or global with one or more blank lines between it and the one written before it now has one blank line over it. A global after a function the LSL had between them counts as apart, since the function is written later. None is written at a block's start, nor a second where a comment over the statement keeps its own. A comment kept apart as a block's first line no longer leaves a blank line under the line that opens the block. allsltoslua 87. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The formatter put a space inside Luau's braces, as StyLua does, but
only where a name, number or string stood next to them. A brace, a
bracket or a sign did not get one, so the converter's own state table
type, `{ [string]: { [string]: (...any) -> () } }`, came out as
`{[string]: {[string]:(...any) -> () } }`, and `{{1}, {2}}` as
`{{ 1 }, { 2 } }`. A bracket after a colon lost its space too, a rule
for a method call that no Luau call has.
Inside a table's braces, or a table type's, the first part and the
closing brace are now spaced alike whatever they are, and a bracket
after a colon is spaced as written. Whether there is a space at all is
a setting, ALScriptFormatBraceSpaces, on by default: StyLua writes
{ 1, 2 }, and Luau's own documentation and SLua's tests write {1, 2}.
It is under Preferences > Editor > Formatting. {} and an interpolated
string's braces are never spaced, and LSL's braces are as they were.
alscriptformatter 12.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The converter wrote a list as {"\n"}, with no spaces, but a list's type
as { any } and a rule table as { auto_play = true }. Tapple's converted
line, `local params: { any } = ll.ParseStringKeepNulls(msg, {"\n"},
{})`, held both, and format on save then changed every list.
Tables and table types are now written by ALScriptFormatBraceSpaces,
the formatter's setting: { "\n" } and { any } by default, as StyLua and
the formatter write them, and {"\n"} and {any} with it off. A list over
several lines takes the spaces where an item shares a line with a
brace. An interpolated string's braces and {} are as they were.
Converted text's braces are then as format on save would leave them.
allsltoslua 88, and the expectations of sixteen others.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…aced
Three slips in the formatter's spacing, found formatting converted
SLua. A sign before a bracket or a brace was spaced from it: -(a) came
out as - (a), #{1, 2} as # { 1, 2 }, and LSL's !(b) as ! (b). A sign
after a < was always put against it, since LSL's < opens a vector
there, but a < that compares does not: Luau's i < -1 came out as
i <-1, i < #t as i <#t, and LSL's if (i < -1) as i <-1. And a sign
after a minus was put against it as if the minus were a sign too, so
x - #t came out as x -#t.
A sign standing as one, where nothing it could be an operator of comes
before it, is now written against the bracket or brace after it. After
a <, it is only where that < opened an LSL vector, and after a minus
only where the minus is a sign itself.
alscriptformatter 13.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…level A comment on a line of its own was indented as far as the block it stood in. One the author wrote over an `elseif`, `else`, `until`, `end` or `}`, at that line's level, was pushed into the block before it. The converter writes its notes on an elseif's or an until's condition that way, over the line they are about, so format on save moved them into the wrong block. Such a comment now keeps the level of the line under it where the author wrote it out of the block: no further in than that line, and less far than the code over it in the block. In a block with no code before it, the author's indentation decides, wherever the text is indented at all. A comment written as far in as the block, or in text with no indentation to tell, is the block's end and stays in it. alscriptformatter 15. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The formatter put one space around every `=`, so a run of declarations the author had lined up came apart, and comments lined up after them came apart with the code. The converter lines up again what the LSL lined up (QA3, 91b94b5), so format on save undid it in converted scripts too. A run is lines one after another at one depth whose `=`, its first outside brackets, or whose trailing comments the author put at one column, with more than one space before at least one of them to show it was meant. A comment on a line of its own at that depth does not break a run. Once each line is spaced afresh, the run's `=` or comments go to that column again, or just past the longest line where it no longer fits. A run of comments whose code did not change keeps its blanks as written. Columns are measured with tabs to their stops, so LSL lined up with tabs is kept, in spaces. alscriptformatter 14. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A comment at the end of the line an unbraced body ends was given to
the wrong statement. After an if's `then` body it went over the
`else`: `llDialog(...); // present submenu` came out on a line of its
own before `elseif`, at the if's level, or as the first line of the
else branch. After the last branch of an if, or a loop's body, it went
over the whole if or loop, since a comment after one of them would
trail its first line.
Such a comment now trails the statement that ends its line, found down
the if's last branch or the loop's body: `print("a") -- after a`. Where
that is an empty body, `if (c); // why`, it trails the line that opens
the body, `if c then -- why`. An if written on one line, with no else,
still takes it after its `end`.
allsltoslua 95.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Two things were still changed when converted SLua was formatted. The
text ended with a blank line, which the formatter drops. And the
lslInteger and lslFloat helpers wrote `{ s }` whatever the brace
setting asked, so with ALScriptFormatBraceSpaces off the formatter
changed them to `{s}`.
The text now ends with one line break, and the helpers' braces follow
the setting. Over Tailslide's 73 test scripts, formatting the
converted SLua now changes nothing, with either brace setting and with
types on or off, but where the width rule breaks a line longer than
the width.
allsltoslua 89, which converts a script with what used to change and
checks that formatting it changes nothing, either brace spacing.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nyway LSL's (integer) of a float is written as math.modf's whole part, always as (math.modf(x)). The brackets keep a call that answers two values to its first, which matters only where its values would spread: a call's last argument, a table's last item, a return. As an operand they did nothing, so `(integer)llFrand(1000) - 10000` came out as `(math.modf(ll.Frand(1000))) - 10000`. The call is now bare where only its first value is read anyway: an operator's operand, a compound assignment's value, and all of the value given a variable or a local, `x = math.modf(f) - 10000`. The brackets stay where the values would spread, including a vector's part assigned, which is made an argument of vector(). allsltoslua 96. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
RyeMutt
force-pushed
the
rye/script-qa5
branch
from
October 11, 2026 06:32
1dc2935 to
268b394
Compare
RyeMutt
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Tapple's fifth round of Script Studio QA, on one converted script.
function timerHandler().== 1).setStatesays when a state is not there.A later note from Tapple drops the brackets around
math.modfwhere they do nothing.Following the brace-spacing item through, converted SLua is now left unchanged by format on save. Over Tailslide's 73 test scripts, formatting the converted SLua changes nothing, with braces on or off and types on or off. The one exception is the 120-column width rule (see below).
Commits
The timer's handler is a function statement
timerHandler = function() ... end, assigning the local that the timers preamble declares abovesetTimer. It is nowfunction timerHandler(), which assigns the same local and makes no global. A state's table field keepstimer = function().setStatestops at a state that is not theresetStateleft the old state and then iteratedstates[name]. A name with no state stopped the script with "attempt to iterate over a nil value", with no handlers left on.error("unknown state: " .. name, 2), reported at thesetState("...")call, before anything is left.A find's place compared with a number, in SLua's count
llSubStringIndex(item, P) == 0came out as(ll.SubStringIndex(item, P) or 0) - 1 == 0. It is nowll.SubStringIndex(item, P) == 1.~=and orders, which keep(x or 0), such as(x or 0) > 1. It works with the number on either side, under anot, and fortable.find. A presence test is written against nil.==against a number below -1 is left as written, and a place read as a number keeps(x or 0) - 1.string.find. A test of where keepsll.SubStringIndex, sincestring.findcounts bytes, not characters.Blank lines between statements are kept
Spaces inside SLua's braces, as a setting
ALScriptFormatBraceSpaces, on by default, under Preferences > Editor > Formatting. The formatter spaces the inside of Luau's braces by it:{ 1, 2 }and{ any }as StyLua writes them, or{1, 2}as Luau's docs and SLua's tests do.{ [string]: (...any) -> () }came out as{[string]:(...any) -> () }, and{{1}}as{{ 1 } }. Both braces are now spaced alike, and a bracket after a colon is spaced as written.{"\n"}and list types as{ any }, two styles on one line. Its tables and table types (lists, rule tables,table.extend's, the states' type, the helpers) now follow the same setting.{}and an interpolated string's braces are never touched.The formatter keeps a sign against what it signs
-(a)came out as- (a),#{ 1 }as# { 1 }, and LSL's!(b)as! (b).<that compares was put against it:i < -1becamei <-1, in Luau and LSL alike. Only a<that opens an LSL vector does that now.x - #tbecamex -#t.The formatter keeps what the author lined up
=back to one space.=, or whose trailing comments, the author put at one column, padded wider than one space somewhere. A comment on a line of its own does not break a run.Comments over an
elseifkeep its levelelseif/else/until/end/}, at that line's level, was pushed into the block above. The converter writes its notes on anelseif's oruntil's condition that way.A comment after an unbraced body trails its statement
llDialog(...); // present submenuafter an unbracedthencame out on its own line beforeelseif, inside theelsebranch, or above the wholeifor loop.if c then -- why).Converted SLua ends with one line break
math.modfbare where one value is read anyway(integer)of a float was always(math.modf(x)). The brackets keep a two-value call to its first, which matters only where its values would spread.(integer)llFrand(1000) - 10000is nowmath.modf(ll.Frand(1000)) - 10000. The call is likewise bare as an operator's operand, a compound assignment's value, and the whole value given to a variable or a local.tostring((math.modf(f)))), list items,return, and a vector part assigned, which becomes an argument ofvector().math.random(0, 999)is not substituted for(integer)llFrand(1000). The converter never changes the source of randomness.Not done
Testing
developwith the test modernisation (Test framework, stage 1: every test ready to share a binary #436); no conflicts.allsltoslua86, 87, 88, 89, 95 and 96;alscriptformatter12, 13, 14 and 15.allsltoslua4 and 68 gained cases, and the expectations of sixteen others changed for the brace spacing.allsltoslua88 and 89 convert a script and check that formatting it changes nothing, with braces on and off.alfloaterscriptstudio.cpp,alfloaterscriptpreferences.cpp) compile. The full viewer was not built, and the XUI parses.{1, 2}throughout; with it on,{ 1, 2 }.🤖 Generated with Claude Code