From 8edf2f0ac6462eb0cf9c1f1427a4bb764a1dc934 Mon Sep 17 00:00:00 2001
From: Misa <infoteddyatgitgud@cock.li>
Date: Thu, 11 Jun 2020 17:31:57 -0700
Subject: [PATCH] Refactor custom scripts to not be stored in one giant vector
 of lines

This commit refactors custom level scripts to no longer be stored in one
giant vector containing not only every single script name, but every
single script's contents as well. More specifically,
scriptclass::customscript has been converted to an std::vector<Script>
scriptclass::customscripts (note the extra S), and a Script is just a
struct with an std::string name and std::vector<std::string> contents.

This is an improvement in both performance and maintainability. The game
no longer has to look through script contents in case they're actually
script names, and then manually extract the script contents from there.
Instead, all it has to do is look for script names only. And the
contents are provided for free. This results in a performance gain.

Also, the old system resulted in lots of boilerplate everywhere anytime
scripts had to be handled or parsed. Now, the boilerplate is only done
when saving or loading a custom level. This makes code quality much,
much better.

To be sure I didn't actually change anything, I tested by first saving
Dimension Open in current 2.3 (because current 2.3 gets rid of the
awful edentity whitespace), and then resaved it on this patch. There is
absolutely no difference between the current-2.3-resave and
this-patch-resave.
---
 desktop_version/src/Script.cpp |  66 ++++++--------
 desktop_version/src/Script.h   |   8 +-
 desktop_version/src/editor.cpp | 156 +++++++++++++--------------------
 3 files changed, 96 insertions(+), 134 deletions(-)

diff --git a/desktop_version/src/Script.cpp b/desktop_version/src/Script.cpp
index 873265e4..7f3777be 100644
--- a/desktop_version/src/Script.cpp
+++ b/desktop_version/src/Script.cpp
@@ -30,7 +30,7 @@ scriptclass::scriptclass()
 }
 
 void scriptclass::clearcustom(){
-	customscript.clear();
+	customscripts.clear();
 }
 
 void scriptclass::tokenize( std::string t )
@@ -3553,38 +3553,24 @@ void scriptclass::loadcustom(std::string t)
 		if(i>=7) cscriptname+=t[i];
 	}
 
-	int scriptstart=-1;
-	int scriptend=-1;
 	std::string tstring;
 
-	for(size_t i=0; i<customscript.size(); i++){
-		if(scriptstart==-1){
-			//Find start of the script
-			if(customscript[i]==cscriptname+":"){
-				scriptstart=i+1;
-			}
-		}else if(scriptend==-1){
-			//Find the end
-			tstring=customscript[i];
-			if (tstring.size() > 0) {
-				tstring=tstring[tstring.size()-1];
-			} else {
-				tstring="";
-			}
-			if(tstring==":"){
-				scriptend=i;
-			}
+	std::vector<std::string>* contents = NULL;
+	for(size_t i = 0; i < customscripts.size(); i++){
+		Script& script_ = customscripts[i];
+
+		if(script_.name == cscriptname){
+			contents = &script_.contents;
+			break;
 		}
 	}
-	if(scriptstart>-1){
-		if(scriptend==-1){
-			scriptend=customscript.size();
-		}
+	if(contents != NULL){
+		std::vector<std::string>& lines = *contents;
 
 		//Ok, we've got the relavent script segment, we do a pass to assess it, then run it!
 		int customcutscenemode=0;
-		for(int i=scriptstart; i<scriptend; i++){
-			tokenize(customscript[i]);
+		for(size_t i=0; i<lines.size(); i++){
+			tokenize(lines[i]);
 			if(words[0] == "say"){
 				customcutscenemode=1;
 			}else if(words[0] == "reply"){
@@ -3600,10 +3586,10 @@ void scriptclass::loadcustom(std::string t)
 		int speakermode=0; //0, terminal, numbers for crew
 		int squeakmode=0;//default on
 		//Now run the script
-		for(int i=scriptstart; i<scriptend; i++){
+		for(size_t i=0; i<lines.size(); i++){
 			words[0]="nothing"; //Default!
 			words[1]="1"; //Default!
-			tokenize(customscript[i]);
+			tokenize(lines[i]);
 			std::transform(words[0].begin(), words[0].end(), words[0].begin(), ::tolower);
 			if(words[0] == "music"){
 				if(customtextmode==1){ add("endtext"); customtextmode=0;}
@@ -3715,28 +3701,28 @@ void scriptclass::loadcustom(std::string t)
 				}
 			}else if(words[0] == "delay"){
 				if(customtextmode==1){ add("endtext"); customtextmode=0;}
-				add(customscript[i]);
+				add(lines[i]);
 			}else if(words[0] == "flag"){
 				if(customtextmode==1){ add("endtext"); customtextmode=0;}
-				add(customscript[i]);
+				add(lines[i]);
 			}else if(words[0] == "map"){
 				if(customtextmode==1){ add("endtext"); customtextmode=0;}
-				add("custom"+customscript[i]);
+				add("custom"+lines[i]);
 			}else if(words[0] == "warpdir"){
 				if(customtextmode==1){ add("endtext"); customtextmode=0;}
-				add(customscript[i]);
+				add(lines[i]);
 			}else if(words[0] == "ifwarp"){
 				if(customtextmode==1){ add("endtext"); customtextmode=0;}
-				add(customscript[i]);
+				add(lines[i]);
 			}else if(words[0] == "iftrinkets"){
 				if(customtextmode==1){ add("endtext"); customtextmode=0;}
-				add("custom"+customscript[i]);
+				add("custom"+lines[i]);
 			}else if(words[0] == "ifflag"){
 				if(customtextmode==1){ add("endtext"); customtextmode=0;}
-				add("custom"+customscript[i]);
+				add("custom"+lines[i]);
 			}else if(words[0] == "iftrinketsless"){
 				if(customtextmode==1){ add("endtext"); customtextmode=0;}
-				add("custom"+customscript[i]);
+				add("custom"+lines[i]);
 			}else if(words[0] == "destroy"){
 				if(customtextmode==1){ add("endtext"); customtextmode=0;}
 				if(words[1]=="gravitylines"){
@@ -3798,8 +3784,8 @@ void scriptclass::loadcustom(std::string t)
 				int nti = ti>=0 && ti<=50 ? ti : 1;
 				for(int ti2=0; ti2<nti; ti2++){
 					i++;
-					if(i < (int) customscript.size()){
-						add(customscript[i]);
+					if(i < lines.size()){
+						add(lines[i]);
 					}
 				}
 
@@ -3823,8 +3809,8 @@ void scriptclass::loadcustom(std::string t)
 				int nti = ti>=0 && ti<=50 ? ti : 1;
 				for(int ti2=0; ti2<nti; ti2++){
 					i++;
-					if(i < (int) customscript.size()){
-						add(customscript[i]);
+					if(i < lines.size()){
+						add(lines[i]);
 					}
 				}
 				add("position(player,above)");
diff --git a/desktop_version/src/Script.h b/desktop_version/src/Script.h
index 1d5073d0..c1aa6bb2 100644
--- a/desktop_version/src/Script.h
+++ b/desktop_version/src/Script.h
@@ -9,6 +9,12 @@
 #define filllines(lines) commands.insert(commands.end(), lines, lines + sizeof(lines)/sizeof(lines[0]))
 
 
+struct Script
+{
+    std::string name;
+    std::vector<std::string> contents;
+};
+
 class scriptclass
 {
 public:
@@ -63,7 +69,7 @@ public:
     int i, j, k;
 
     //Custom level stuff
-     std::vector <std::string>  customscript;
+    std::vector<Script> customscripts;
 };
 
 extern scriptclass script;
diff --git a/desktop_version/src/editor.cpp b/desktop_version/src/editor.cpp
index ff5662db..b9a1748d 100644
--- a/desktop_version/src/editor.cpp
+++ b/desktop_version/src/editor.cpp
@@ -380,7 +380,7 @@ void editorclass::reset()
 
     hookmenupage=0;
     hookmenu=0;
-    script.customscript.clear();
+    script.customscripts.clear();
 
     returneditoralpha = 0;
 }
@@ -389,29 +389,11 @@ void editorclass::gethooks()
 {
     //Scan through the script and create a hooks list based on it
     hooklist.clear();
-    std::string tstring;
-    std::string tstring2;
-    for(size_t i=0; i<script.customscript.size(); i++)
+    for (size_t i = 0; i < script.customscripts.size(); i++)
     {
-        tstring=script.customscript[i];
-        if((int) tstring.length() - 1 >= 0) // FIXME: This is sketchy. -flibit
-        {
-            tstring=tstring[tstring.length()-1];
-        }
-        else
-        {
-            tstring="";
-        }
-        if(tstring==":")
-        {
-            tstring2="";
-            tstring=script.customscript[i];
-            for(size_t j=0; j<tstring.length()-1; j++)
-            {
-                tstring2+=tstring[j];
-            }
-            hooklist.push_back(tstring2);
-        }
+        Script& script_ = script.customscripts[i];
+
+        hooklist.push_back(script_.name);
     }
 }
 
@@ -420,34 +402,17 @@ void editorclass::loadhookineditor(std::string t)
     //Find hook t in the scriptclass, then load it into the editor
     clearscriptbuffer();
 
-    std::string tstring;
-
-    bool removemode=false;
-    for(size_t i=0; i<script.customscript.size(); i++)
+    for(size_t i = 0; i < script.customscripts.size(); i++)
     {
-        if(script.customscript[i]==t+":")
+        Script& script_ = script.customscripts[i];
+
+        if(script_.name == t)
         {
-            removemode=true;
-        }
-        else if(removemode)
-        {
-            tstring=script.customscript[i];
-            if(tstring != "")
-            {
-                tstring = tstring[tstring.length()-1];
-            }
-            if(tstring==":")
-            {
-                //this is a hook
-                removemode=false;
-            }
-            else
-            {
-                //load in this line
-                sb.push_back(script.customscript[i]);
-            }
+            sb = script_.contents;
+            break;
         }
     }
+
     if(sb.empty())
     {
         //Always have one line or we'll have problems
@@ -459,57 +424,23 @@ void editorclass::addhooktoscript(std::string t)
 {
     //Adds hook+the scriptbuffer to the end of the scriptclass
     removehookfromscript(t);
-    script.customscript.push_back(t+":");
-    for(size_t i=0; i<sb.size(); i++)
-    {
-        script.customscript.push_back(sb[i]);
-    }
+    Script script_;
+    script_.name = t;
+    script_.contents = sb;
+    script.customscripts.push_back(script_);
 }
 
 void editorclass::removehookfromscript(std::string t)
 {
     //Find hook t in the scriptclass, then removes it (and any other code with it)
-    std::string tstring;
-    bool removemode=false;
-    for(size_t i=0; i<script.customscript.size(); i++)
+    for (size_t i = 0; i < script.customscripts.size(); i++)
     {
-        if(script.customscript[i]==t+":")
-        {
-            removemode=true;
-            //Remove this line
-            for(size_t j=i; j<script.customscript.size()-1; j++)
-            {
-                script.customscript[j]=script.customscript[j+1];
-            }
-            script.customscript.pop_back();
+        Script& script_ = script.customscripts[i];
 
-            i--;
-        }
-        else if(removemode)
+        if (script_.name == t)
         {
-            //If this line is not the start of a new hook, remove it!
-            tstring=script.customscript[i];
-            if (tstring.length() > 0) {
-                tstring=tstring[tstring.length()-1];
-            } else {
-                tstring="";
-            }
-            if(tstring==":")
-            {
-                //this is a hook
-                removemode=false;
-            }
-            else
-            {
-                //Remove this line
-                for(size_t j=i; j<script.customscript.size()-1; j++)
-                {
-                    script.customscript[j]=script.customscript[j+1];
-                }
-                script.customscript.pop_back();
-
-                i--;
-            }
+            script.customscripts.erase(script.customscripts.begin() + i);
+            break;
         }
     }
 }
@@ -1915,9 +1846,42 @@ bool editorclass::load(std::string& _path)
             {
                 std::vector<std::string> values = split(TextString,'|');
                 script.clearcustom();
+                Script script_;
+                bool headerfound = false;
                 for(size_t i = 0; i < values.size(); i++)
                 {
-                    script.customscript.push_back(values[i]);
+                    std::string& line = values[i];
+
+                    //Comparing line[line.length()-1] directly to a string literal is UB
+                    //Workaround: assign line[line.length()-1] to a string first
+                    std::string temp;
+                    if(line.length())
+                    {
+                        temp = line[line.length()-1];
+                    }
+                    if(temp == ":")
+                    {
+                        if(headerfound)
+                        {
+                            //Add the script if we have a preceding header
+                            script.customscripts.push_back(script_);
+                        }
+                        script_.name = line.substr(0, line.length()-1);
+                        script_.contents.clear();
+                        headerfound = true;
+                        continue;
+                    }
+
+                    if(headerfound)
+                    {
+                        script_.contents.push_back(line);
+                    }
+                }
+                //Add the last script
+                if(headerfound)
+                {
+                    //Add the script if we have a preceding header
+                    script.customscripts.push_back(script_);
                 }
 
             }
@@ -2074,9 +2038,15 @@ bool editorclass::save(std::string& _path)
     data->LinkEndChild( msg );
 
     std::string scriptString;
-    for(size_t i = 0; i < script.customscript.size(); i++ )
+    for(size_t i = 0; i < script.customscripts.size(); i++)
     {
-        scriptString += script.customscript[i] + "|";
+        Script& script_ = script.customscripts[i];
+
+        scriptString += script_.name + ":|";
+        for (size_t i = 0; i < script_.contents.size(); i++)
+        {
+            scriptString += script_.contents[i] + "|";
+        }
     }
     msg = new TiXmlElement( "script" );
     msg->LinkEndChild( new TiXmlText( scriptString.c_str() ));