refactor: change scriptType to be emitLegacyScripts

rojo-rbx · Sep 14, 2023 · 69f6be1 · 69f6be1
1 parent 35619f3
commit 69f6be1
Show file tree

Hide file tree

Showing 44 changed files with 104 additions and 96 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -25,10 +25,15 @@
 * Added rich Source diffs in patch visualizer ([#748])
 * Fix PatchTree performance issues ([#755])
 * Don't override the initial enabled state for source diffing ([#760])
-* A `$scriptType` field has been added to the project.json schema, allowing for scripts to be differentiated through classes or RunContext ([#765]).
 * Added support for `Terrain.MaterialColors` ([#770])
 * Allow `Terrain` to be specified without a classname ([#771])
 * Add Confirmation Behavior setting ([#774])
+* Added the `emitLegacyScripts` field to the project format ([#765]). Right now, it defaults to `true`:
+
+| `$emitLegacyScripts` Value | Action Taken by Rojo                                                                                             |
+|----------------------------|------------------------------------------------------------------------------------------------------------------|
+| false                      | Rojo emits Scripts with the appropriate `RunContext` for `*.client.lua` and `*.server.lua` files in the project. |
+| true   (current default)   | Rojo emits LocalScripts and Scripts with legacy Runcontext (same behavior as previously).                        |
 
 [#761]: https://github.com/rojo-rbx/rojo/pull/761
 [#745]: https://github.com/rojo-rbx/rojo/pull/745

diff --git a/rojo-test/build-tests/nested_runcontext/default.project.json b/rojo-test/build-tests/nested_runcontext/default.project.json
@@ -1,6 +1,6 @@
 {
   "name": "nested_runcontext",
-  "scriptType": "RunContext",
+  "emitLegacyScripts": false,
   "tree": {
     "$className": "Folder",
     "folder1": {

diff --git a/rojo-test/build-tests/nested_runcontext/folder2/default.project.json b/rojo-test/build-tests/nested_runcontext/folder2/default.project.json
@@ -1,6 +1,6 @@
 {
   "name": "nested_runcontext",
-  "scriptType": "Class",
+  "emitLegacyScripts": true,
   "tree": {
     "$path": "folder3"
   }

diff --git a/src/project.rs b/src/project.rs
@@ -8,7 +8,9 @@ use std::{
 use serde::{Deserialize, Serialize};
 use thiserror::Error;
 
-use crate::{glob::Glob, resolution::UnresolvedValue, snapshot_middleware::ScriptContextType};
+use crate::{
+    glob::Glob, resolution::UnresolvedValue, snapshot_middleware::emit_legacy_scripts_default,
+};
 
 static PROJECT_FILENAME: &str = "default.project.json";
 
@@ -76,8 +78,11 @@ pub struct Project {
     /// The mode to use when mapping scripts into Roblox.
     /// Can be either `Class` or `RunContext` and determines whether script
     /// behavior is set using the `RunContext` property or the script's `ClassName`.
-    #[serde(default, skip_serializing_if = "Option::is_none")]
-    pub script_type: Option<ScriptContextType>,
+    #[serde(
+        default = "emit_legacy_scripts_default",
+        skip_serializing_if = "Option::is_none"
+    )]
+    pub emit_legacy_scripts: Option<bool>,
 
     /// A list of globs, relative to the folder the project file is in, that
     /// match files that should be excluded if Rojo encounters them.

diff --git a/src/serve_session.rs b/src/serve_session.rs
@@ -123,7 +123,7 @@ impl ServeSession {
 
         let root_id = tree.get_root_id();
 
-        let instance_context = InstanceContext::from(root_project.script_type);
+        let instance_context = InstanceContext::from(root_project.emit_legacy_scripts);
 
         log::trace!("Generating snapshot of instances from VFS");
         let snapshot = snapshot_from_vfs(&instance_context, &vfs, start_path)?;

diff --git a/src/snapshot/metadata.rs b/src/snapshot/metadata.rs
@@ -7,7 +7,8 @@ use std::{
 use serde::{Deserialize, Serialize};
 
 use crate::{
-    glob::Glob, path_serializer, project::ProjectNode, snapshot_middleware::ScriptContextType,
+    glob::Glob, path_serializer, project::ProjectNode,
+    snapshot_middleware::emit_legacy_scripts_default,
 };
 
 /// Rojo-specific metadata that can be associated with an instance or a snapshot
@@ -105,14 +106,14 @@ impl Default for InstanceMetadata {
 pub struct InstanceContext {
     #[serde(skip_serializing_if = "Vec::is_empty")]
     pub path_ignore_rules: Arc<Vec<PathIgnoreRule>>,
-    pub script_type: ScriptContextType,
+    pub emit_legacy_scripts: bool,
 }
 
 impl InstanceContext {
     pub fn new() -> Self {
         Self {
             path_ignore_rules: Arc::new(Vec::new()),
-            script_type: ScriptContextType::Class,
+            emit_legacy_scripts: emit_legacy_scripts_default().unwrap(),
         }
     }
 
@@ -134,26 +135,27 @@ impl InstanceContext {
         rules.extend(new_rules);
     }
 
-    pub fn set_script_type(&mut self, script_type: ScriptContextType) {
-        self.script_type = script_type
+    pub fn set_emit_legacy_scripts(&mut self, emit_legacy_scripts: bool) {
+        self.emit_legacy_scripts = emit_legacy_scripts
     }
 }
 
 // serve_session always passes an option from the config file, but tests want it to be explict
 #[cfg(test)]
-impl From<ScriptContextType> for InstanceContext {
-    fn from(script_type: ScriptContextType) -> Self {
+impl From<bool> for InstanceContext {
+    fn from(emit_legacy_scripts: bool) -> Self {
         Self {
-            script_type,
+            emit_legacy_scripts,
             ..Self::new()
         }
     }
 }
 
-impl From<Option<ScriptContextType>> for InstanceContext {
-    fn from(script_type: Option<ScriptContextType>) -> Self {
+impl From<Option<bool>> for InstanceContext {
+    fn from(emit_legacy_scripts: Option<bool>) -> Self {
         Self {
-            script_type: script_type.unwrap_or_default(),
+            emit_legacy_scripts: emit_legacy_scripts
+                .unwrap_or_else(|| emit_legacy_scripts_default().unwrap()),
             ..Self::new()
         }
     }

diff --git a/src/snapshot/tests/snapshots/librojo__snapshot__tests__apply__add_property.snap b/src/snapshot/tests/snapshots/librojo__snapshot__tests__apply__add_property.snap
@@ -1,7 +1,6 @@
 ---
 source: src/snapshot/tests/apply.rs
 expression: tree_view
-
 ---
 id: id-1
 name: ROOT
@@ -13,6 +12,6 @@ metadata:
   ignore_unknown_instances: false
   relevant_paths: []
   context:
-    script_type: Class
+    emit_legacy_scripts: true
 children: []
 
diff --git a/...napshot/tests/snapshots/librojo__snapshot__tests__apply__remove_property_after_patch.snap b/...napshot/tests/snapshots/librojo__snapshot__tests__apply__remove_property_after_patch.snap
@@ -10,5 +10,6 @@ metadata:
   ignore_unknown_instances: false
   relevant_paths: []
   context:
-    script_type: Class
+    emit_legacy_scripts: true
 children: []
+
diff --git a/src/snapshot/tests/snapshots/librojo__snapshot__tests__apply__remove_property_initial.snap b/src/snapshot/tests/snapshots/librojo__snapshot__tests__apply__remove_property_initial.snap
@@ -1,7 +1,6 @@
 ---
 source: src/snapshot/tests/apply.rs
 expression: tree_view
-
 ---
 id: id-1
 name: ROOT
@@ -13,6 +12,6 @@ metadata:
   ignore_unknown_instances: false
   relevant_paths: []
   context:
-    script_type: Class
+    emit_legacy_scripts: true
 children: []
 
diff --git a/src/snapshot/tests/snapshots/librojo__snapshot__tests__apply__set_name_and_class_name.snap b/src/snapshot/tests/snapshots/librojo__snapshot__tests__apply__set_name_and_class_name.snap
@@ -10,5 +10,6 @@ metadata:
   ignore_unknown_instances: false
   relevant_paths: []
   context:
-    script_type: Class
+    emit_legacy_scripts: true
 children: []
+
diff --git a/src/snapshot/tests/snapshots/librojo__snapshot__tests__compute__add_child.snap b/src/snapshot/tests/snapshots/librojo__snapshot__tests__compute__add_child.snap
@@ -11,7 +11,7 @@ added_instances:
         ignore_unknown_instances: false
         relevant_paths: []
         context:
-          script_type: Class
+          emit_legacy_scripts: true
       name: New
       class_name: Folder
       properties: {}

diff --git a/src/snapshot_middleware/lua.rs b/src/snapshot_middleware/lua.rs
@@ -3,7 +3,6 @@ use std::{collections::HashMap, path::Path, str};
 use anyhow::Context;
 use memofs::{IoResultExt, Vfs};
 use rbx_dom_weak::types::Enum;
-use serde::{Deserialize, Serialize};
 
 use crate::snapshot::{InstanceContext, InstanceMetadata, InstanceSnapshot};
 
@@ -13,13 +12,6 @@ use super::{
     util::match_trailing,
 };
 
-#[derive(Debug, Clone, Copy, PartialEq, Serialize, Deserialize, Default)]
-pub enum ScriptContextType {
-    #[default]
-    Class,
-    RunContext,
-}
-
 #[derive(Debug)]
 enum ScriptType {
     Server,
@@ -58,13 +50,11 @@ pub fn snapshot_lua(
         return Ok(None);
     };
 
-    let is_run_context_enabled = context.script_type == ScriptContextType::RunContext;
-
-    let (class_name, run_context) = match (is_run_context_enabled, script_type) {
-        (true, ScriptType::Server) => ("Script", run_context_enums.get("Server")),
-        (true, ScriptType::Client) => ("Script", run_context_enums.get("Client")),
-        (false, ScriptType::Server) => ("Script", run_context_enums.get("Legacy")),
-        (false, ScriptType::Client) => ("LocalScript", None),
+    let (class_name, run_context) = match (context.emit_legacy_scripts, script_type) {
+        (false, ScriptType::Server) => ("Script", run_context_enums.get("Server")),
+        (false, ScriptType::Client) => ("Script", run_context_enums.get("Client")),
+        (true, ScriptType::Server) => ("Script", run_context_enums.get("Legacy")),
+        (true, ScriptType::Client) => ("LocalScript", None),
         (_, ScriptType::Module) => ("ModuleScript", None),
     };
 
@@ -158,7 +148,7 @@ mod test {
         let mut vfs = Vfs::new(imfs);
 
         let instance_snapshot = snapshot_lua(
-            &InstanceContext::from(ScriptContextType::Class),
+            &InstanceContext::from(true),
             &mut vfs,
             Path::new("/foo.lua"),
         )
@@ -179,7 +169,7 @@ mod test {
         let mut vfs = Vfs::new(imfs);
 
         let instance_snapshot = snapshot_lua(
-            &InstanceContext::from(ScriptContextType::RunContext),
+            &InstanceContext::from(false),
             &mut vfs,
             Path::new("/foo.lua"),
         )
@@ -200,7 +190,7 @@ mod test {
         let mut vfs = Vfs::new(imfs);
 
         let instance_snapshot = snapshot_lua(
-            &InstanceContext::from(ScriptContextType::Class),
+            &InstanceContext::from(true),
             &mut vfs,
             Path::new("/foo.server.lua"),
         )
@@ -221,7 +211,7 @@ mod test {
         let mut vfs = Vfs::new(imfs);
 
         let instance_snapshot = snapshot_lua(
-            &InstanceContext::from(ScriptContextType::RunContext),
+            &InstanceContext::from(false),
             &mut vfs,
             Path::new("/foo.server.lua"),
         )
@@ -242,7 +232,7 @@ mod test {
         let mut vfs = Vfs::new(imfs);
 
         let instance_snapshot = snapshot_lua(
-            &InstanceContext::from(ScriptContextType::Class),
+            &InstanceContext::from(true),
             &mut vfs,
             Path::new("/foo.client.lua"),
         )
@@ -263,7 +253,7 @@ mod test {
         let mut vfs = Vfs::new(imfs);
 
         let instance_snapshot = snapshot_lua(
-            &InstanceContext::from(ScriptContextType::RunContext),
+            &InstanceContext::from(false),
             &mut vfs,
             Path::new("/foo.client.lua"),
         )
@@ -289,13 +279,10 @@ mod test {
 
         let mut vfs = Vfs::new(imfs);
 
-        let instance_snapshot = snapshot_lua(
-            &InstanceContext::from(ScriptContextType::Class),
-            &mut vfs,
-            Path::new("/root"),
-        )
-        .unwrap()
-        .unwrap();
+        let instance_snapshot =
+            snapshot_lua(&InstanceContext::from(true), &mut vfs, Path::new("/root"))
+                .unwrap()
+                .unwrap();
 
         insta::with_settings!({ sort_maps => true }, {
             insta::assert_yaml_snapshot!(instance_snapshot);
@@ -322,7 +309,7 @@ mod test {
         let mut vfs = Vfs::new(imfs);
 
         let instance_snapshot = snapshot_lua(
-            &InstanceContext::from(ScriptContextType::Class),
+            &InstanceContext::from(true),
             &mut vfs,
             Path::new("/foo.lua"),
         )
@@ -354,7 +341,7 @@ mod test {
         let mut vfs = Vfs::new(imfs);
 
         let instance_snapshot = snapshot_lua(
-            &InstanceContext::from(ScriptContextType::RunContext),
+            &InstanceContext::from(false),
             &mut vfs,
             Path::new("/foo.lua"),
         )
@@ -386,7 +373,7 @@ mod test {
         let mut vfs = Vfs::new(imfs);
 
         let instance_snapshot = snapshot_lua(
-            &InstanceContext::from(ScriptContextType::Class),
+            &InstanceContext::from(true),
             &mut vfs,
             Path::new("/foo.server.lua"),
         )
@@ -418,7 +405,7 @@ mod test {
         let mut vfs = Vfs::new(imfs);
 
         let instance_snapshot = snapshot_lua(
-            &InstanceContext::from(ScriptContextType::RunContext),
+            &InstanceContext::from(false),
             &mut vfs,
             Path::new("/foo.server.lua"),
         )
@@ -452,7 +439,7 @@ mod test {
         let mut vfs = Vfs::new(imfs);
 
         let instance_snapshot = snapshot_lua(
-            &InstanceContext::from(ScriptContextType::Class),
+            &InstanceContext::from(true),
             &mut vfs,
             Path::new("/bar.server.lua"),
         )
@@ -486,7 +473,7 @@ mod test {
         let mut vfs = Vfs::new(imfs);
 
         let instance_snapshot = snapshot_lua(
-            &InstanceContext::from(ScriptContextType::RunContext),
+            &InstanceContext::from(false),
             &mut vfs,
             Path::new("/bar.server.lua"),
         )

diff --git a/src/snapshot_middleware/mod.rs b/src/snapshot_middleware/mod.rs
@@ -38,7 +38,7 @@ use self::{
     util::PathExt,
 };
 
-pub use self::{lua::ScriptContextType, project::snapshot_project_node};
+pub use self::{project::snapshot_project_node, util::emit_legacy_scripts_default};
 
 /// The main entrypoint to the snapshot function. This function can be pointed
 /// at any path and will return something if Rojo knows how to deal with it.

diff --git a/src/snapshot_middleware/project.rs b/src/snapshot_middleware/project.rs
@@ -12,7 +12,7 @@ use crate::{
     },
 };
 
-use super::snapshot_from_vfs;
+use super::{emit_legacy_scripts_default, snapshot_from_vfs};
 
 pub fn snapshot_project(
     context: &InstanceContext,
@@ -30,7 +30,11 @@ pub fn snapshot_project(
     });
 
     context.add_path_ignore_rules(rules);
-    context.set_script_type(project.script_type.unwrap_or_default());
+    context.set_emit_legacy_scripts(
+        project
+            .emit_legacy_scripts
+            .unwrap_or_else(|| emit_legacy_scripts_default().unwrap()),
+    );
 
     match snapshot_project_node(&context, path, &project.name, &project.tree, vfs, None)? {
         Some(found_snapshot) => {

diff --git a/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__csv__test__csv_from_vfs.snap b/src/snapshot_middleware/snapshots/librojo__snapshot_middleware__csv__test__csv_from_vfs.snap
@@ -11,7 +11,7 @@ metadata:
     - /foo.csv
     - /foo.meta.json
   context:
-    script_type: Class
+    emit_legacy_scripts: true
 name: foo
 class_name: LocalizationTable
 properties: