hakowork/rojo
3
0
Fork 0
mirror of https://github.com/rojo-rbx/rojo.git synced 2026-10-23 05:51:39 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

Enhance script handling and UI consistency

Browse source 
- Added support for encoding the RunContext property in the Script class to determine file suffixes (.server.luau, .client.luau, .plugin.luau, .legacy.luau).
- Updated the Confirming.lua file to improve UI consistency in the confirmation message display.
- Modified snapshot middleware and syncback logic to accommodate new file extensions and ensure proper handling of plugin scripts.
- Refactored tests to align with the new naming conventions for script suffixes and improved path uniqueness checks.
This commit is contained in:
Joe 2026-01-30 10:31:36 -06:00
parent 38cbc582f4
commit 18a74c8e4d
7 changed files with 95 additions and 87 deletions
Download patch file Download diff file Expand all files Collapse all files

24
plugin/src/App/StatusPages/Confirming.lua
View file

@ -327,18 +327,18 @@ function ConfirmingPage:render()
self:setState({ showingAcceptConfirm = false }) self:setState({ showingAcceptConfirm = false })
end, end,
}, { }, {
Content = e("Frame", { Content = e("Frame", {
Size = UDim2.fromScale(1, 1), Size = UDim2.fromScale(1, 1),
BackgroundColor3 = theme.BackgroundColor, BackgroundColor3 = theme.BackgroundColor,
BorderSizePixel = 0, BorderSizePixel = 0,
}, { }, {
Message = e("TextLabel", { Message = e("TextLabel", {
Text = string.format( Text = string.format(
"%d item%s not been reviewed.\n\nAccept selected changes and skip the rest?", "%d item%s not been reviewed.\n\nAccept selected changes and skip the rest?",
self.state.unselectedCount, self.state.unselectedCount,
self.state.unselectedCount == 1 and " has" or "s have" self.state.unselectedCount == 1 and " has" or "s have"
), ),
FontFace = theme.Font.Main, FontFace = theme.Font.Main,
TextSize = theme.TextSize.Body, TextSize = theme.TextSize.Body,
TextColor3 = theme.TextColor, TextColor3 = theme.TextColor,
TextWrapped = true, TextWrapped = true,

12
plugin/src/ChangeBatcher/encodeInstance.lua
View file

@ -161,6 +161,18 @@ encodeInstance = function(instance, parentId, _skipPathCheck)
Log.warn("Could not find Source property descriptor for {:?}", instance) Log.warn("Could not find Source property descriptor for {:?}", instance)
return nil return nil
end end
-- For Script class specifically, also encode RunContext property
-- This determines the file suffix (.server.luau, .client.luau, .plugin.luau, .legacy.luau)
if instance.ClassName == "Script" then
local runContextDescriptor = RbxDom.findCanonicalPropertyDescriptor("Script", "RunContext")
if runContextDescriptor then
local encodeSuccess, encodeResult = encodeProperty(instance, "RunContext", runContextDescriptor)
if encodeSuccess and encodeResult ~= nil then
properties.RunContext = encodeResult
end
end
end
elseif not DIRECTORY_CLASSES[instance.ClassName] then elseif not DIRECTORY_CLASSES[instance.ClassName] then
-- For non-directory, non-script classes, encode all relevant properties -- For non-directory, non-script classes, encode all relevant properties
local classDescriptor = RbxDom.findClassDescriptor(instance.ClassName) local classDescriptor = RbxDom.findClassDescriptor(instance.ClassName)

10
src/snapshot_middleware/mod.rs
View file

@ -96,7 +96,7 @@ pub fn snapshot_from_vfs(
match file_name { match file_name {
// Modern extensions // Modern extensions
"init.server.luau" | "init.client.luau" | "init.local.luau" | "init.legacy.luau" "init.server.luau" | "init.client.luau" | "init.local.luau" | "init.legacy.luau"
| "init.luau" | "init.csv" | | "init.plugin.luau" | "init.luau" | "init.csv" |
// Legacy extensions (for backwards compatibility) // Legacy extensions (for backwards compatibility)
"init.server.lua" | "init.client.lua" | "init.lua" => return Ok(None), "init.server.lua" | "init.client.lua" | "init.lua" => return Ok(None),
_ => {} _ => {}
@ -129,6 +129,7 @@ fn get_dir_middleware<'path>(
(Middleware::ModuleScriptDir, "init.luau"), (Middleware::ModuleScriptDir, "init.luau"),
(Middleware::ServerScriptDir, "init.server.luau"), (Middleware::ServerScriptDir, "init.server.luau"),
(Middleware::ClientScriptDir, "init.client.luau"), (Middleware::ClientScriptDir, "init.client.luau"),
(Middleware::PluginScriptDir, "init.plugin.luau"),
(Middleware::LocalScriptDir, "init.local.luau"), (Middleware::LocalScriptDir, "init.local.luau"),
(Middleware::LegacyScriptDir, "init.legacy.luau"), (Middleware::LegacyScriptDir, "init.legacy.luau"),
(Middleware::CsvDir, "init.csv"), (Middleware::CsvDir, "init.csv"),
@ -222,6 +223,8 @@ pub enum Middleware {
#[serde(skip_deserializing)] #[serde(skip_deserializing)]
ModuleScriptDir, ModuleScriptDir,
#[serde(skip_deserializing)] #[serde(skip_deserializing)]
PluginScriptDir,
#[serde(skip_deserializing)]
LocalScriptDir, LocalScriptDir,
#[serde(skip_deserializing)] #[serde(skip_deserializing)]
LegacyScriptDir, LegacyScriptDir,
@ -267,6 +270,9 @@ impl Middleware {
Self::ModuleScriptDir => { Self::ModuleScriptDir => {
snapshot_lua_init(context, vfs, path, name, ScriptType::Module) snapshot_lua_init(context, vfs, path, name, ScriptType::Module)
} }
Self::PluginScriptDir => {
snapshot_lua_init(context, vfs, path, name, ScriptType::Plugin)
}
Self::LocalScriptDir => snapshot_lua_init(context, vfs, path, name, ScriptType::Local), Self::LocalScriptDir => snapshot_lua_init(context, vfs, path, name, ScriptType::Local),
Self::LegacyScriptDir => { Self::LegacyScriptDir => {
snapshot_lua_init(context, vfs, path, name, ScriptType::Legacy) snapshot_lua_init(context, vfs, path, name, ScriptType::Legacy)
@ -316,6 +322,7 @@ impl Middleware {
Middleware::ModuleScriptDir => syncback_lua_init(ScriptType::Module, snapshot), Middleware::ModuleScriptDir => syncback_lua_init(ScriptType::Module, snapshot),
Middleware::LocalScriptDir => syncback_lua_init(ScriptType::Local, snapshot), Middleware::LocalScriptDir => syncback_lua_init(ScriptType::Local, snapshot),
Middleware::LegacyScriptDir => syncback_lua_init(ScriptType::Legacy, snapshot), Middleware::LegacyScriptDir => syncback_lua_init(ScriptType::Legacy, snapshot),
Middleware::PluginScriptDir => syncback_lua_init(ScriptType::Plugin, snapshot),
Middleware::CsvDir => syncback_csv_init(snapshot), Middleware::CsvDir => syncback_csv_init(snapshot),
} }
} }
@ -329,6 +336,7 @@ impl Middleware {
| Middleware::ServerScriptDir | Middleware::ServerScriptDir
| Middleware::ClientScriptDir | Middleware::ClientScriptDir
| Middleware::ModuleScriptDir | Middleware::ModuleScriptDir
| Middleware::PluginScriptDir
| Middleware::LocalScriptDir | Middleware::LocalScriptDir
| Middleware::LegacyScriptDir | Middleware::LegacyScriptDir
| Middleware::CsvDir | Middleware::CsvDir

2
src/syncback/file_names.rs
View file

@ -39,6 +39,7 @@ pub fn name_for_inst<'old>(
| Middleware::ServerScriptDir | Middleware::ServerScriptDir
| Middleware::ClientScriptDir | Middleware::ClientScriptDir
| Middleware::ModuleScriptDir | Middleware::ModuleScriptDir
| Middleware::PluginScriptDir
| Middleware::LocalScriptDir | Middleware::LocalScriptDir
| Middleware::LegacyScriptDir => { | Middleware::LegacyScriptDir => {
let name = if encode_invalid_chars { let name = if encode_invalid_chars {
@ -110,6 +111,7 @@ pub fn extension_for_middleware(middleware: Middleware) -> &'static str {
| Middleware::ServerScriptDir | Middleware::ServerScriptDir
| Middleware::ClientScriptDir | Middleware::ClientScriptDir
| Middleware::ModuleScriptDir | Middleware::ModuleScriptDir
| Middleware::PluginScriptDir
| Middleware::LocalScriptDir | Middleware::LocalScriptDir
| Middleware::LegacyScriptDir => { | Middleware::LegacyScriptDir => {
unimplemented!("directory middleware requires special treatment") unimplemented!("directory middleware requires special treatment")

2
src/syncback/mod.rs
View file

@ -383,7 +383,7 @@ pub fn get_best_middleware(snapshot: &SyncbackSnapshot) -> Middleware {
Middleware::ServerScript => Middleware::ServerScriptDir, Middleware::ServerScript => Middleware::ServerScriptDir,
Middleware::ClientScript => Middleware::ClientScriptDir, Middleware::ClientScript => Middleware::ClientScriptDir,
Middleware::ModuleScript => Middleware::ModuleScriptDir, Middleware::ModuleScript => Middleware::ModuleScriptDir,
Middleware::PluginScript => Middleware::ServerScriptDir, // Plugin scripts with children use server script dir for now Middleware::PluginScript => Middleware::PluginScriptDir,
Middleware::LegacyScript => Middleware::LegacyScriptDir, Middleware::LegacyScript => Middleware::LegacyScriptDir,
Middleware::LocalScript => Middleware::LocalScriptDir, Middleware::LocalScript => Middleware::LocalScriptDir,
Middleware::Csv => Middleware::CsvDir, Middleware::Csv => Middleware::CsvDir,

11
src/syncback/ref_properties.rs
View file

@ -71,7 +71,10 @@ fn compute_refs_with_duplicate_siblings(dom: &WeakDom) -> HashSet<Ref> {
let mut name_to_refs: HashMap<&str, Vec<Ref>> = HashMap::new(); let mut name_to_refs: HashMap<&str, Vec<Ref>> = HashMap::new();
for child_ref in inst.children() { for child_ref in inst.children() {
if let Some(child) = dom.get_by_ref(*child_ref) { if let Some(child) = dom.get_by_ref(*child_ref) {
name_to_refs.entry(&child.name).or_default().push(*child_ref); name_to_refs
.entry(&child.name)
.or_default()
.push(*child_ref);
} }
queue.push_back(*child_ref); queue.push_back(*child_ref);
} }
@ -157,9 +160,9 @@ pub fn collect_referents(dom: &WeakDom, pre_prune_paths: &HashMap<Ref, String>)
// Check if target exists in current DOM // Check if target exists in current DOM
if let Some(_target) = dom.get_by_ref(*target_ref) { if let Some(_target) = dom.get_by_ref(*target_ref) {
// Target exists - check if path is unique (with caching) // Target exists - check if path is unique (with caching)
let is_unique = *path_unique_cache let is_unique = *path_unique_cache.entry(*target_ref).or_insert_with(|| {
.entry(*target_ref) is_path_unique_with_cache(dom, *target_ref, &has_duplicate_siblings)
.or_insert_with(|| is_path_unique_with_cache(dom, *target_ref, &has_duplicate_siblings)); });
if is_unique { if is_unique {
// Path is unique - use path-based system // Path is unique - use path-based system

121
src/web/api.rs
View file

@ -273,9 +273,7 @@ impl ApiService {
/// ///
/// This is O(N) where N is the number of instances, and allows subsequent /// This is O(N) where N is the number of instances, and allows subsequent
/// path uniqueness checks to be O(d) instead of O(d × s) where d=depth, s=siblings. /// path uniqueness checks to be O(d) instead of O(d × s) where d=depth, s=siblings.
fn compute_tree_refs_with_duplicate_siblings( fn compute_tree_refs_with_duplicate_siblings(tree: &crate::snapshot::RojoTree) -> HashSet<Ref> {
tree: &crate::snapshot::RojoTree,
) -> HashSet<Ref> {
use std::collections::VecDeque; use std::collections::VecDeque;
let mut has_duplicate_siblings = HashSet::new(); let mut has_duplicate_siblings = HashSet::new();
@ -292,7 +290,10 @@ impl ApiService {
let mut name_to_refs: HashMap<&str, Vec<Ref>> = HashMap::new(); let mut name_to_refs: HashMap<&str, Vec<Ref>> = HashMap::new();
for child_ref in inst.children() { for child_ref in inst.children() {
if let Some(child) = tree.get_instance(*child_ref) { if let Some(child) = tree.get_instance(*child_ref) {
name_to_refs.entry(child.name()).or_default().push(*child_ref); name_to_refs
.entry(child.name())
.or_default()
.push(*child_ref);
} }
queue.push_back(*child_ref); queue.push_back(*child_ref);
} }
@ -2295,7 +2296,7 @@ mod tests {
.and_then(|e| e.items.get(name).copied()) .and_then(|e| e.items.get(name).copied())
} }
/// Simulates suffix determination logic /// Simulates suffix determination logic (matches get_script_suffix_for_run_context)
fn determine_suffix_from_run_context(run_context_value: Option<u32>) -> &'static str { fn determine_suffix_from_run_context(run_context_value: Option<u32>) -> &'static str {
let run_context_enums = rbx_reflection_database::get() let run_context_enums = rbx_reflection_database::get()
.ok() .ok()
@ -2308,14 +2309,14 @@ mod tests {
return match name.as_ref() { return match name.as_ref() {
"Client" => "client", "Client" => "client",
"Server" => "server", "Server" => "server",
"Legacy" => "server", "Legacy" => "legacy",
"Plugin" => "plugin", "Plugin" => "plugin",
_ => "server", _ => "legacy",
}; };
} }
} }
} }
"server" "legacy"
} }
#[test] #[test]
@ -2354,13 +2355,13 @@ mod tests {
} }
#[test] #[test]
fn test_run_context_legacy_produces_server_suffix() { fn test_run_context_legacy_produces_legacy_suffix() {
let value = get_run_context_value("Legacy"); let value = get_run_context_value("Legacy");
assert!(value.is_some(), "Legacy RunContext should exist"); assert!(value.is_some(), "Legacy RunContext should exist");
let suffix = determine_suffix_from_run_context(value); let suffix = determine_suffix_from_run_context(value);
assert_eq!( assert_eq!(
suffix, "server", suffix, "legacy",
"RunContext::Legacy should produce 'server' suffix" "RunContext::Legacy should produce 'legacy' suffix"
); );
} }
@ -2376,21 +2377,21 @@ mod tests {
} }
#[test] #[test]
fn test_no_run_context_defaults_to_server() { fn test_no_run_context_defaults_to_legacy() {
let suffix = determine_suffix_from_run_context(None); let suffix = determine_suffix_from_run_context(None);
assert_eq!( assert_eq!(
suffix, "server", suffix, "legacy",
"No RunContext should default to 'server' suffix" "No RunContext should default to 'legacy' suffix"
); );
} }
#[test] #[test]
fn test_invalid_run_context_defaults_to_server() { fn test_invalid_run_context_defaults_to_legacy() {
// Some unlikely value that doesn't match any known RunContext // Some unlikely value that doesn't match any known RunContext
let suffix = determine_suffix_from_run_context(Some(99999)); let suffix = determine_suffix_from_run_context(Some(99999));
assert_eq!( assert_eq!(
suffix, "server", suffix, "legacy",
"Invalid RunContext should default to 'server' suffix" "Invalid RunContext should default to 'legacy' suffix"
); );
} }
@ -2399,18 +2400,18 @@ mod tests {
// Verify expected file extension patterns // Verify expected file extension patterns
let test_cases = vec![ let test_cases = vec![
("ModuleScript", None, ".luau"), ("ModuleScript", None, ".luau"),
("LocalScript", None, ".client.luau"), ("LocalScript", None, ".local.luau"),
("Script", get_run_context_value("Server"), ".server.luau"), ("Script", get_run_context_value("Server"), ".server.luau"),
("Script", get_run_context_value("Client"), ".client.luau"), ("Script", get_run_context_value("Client"), ".client.luau"),
("Script", get_run_context_value("Legacy"), ".server.luau"), ("Script", get_run_context_value("Legacy"), ".legacy.luau"),
("Script", get_run_context_value("Plugin"), ".plugin.luau"), ("Script", get_run_context_value("Plugin"), ".plugin.luau"),
("Script", None, ".server.luau"), ("Script", None, ".legacy.luau"),
]; ];
for (class_name, run_context, expected_ext) in test_cases { for (class_name, run_context, expected_ext) in test_cases {
let suffix = match class_name { let suffix = match class_name {
"ModuleScript" => "", "ModuleScript" => "",
"LocalScript" => "client", "LocalScript" => "local",
"Script" => determine_suffix_from_run_context(run_context), "Script" => determine_suffix_from_run_context(run_context),
_ => panic!("Unknown class"), _ => panic!("Unknown class"),
}; };
@ -2434,7 +2435,7 @@ mod tests {
// Verify init file patterns for scripts with children // Verify init file patterns for scripts with children
let test_cases = vec![ let test_cases = vec![
("ModuleScript", None, "init.luau"), ("ModuleScript", None, "init.luau"),
("LocalScript", None, "init.client.luau"), ("LocalScript", None, "init.local.luau"),
( (
"Script", "Script",
get_run_context_value("Server"), get_run_context_value("Server"),
@ -2448,7 +2449,7 @@ mod tests {
( (
"Script", "Script",
get_run_context_value("Legacy"), get_run_context_value("Legacy"),
"init.server.luau", "init.legacy.luau",
), ),
( (
"Script", "Script",
@ -2460,7 +2461,7 @@ mod tests {
for (class_name, run_context, expected_init) in test_cases { for (class_name, run_context, expected_init) in test_cases {
let suffix = match class_name { let suffix = match class_name {
"ModuleScript" => "", "ModuleScript" => "",
"LocalScript" => "client", "LocalScript" => "local",
"Script" => determine_suffix_from_run_context(run_context), "Script" => determine_suffix_from_run_context(run_context),
_ => panic!("Unknown class"), _ => panic!("Unknown class"),
}; };
@ -3088,62 +3089,44 @@ mod tests {
} }
#[test] #[test]
fn test_emit_legacy_scripts_compatibility() { fn test_new_script_naming_convention() {
// Test that our syncback is compatible with emitLegacyScripts mode // Test that syncback produces the correct file extensions with the new naming convention
// //
// emitLegacyScripts: true // New naming convention (emitLegacyScripts removed):
// - .client.luau -> LocalScript // - LocalScript -> .local.luau
// - .server.luau -> Script with RunContext::Legacy // - Script + RunContext::Server -> .server.luau
// - Script + RunContext::Client -> .client.luau
// - Script + RunContext::Plugin -> .plugin.luau
// - Script + RunContext::Legacy -> .legacy.luau
// - Script (no RunContext) -> .legacy.luau (default)
// //
// emitLegacyScripts: false // Backwards compatibility for READING old files:
// - .client.luau -> Script with RunContext::Client // - .client.lua -> LocalScript
// - .server.luau -> Script with RunContext::Server // - .server.lua -> Script with RunContext::Legacy
// Our syncback produces: // Test expected syncback extensions
// - LocalScript -> .client.luau (compatible with legacy: true) let test_cases = vec![
// - Script + RunContext::Client -> .client.luau (compatible with legacy: false) ("LocalScript", None, ".local.luau"),
// - Script + RunContext::Server -> .server.luau (compatible with legacy: false) ("Script", Some("Server"), ".server.luau"),
// - Script + RunContext::Legacy -> .server.luau (compatible with legacy: true) ("Script", Some("Client"), ".client.luau"),
("Script", Some("Plugin"), ".plugin.luau"),
// This test verifies the design is correct ("Script", Some("Legacy"), ".legacy.luau"),
let legacy_mode_read = vec![ ("Script", None, ".legacy.luau"),
(".client.luau", "LocalScript", None),
(".server.luau", "Script", Some("Legacy")),
]; ];
let non_legacy_mode_read = vec![ for (class, run_context, expected_ext) in &test_cases {
(".client.luau", "Script", Some("Client")),
(".server.luau", "Script", Some("Server")),
];
// Verify legacy mode round-trips correctly
for (extension, class, run_context) in &legacy_mode_read {
// If we syncback this class with this RunContext, what extension do we get?
let syncback_ext = match (*class, *run_context) { let syncback_ext = match (*class, *run_context) {
("LocalScript", _) => ".client.luau", ("LocalScript", _) => ".local.luau",
("Script", Some("Legacy")) => ".server.luau",
("Script", Some("Server")) => ".server.luau", ("Script", Some("Server")) => ".server.luau",
("Script", Some("Client")) => ".client.luau", ("Script", Some("Client")) => ".client.luau",
("Script", None) => ".server.luau", ("Script", Some("Plugin")) => ".plugin.luau",
("Script", Some("Legacy")) => ".legacy.luau",
("Script", None) => ".legacy.luau",
_ => panic!("Unexpected combo"), _ => panic!("Unexpected combo"),
}; };
assert_eq!( assert_eq!(
syncback_ext, *extension, syncback_ext, *expected_ext,
"Legacy mode round-trip failed for {} {:?}", "New naming convention failed for {} {:?}",
class, run_context
);
}
// Verify non-legacy mode round-trips correctly
for (extension, class, run_context) in &non_legacy_mode_read {
let syncback_ext = match (*class, *run_context) {
("Script", Some("Client")) => ".client.luau",
("Script", Some("Server")) => ".server.luau",
_ => panic!("Unexpected combo"),
};
assert_eq!(
syncback_ext, *extension,
"Non-legacy mode round-trip failed for {} {:?}",
class, run_context class, run_context
); );
} }