From 000373a3cca22ca61400ff457833732dfff1afef Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mart=C3=ADn=20=28Netux=29=20Rodr=C3=ADguez?=
 <send.email@netux.site>
Date: Sun, 21 Jan 2024 22:27:48 -0300
Subject: [PATCH] feat(editor compiler): include subroutines used in Call
 Subroutine and Start Rule actions in the compiled output

While there is linter logic to catch these, it is sometimes hard to see the linter error in certain cases.

An argument can be made that, because the in-game editor allows for creating a subroutine but never defining a rule for it, we should also allow it as well.

In fact, this change fixes a case where importing a snippet that declares a subroutine but never define it, then compiling the project and trying to import that into the game would cause a workshop error because we stripped the subroutine from the subroutines list.

Also: refactor `getSubroutines` to use `String.matchAll()`, which is a bit better than `String.match()` and a ton of `String.replace()`s.
---
 .../src/utils/compiler/subroutines.js         |  8 ++++---
 .../utils/compiler/subroutines.test.js        | 23 ++++++++++++++++---
 2 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/app/javascript/src/utils/compiler/subroutines.js b/app/javascript/src/utils/compiler/subroutines.js
index 2bf3366e0..5a9a61fe1 100644
--- a/app/javascript/src/utils/compiler/subroutines.js
+++ b/app/javascript/src/utils/compiler/subroutines.js
@@ -10,7 +10,9 @@ ${ subroutines.map((v, i) => `    ${ i }: ${ v }`).join("\n") }
 }
 
 export function getSubroutines(joinedItems) {
-  let subroutines = joinedItems.match(/Subroutine;[\r\n]+([^\r\n;]+)/g) || []
-  subroutines = subroutines.map(s => s.replace("Subroutine;\n", "").replace("Call Subroutine", "").replace(/[\())\s]/g, ""))
-  return [...new Set(subroutines)]
+  const declaredSubroutines = Array.from(joinedItems.matchAll(/Subroutine;[\r\n]+([^\r\n;]+)/g))
+    .map((match) => match[1].trim())
+  const usedSubroutines = Array.from(joinedItems.matchAll(/(?:Call Subroutine|Start Rule)\s*\(([^,\)]+)/g))
+    .map((match) => match[1].trim())
+  return [...new Set([...declaredSubroutines, ...usedSubroutines])]
 }
diff --git a/spec/javascript/utils/compiler/subroutines.test.js b/spec/javascript/utils/compiler/subroutines.test.js
index 77cba9ebc..7b233f975 100644
--- a/spec/javascript/utils/compiler/subroutines.test.js
+++ b/spec/javascript/utils/compiler/subroutines.test.js
@@ -3,7 +3,7 @@ import { disregardWhitespace } from "../../helpers/text"
 
 describe("subroutines.js", () => {
   describe("getSubroutines", () => {
-    test("Should extract subroutines", () => {
+    test("Should extract subroutines declared from rules", () => {
       const input = `
         Subroutine;
         someSubroutine1;
@@ -15,6 +15,16 @@ describe("subroutines.js", () => {
       expect(getSubroutines(input)).toEqual(expectedOutput)
     })
 
+    test("Should extract subroutines used in actions", () => {
+      const input = `
+        Call Subroutine(someSubroutine1);
+
+        Start Rule(someSubroutine2, OtherParamIdk);
+      `
+      const expectedOutput = ["someSubroutine1", "someSubroutine2"]
+      expect(getSubroutines(input)).toEqual(expectedOutput)
+    })
+
     test("Should handle duplicate subroutines", () => {
       const input = `
         Subroutine;
@@ -22,6 +32,10 @@ describe("subroutines.js", () => {
 
         Subroutine;
         someSubroutine;
+
+        Call Subroutine(someSubroutine);
+
+        Start Rule(someSubroutine, OtherParamIdk);
       `
       const expectedOutput = ["someSubroutine"]
       expect(getSubroutines(input)).toEqual(expectedOutput)
@@ -32,11 +46,14 @@ describe("subroutines.js", () => {
     test("Should compile subroutines", () => {
       const input = `
         Subroutine;
-        someSubroutine;
+        someSubroutine1;
+
+        Call Subroutine(someSubroutine2);
       `
       const expectedOutput = `
         subroutines {
-          0: someSubroutine
+          0: someSubroutine1
+          1: someSubroutine2
         }\n\n
       `
       expect(disregardWhitespace(compileSubroutines(input))).toBe(disregardWhitespace(expectedOutput))