From e7ab9e3e0dd77626c7691d7a95bfe5eb0b0e2b5e Mon Sep 17 00:00:00 2001 From: ike709 Date: Sun, 27 Oct 2024 16:55:28 -0500 Subject: [PATCH 01/25] Adds a system for printing stats to the disassembler --- DMDisassembler/Program.cs | 120 +++++++++++++++++++++++++++++++++----- 1 file changed, 106 insertions(+), 14 deletions(-) diff --git a/DMDisassembler/Program.cs b/DMDisassembler/Program.cs index 36fdff2cf2..bf9149153e 100644 --- a/DMDisassembler/Program.cs +++ b/DMDisassembler/Program.cs @@ -1,9 +1,11 @@ using System; using System.Collections.Generic; using System.IO; +using System.Linq; using System.Text; using System.Text.Json; using DMCompiler.Json; +using JetBrains.Annotations; namespace DMDisassembler; @@ -13,6 +15,7 @@ internal class Program { public static DMProc GlobalInitProc = null; public static List Procs = null; public static Dictionary AllTypes = null; + public static List TypesById = null; private static readonly string NoTypeSelectedMessage = "No type is selected"; @@ -52,6 +55,8 @@ static void Main(string[] args) { } } + Console.WriteLine("DM Disassembler for OpenDream. Enter a command or \"help\" for more information."); + bool acceptingCommands = true; while (acceptingCommands) { if (_selectedType != null) { @@ -71,6 +76,7 @@ static void Main(string[] args) { switch (command) { case "quit": + case "exit": case "q": acceptingCommands = false; break; case "search": Search(split); break; case "sel": @@ -78,25 +84,108 @@ static void Main(string[] args) { case "list": List(split); break; case "d": case "decompile": Decompile(split); break; + case "stats": Stats(GetArg()); break; case "test-all": TestAll(); break; case "dump-all": DumpAll(); break; - case "help": PrintHelp(); break; - default: Console.WriteLine("Invalid command \"" + command + "\""); break; + case "help": { + PrintHelp(GetArg()); + break; + } + default: Console.WriteLine($"Invalid command \"{command}\""); break; + } + + [CanBeNull] + string GetArg() { + if (split.Length > 2) { + Console.WriteLine($"Command \"{command}\" takes 0 or 1 arguments."); + } + + return split.Length > 1 ? split[1] : null; } } } - private static void PrintHelp() { - Console.WriteLine("DM Disassembler for OpenDream"); - Console.WriteLine("Commands and arguments:"); - Console.WriteLine("help : Show this help"); - Console.WriteLine("quit|q : Exits the disassembler"); - Console.WriteLine("search type|proc [name] : Search for a particular typepath or a proc on a selected type"); - Console.WriteLine("select|sel : Select a typepath to run further commands on"); - Console.WriteLine("list procs|globals : List all globals, or all procs on a selected type"); - Console.WriteLine("decompile|d [name] : Decompiles the proc on the selected type"); - Console.WriteLine("dump-all : Decompiles every proc and writes the output to a file"); - Console.WriteLine("test-all : Tries to decompile every single proc to check for issues with this disassembler; not for production use"); + private static void PrintHelp([CanBeNull] string command) { + if (string.IsNullOrEmpty(command)) { + AllCommands(); + return; + } + + command = command.ToLower(); + + switch (command) { + case "stats": { + Console.WriteLine("Prints various statistics. Usage: stats [type]"); + Console.WriteLine("Options for [type]:"); + Console.WriteLine("procs-by-type : Prints the number of proc declarations (not overrides) on each type in descending order"); + break; + } + default: { + Console.WriteLine($"No additional help for \"{command}\""); + AllCommands(); + break; + } + } + + void AllCommands() + { + Console.WriteLine("DM Disassembler for OpenDream"); + Console.WriteLine("Commands and arguments:"); + Console.WriteLine("help [command] : Show additional help for [command] if applicable"); + Console.WriteLine("quit|q : Exits the disassembler"); + Console.WriteLine("search type|proc [name] : Search for a particular typepath or a proc on a selected type"); + Console.WriteLine("select|sel : Select a typepath to run further commands on"); + Console.WriteLine("list procs|globals : List all globals, or all procs on a selected type"); + Console.WriteLine("decompile|d [name] : Decompiles the proc on the selected type"); + Console.WriteLine("stats [type] : Prints various stats about the game. Use \"help stats\" for more info"); + Console.WriteLine("dump-all : Decompiles every proc and writes the output to a file"); + Console.WriteLine("test-all : Tries to decompile every single proc to check for issues with this disassembler; not for production use"); + } + } + + private static void Stats([CanBeNull] string statType) { + if (string.IsNullOrEmpty(statType)) { + PrintHelp("stats"); + return; + } + + switch (statType) { + case "procs-by-type": { + ProcsByType(); + return; + } + default: { + Console.WriteLine($"Unknown stat \"{statType}\""); + PrintHelp("stats"); + return; + } + } + + void ProcsByType() { + Console.WriteLine("Counting all proc declarations (no overrides) by type. This may take a moment."); + Dictionary TypeIdToProcCount = new Dictionary(); + foreach (DMProc proc in Procs) { + if(proc.IsOverride || proc.Name == "") continue; // Don't count overrides or procs + if (TypeIdToProcCount.TryGetValue(proc.OwningTypeId, out var count)) { + TypeIdToProcCount[proc.OwningTypeId] = count + 1; + } else { + TypeIdToProcCount[proc.OwningTypeId] = 1; + } + } + + var sorted = TypeIdToProcCount.OrderByDescending(kvp => kvp.Value).ToList(); + Console.WriteLine("Type: Proc Declarations"); + for (int i = 0; i < sorted.Count; i++) { + var pair = sorted[i]; + + var type = TypesById[pair.Key]; + if (pair.Key == 0) { + Console.WriteLine($": {pair.Value}"); + } else { + Console.WriteLine($"{type.Path}: {pair.Value}"); + } + } + } } private static void Search(string[] args) { @@ -224,9 +313,12 @@ private static void LoadAllProcs() { private static void LoadAllTypes() { AllTypes = new Dictionary(CompiledJson.Types.Length); + TypesById = new List(CompiledJson.Types.Length); foreach (DreamTypeJson json in CompiledJson.Types) { - AllTypes.Add(json.Path, new DMType(json)); + var dmType = new DMType(json); + AllTypes.Add(json.Path, dmType); + TypesById.Add(dmType); } //Add global procs to the root type From 006acf109286919db3119c33eebee2eda1f64a7e Mon Sep 17 00:00:00 2001 From: ike709 Date: Sun, 27 Oct 2024 17:04:51 -0500 Subject: [PATCH 02/25] Clarify message --- DMDisassembler/Program.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DMDisassembler/Program.cs b/DMDisassembler/Program.cs index bf9149153e..0a22ec1e65 100644 --- a/DMDisassembler/Program.cs +++ b/DMDisassembler/Program.cs @@ -97,7 +97,7 @@ static void Main(string[] args) { [CanBeNull] string GetArg() { if (split.Length > 2) { - Console.WriteLine($"Command \"{command}\" takes 0 or 1 arguments."); + Console.WriteLine($"Command \"{command}\" takes 0 or 1 arguments. Ignoring extra arguments."); } return split.Length > 1 ? split[1] : null; From 91657f46e4a2d514ed6dd66fcd7d8a81b5815ad3 Mon Sep 17 00:00:00 2001 From: ike709 Date: Sun, 27 Oct 2024 17:06:27 -0500 Subject: [PATCH 03/25] rename --- DMDisassembler/Program.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/DMDisassembler/Program.cs b/DMDisassembler/Program.cs index bf9149153e..27a44d53b1 100644 --- a/DMDisassembler/Program.cs +++ b/DMDisassembler/Program.cs @@ -163,17 +163,17 @@ private static void Stats([CanBeNull] string statType) { void ProcsByType() { Console.WriteLine("Counting all proc declarations (no overrides) by type. This may take a moment."); - Dictionary TypeIdToProcCount = new Dictionary(); + Dictionary typeIdToProcCount = new Dictionary(); foreach (DMProc proc in Procs) { if(proc.IsOverride || proc.Name == "") continue; // Don't count overrides or procs - if (TypeIdToProcCount.TryGetValue(proc.OwningTypeId, out var count)) { - TypeIdToProcCount[proc.OwningTypeId] = count + 1; + if (typeIdToProcCount.TryGetValue(proc.OwningTypeId, out var count)) { + typeIdToProcCount[proc.OwningTypeId] = count + 1; } else { - TypeIdToProcCount[proc.OwningTypeId] = 1; + typeIdToProcCount[proc.OwningTypeId] = 1; } } - var sorted = TypeIdToProcCount.OrderByDescending(kvp => kvp.Value).ToList(); + var sorted = typeIdToProcCount.OrderByDescending(kvp => kvp.Value).ToList(); Console.WriteLine("Type: Proc Declarations"); for (int i = 0; i < sorted.Count; i++) { var pair = sorted[i]; From 782bc7df2a787a06d0d62fb4df34e449e976f6b4 Mon Sep 17 00:00:00 2001 From: ike709 Date: Sat, 30 Nov 2024 14:18:35 -0600 Subject: [PATCH 04/25] Cope with extra tokens after proc statements --- .../Tests/Statements/extra_token_pragma.dm | 6 +++ .../Statements/thing_after_statement.dm} | 4 ++ DMCompiler/Compiler/CompilerError.cs | 1 + DMCompiler/Compiler/DM/DMParser.cs | 38 ++++++++++++++++++- DMCompiler/DMStandard/DefaultPragmaConfig.dm | 1 + 5 files changed, 48 insertions(+), 2 deletions(-) create mode 100644 Content.Tests/DMProject/Tests/Statements/extra_token_pragma.dm rename Content.Tests/DMProject/{Broken Tests/Statements/If/thing_after_if.dm => Tests/Statements/thing_after_statement.dm} (58%) diff --git a/Content.Tests/DMProject/Tests/Statements/extra_token_pragma.dm b/Content.Tests/DMProject/Tests/Statements/extra_token_pragma.dm new file mode 100644 index 0000000000..ce54b039e4 --- /dev/null +++ b/Content.Tests/DMProject/Tests/Statements/extra_token_pragma.dm @@ -0,0 +1,6 @@ +// COMPILE ERROR OD3205 +#pragma ExtraToken error + +/proc/RunTest() + if(1). + ASSERT(TRUE) diff --git a/Content.Tests/DMProject/Broken Tests/Statements/If/thing_after_if.dm b/Content.Tests/DMProject/Tests/Statements/thing_after_statement.dm similarity index 58% rename from Content.Tests/DMProject/Broken Tests/Statements/If/thing_after_if.dm rename to Content.Tests/DMProject/Tests/Statements/thing_after_statement.dm index cd943fa17f..194f1ab340 100644 --- a/Content.Tests/DMProject/Broken Tests/Statements/If/thing_after_if.dm +++ b/Content.Tests/DMProject/Tests/Statements/thing_after_statement.dm @@ -8,3 +8,7 @@ ASSERT(TRUE) else ASSERT(FALSE) + for(var/i in 1 to 1): + ASSERT(TRUE) + for(var/i in 1 to 1). + ASSERT(TRUE) diff --git a/DMCompiler/Compiler/CompilerError.cs b/DMCompiler/Compiler/CompilerError.cs index f23dc9a2df..b2752df1ce 100644 --- a/DMCompiler/Compiler/CompilerError.cs +++ b/DMCompiler/Compiler/CompilerError.cs @@ -74,6 +74,7 @@ public enum WarningCode { AssignmentInConditional = 3202, PickWeightedSyntax = 3203, AmbiguousInOrder = 3204, + ExtraToken = 3205, RuntimeSearchOperator = 3300, // 4000 - 4999 are reserved for runtime configuration. (TODO: Runtime doesn't know about configs yet!) diff --git a/DMCompiler/Compiler/DM/DMParser.cs b/DMCompiler/Compiler/DM/DMParser.cs index 00c566fd16..4f6f710f68 100644 --- a/DMCompiler/Compiler/DM/DMParser.cs +++ b/DMCompiler/Compiler/DM/DMParser.cs @@ -1125,8 +1125,10 @@ private DMASTProcStatementIf If() { BracketWhitespace(); ConsumeRightParenthesis(); - Whitespace(); - Check(TokenType.DM_Colon); + if (Check(TokenType.DM_Colon) || Check(TokenType.DM_Period)) { + Emit(WarningCode.ExtraToken, loc, "Extra token at end of proc statement"); + } + Whitespace(); DMASTProcStatement? procStatement = ProcStatement(); @@ -1165,6 +1167,10 @@ private DMASTProcStatement For() { Whitespace(); if (Check(TokenType.DM_RightParenthesis)) { + if (Check(TokenType.DM_Colon) || Check(TokenType.DM_Period)) { + Emit(WarningCode.ExtraToken, loc, "Extra token at end of proc statement"); + } + return new DMASTProcStatementInfLoop(loc, GetForBody(loc)); } @@ -1185,6 +1191,10 @@ private DMASTProcStatement For() { if (expr1 is DMASTAssign assign) { ExpressionTo(out var endRange, out var step); Consume(TokenType.DM_RightParenthesis, "Expected ')' in for after to expression"); + if (Check(TokenType.DM_Colon) || Check(TokenType.DM_Period)) { + Emit(WarningCode.ExtraToken, loc, "Extra token at end of proc statement"); + } + return new DMASTProcStatementFor(loc, new DMASTExpressionInRange(loc, assign.LHS, assign.RHS, endRange, step), null, null, dmTypes, GetForBody(loc)); } else { Emit(WarningCode.BadExpression, "Expected = before to in for"); @@ -1197,15 +1207,27 @@ private DMASTProcStatement For() { DMASTExpression? listExpr = Expression(); Whitespace(); Consume(TokenType.DM_RightParenthesis, "Expected ')' in for after expression 2"); + if (Check(TokenType.DM_Colon) || Check(TokenType.DM_Period)) { + Emit(WarningCode.ExtraToken, loc, "Extra token at end of proc statement"); + } + return new DMASTProcStatementFor(loc, new DMASTExpressionIn(loc, expr1, listExpr), null, null, dmTypes, GetForBody(loc)); } if (!Check(ForSeparatorTypes)) { Consume(TokenType.DM_RightParenthesis, "Expected ')' in for after expression 1"); + if (Check(TokenType.DM_Colon) || Check(TokenType.DM_Period)) { + Emit(WarningCode.ExtraToken, loc, "Extra token at end of proc statement"); + } + return new DMASTProcStatementFor(loc, expr1, null, null, dmTypes, GetForBody(loc)); } if (Check(TokenType.DM_RightParenthesis)) { + if (Check(TokenType.DM_Colon) || Check(TokenType.DM_Period)) { + Emit(WarningCode.ExtraToken, loc, "Extra token at end of proc statement"); + } + return new DMASTProcStatementFor(loc, expr1, null, null, dmTypes, GetForBody(loc)); } @@ -1221,10 +1243,18 @@ private DMASTProcStatement For() { if (!Check(ForSeparatorTypes)) { Consume(TokenType.DM_RightParenthesis, "Expected ')' in for after expression 2"); + if (Check(TokenType.DM_Colon) || Check(TokenType.DM_Period)) { + Emit(WarningCode.ExtraToken, loc, "Extra token at end of proc statement"); + } + return new DMASTProcStatementFor(loc, expr1, expr2, null, dmTypes, GetForBody(loc)); } if (Check(TokenType.DM_RightParenthesis)) { + if (Check(TokenType.DM_Colon) || Check(TokenType.DM_Period)) { + Emit(WarningCode.ExtraToken, loc, "Extra token at end of proc statement"); + } + return new DMASTProcStatementFor(loc, expr1, expr2, null, dmTypes, GetForBody(loc)); } @@ -1239,6 +1269,10 @@ private DMASTProcStatement For() { } Consume(TokenType.DM_RightParenthesis, "Expected ')' in for after expression 3"); + if (Check(TokenType.DM_Colon) || Check(TokenType.DM_Period)) { + Emit(WarningCode.ExtraToken, loc, "Extra token at end of proc statement"); + } + return new DMASTProcStatementFor(loc, expr1, expr2, expr3, dmTypes, GetForBody(loc)); DMASTProcBlockInner GetForBody(Location forLocation) { diff --git a/DMCompiler/DMStandard/DefaultPragmaConfig.dm b/DMCompiler/DMStandard/DefaultPragmaConfig.dm index f6adf4e312..103b84ab59 100644 --- a/DMCompiler/DMStandard/DefaultPragmaConfig.dm +++ b/DMCompiler/DMStandard/DefaultPragmaConfig.dm @@ -52,4 +52,5 @@ #pragma AssignmentInConditional warning #pragma PickWeightedSyntax disabled #pragma AmbiguousInOrder warning +#pragma ExtraToken warning #pragma RuntimeSearchOperator disabled From e6384223be6cf0b120dfdc998fe6027e24e2d25d Mon Sep 17 00:00:00 2001 From: Amy <3855802+amylizzle@users.noreply.github.com> Date: Wed, 4 Dec 2024 18:27:53 +0000 Subject: [PATCH 05/25] Fix initial filter in object defs and adds test (#2121) Co-authored-by: amylizzle --- .../DMProject/Tests/filter_initial.dm | 16 ++++++++++++++++ Content.IntegrationTests/DMProject/code.dm | 1 + .../DMProject/environment.dme | 1 + DMCompiler/DM/DMCodeTree.Vars.cs | 1 + 4 files changed, 19 insertions(+) create mode 100644 Content.IntegrationTests/DMProject/Tests/filter_initial.dm diff --git a/Content.IntegrationTests/DMProject/Tests/filter_initial.dm b/Content.IntegrationTests/DMProject/Tests/filter_initial.dm new file mode 100644 index 0000000000..65812623c0 --- /dev/null +++ b/Content.IntegrationTests/DMProject/Tests/filter_initial.dm @@ -0,0 +1,16 @@ +/obj/blurry + filters = filter(type="blur", size=2) + +/obj/veryblurry + filters = list(type="blur", size=4) + +/obj/notatallblurry + filters = list() + +/proc/test_filter_init() + var/obj/veryblurry/VB = new() + ASSERT(length(VB.filters) == 1) + var/obj/blurry/B = new() + ASSERT(length(B.filters) == 1) + var/obj/notatallblurry/NAB = new() + ASSERT(length(NAB.filters) == 0) \ No newline at end of file diff --git a/Content.IntegrationTests/DMProject/code.dm b/Content.IntegrationTests/DMProject/code.dm index ebb99dd087..22379ac075 100644 --- a/Content.IntegrationTests/DMProject/code.dm +++ b/Content.IntegrationTests/DMProject/code.dm @@ -30,4 +30,5 @@ test_color_matrix() test_range() test_verb_duplicate() + test_filter_init() world.log << "IntegrationTests successful, /world/New() exiting..." \ No newline at end of file diff --git a/Content.IntegrationTests/DMProject/environment.dme b/Content.IntegrationTests/DMProject/environment.dme index a4abe97328..f58687b514 100644 --- a/Content.IntegrationTests/DMProject/environment.dme +++ b/Content.IntegrationTests/DMProject/environment.dme @@ -3,5 +3,6 @@ #include "Tests/color_matrix.dm" #include "Tests/range.dm" #include "Tests/verb_duplicate.dm" +#include "Tests/filter_initial.dm" #include "map.dmm" #include "interface.dmf" \ No newline at end of file diff --git a/DMCompiler/DM/DMCodeTree.Vars.cs b/DMCompiler/DM/DMCodeTree.Vars.cs index 0657c8cccd..b259047fb3 100644 --- a/DMCompiler/DM/DMCodeTree.Vars.cs +++ b/DMCompiler/DM/DMCodeTree.Vars.cs @@ -74,6 +74,7 @@ private bool IsValidRightHandSide(DMCompiler compiler, DMObject dmObject, DMExpr "file" => true, "sound" => true, "nameof" => true, + "filter" => true, _ => false }, From d7962db2579e4a2d232efe51eadb2c184b680ab2 Mon Sep 17 00:00:00 2001 From: Penelope Haze Date: Sat, 7 Dec 2024 16:44:26 -0500 Subject: [PATCH 06/25] Fix __IMPLIED_TYPE__ in proc arguments (#2122) --- Content.Tests/DMProject/Tests/Builtins/__IMPLIED_TYPE__.dm | 7 +++++-- DMCompiler/DM/Builders/DMExpressionBuilder.cs | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/Content.Tests/DMProject/Tests/Builtins/__IMPLIED_TYPE__.dm b/Content.Tests/DMProject/Tests/Builtins/__IMPLIED_TYPE__.dm index 97b3ef4fee..128041a6ce 100644 --- a/Content.Tests/DMProject/Tests/Builtins/__IMPLIED_TYPE__.dm +++ b/Content.Tests/DMProject/Tests/Builtins/__IMPLIED_TYPE__.dm @@ -1,7 +1,10 @@ -/datum/test/var/bar = "foobar" +/datum/test /proc/RunTest() var/datum/test/D = __IMPLIED_TYPE__ - ASSERT(D.bar == "foobar") + ASSERT(D == /datum/test) + D = ArgumentTest(__IMPLIED_TYPE__) +/proc/ArgumentTest(some_argument) + ASSERT(some_argument == /datum/test) \ No newline at end of file diff --git a/DMCompiler/DM/Builders/DMExpressionBuilder.cs b/DMCompiler/DM/Builders/DMExpressionBuilder.cs index 6c72adc963..159deccf70 100644 --- a/DMCompiler/DM/Builders/DMExpressionBuilder.cs +++ b/DMCompiler/DM/Builders/DMExpressionBuilder.cs @@ -751,7 +751,7 @@ private DMExpression BuildProcCall(DMASTProcCall procCall, DreamPath? inferredPa } var target = BuildExpression((DMASTExpression)procCall.Callable, inferredPath); - var args = BuildArgumentList(procCall.Location, procCall.Parameters); + var args = BuildArgumentList(procCall.Location, procCall.Parameters, inferredPath); if (target is Proc targetProc) { // GlobalProc handles returnType itself var returnType = targetProc.GetReturnType(ctx.Type); From 939512de195b4e4ded9fa609d06d5fb2fdb6faee Mon Sep 17 00:00:00 2001 From: ike709 Date: Sat, 7 Dec 2024 18:55:26 -0600 Subject: [PATCH 07/25] Don't instantiate multidimensional lists in proc params (#2125) Co-authored-by: ike709 --- .../DMProject/{Broken Tests => Tests}/List/ListNullArg.dm | 2 +- .../DMProject/{Broken Tests => Tests}/List/ListNullArg1.dm | 2 +- .../DMProject/{Broken Tests => Tests}/Stdlib/Array/arg.dm | 0 DMCompiler/Compiler/DM/DMParser.cs | 4 +++- 4 files changed, 5 insertions(+), 3 deletions(-) rename Content.Tests/DMProject/{Broken Tests => Tests}/List/ListNullArg.dm (64%) rename Content.Tests/DMProject/{Broken Tests => Tests}/List/ListNullArg1.dm (65%) rename Content.Tests/DMProject/{Broken Tests => Tests}/Stdlib/Array/arg.dm (100%) diff --git a/Content.Tests/DMProject/Broken Tests/List/ListNullArg.dm b/Content.Tests/DMProject/Tests/List/ListNullArg.dm similarity index 64% rename from Content.Tests/DMProject/Broken Tests/List/ListNullArg.dm rename to Content.Tests/DMProject/Tests/List/ListNullArg.dm index 082351cda2..ccbb0c7643 100644 --- a/Content.Tests/DMProject/Broken Tests/List/ListNullArg.dm +++ b/Content.Tests/DMProject/Tests/List/ListNullArg.dm @@ -1,7 +1,7 @@ // RUNTIME ERROR /proc/ListNullArg2(a[5][3]) - ASSERT(a[1].len == 3) + ASSERT(a[1].len == 3) // a should be null /proc/RunTest() ListNullArg2() diff --git a/Content.Tests/DMProject/Broken Tests/List/ListNullArg1.dm b/Content.Tests/DMProject/Tests/List/ListNullArg1.dm similarity index 65% rename from Content.Tests/DMProject/Broken Tests/List/ListNullArg1.dm rename to Content.Tests/DMProject/Tests/List/ListNullArg1.dm index ade75cf317..9b8360c394 100644 --- a/Content.Tests/DMProject/Broken Tests/List/ListNullArg1.dm +++ b/Content.Tests/DMProject/Tests/List/ListNullArg1.dm @@ -1,7 +1,7 @@ // RUNTIME ERROR /proc/ListNullArg1(a[5]) - ASSERT(a.len == 5) + ASSERT(a.len == 5) // a should be null /proc/RunTest() ListNullArg1() diff --git a/Content.Tests/DMProject/Broken Tests/Stdlib/Array/arg.dm b/Content.Tests/DMProject/Tests/Stdlib/Array/arg.dm similarity index 100% rename from Content.Tests/DMProject/Broken Tests/Stdlib/Array/arg.dm rename to Content.Tests/DMProject/Tests/Stdlib/Array/arg.dm diff --git a/DMCompiler/Compiler/DM/DMParser.cs b/DMCompiler/Compiler/DM/DMParser.cs index 00c566fd16..450d219a09 100644 --- a/DMCompiler/Compiler/DM/DMParser.cs +++ b/DMCompiler/Compiler/DM/DMParser.cs @@ -1690,7 +1690,9 @@ private List DefinitionParameters(out bool wasIndeterm var loc = Current().Location; Whitespace(); - DMASTExpression? value = PathArray(ref path.Path); + PathArray(ref path.Path); + + DMASTExpression? value = null; DMASTExpression? possibleValues = null; if (Check(TokenType.DM_DoubleSquareBracketEquals)) { From 8a7abfc0e771f3f673764b79525aaf844ac3bbac Mon Sep 17 00:00:00 2001 From: ike709 Date: Sat, 7 Dec 2024 19:02:54 -0600 Subject: [PATCH 08/25] Implement `initial()` for `arglist()` (#2123) Co-authored-by: ike709 --- .../DMProject/Broken Tests/Procs/Arglist/initial.dm | 6 ------ Content.Tests/DMProject/Tests/Procs/Arglist/initial.dm | 6 ++++++ DMCompiler/DM/Expressions/Builtins.cs | 7 +++++++ 3 files changed, 13 insertions(+), 6 deletions(-) delete mode 100644 Content.Tests/DMProject/Broken Tests/Procs/Arglist/initial.dm create mode 100644 Content.Tests/DMProject/Tests/Procs/Arglist/initial.dm diff --git a/Content.Tests/DMProject/Broken Tests/Procs/Arglist/initial.dm b/Content.Tests/DMProject/Broken Tests/Procs/Arglist/initial.dm deleted file mode 100644 index 2e77abf518..0000000000 --- a/Content.Tests/DMProject/Broken Tests/Procs/Arglist/initial.dm +++ /dev/null @@ -1,6 +0,0 @@ - -/proc/_initial(...) - return initial(arglist(args)) - -/proc/RunTest() - return \ No newline at end of file diff --git a/Content.Tests/DMProject/Tests/Procs/Arglist/initial.dm b/Content.Tests/DMProject/Tests/Procs/Arglist/initial.dm new file mode 100644 index 0000000000..716becd5d4 --- /dev/null +++ b/Content.Tests/DMProject/Tests/Procs/Arglist/initial.dm @@ -0,0 +1,6 @@ + +/proc/_initial(...) + ASSERT(initial(arglist(args))[1] == "foo") + +/proc/RunTest() + _initial("foo") diff --git a/DMCompiler/DM/Expressions/Builtins.cs b/DMCompiler/DM/Expressions/Builtins.cs index 4b7ec230b6..e11b037cf7 100644 --- a/DMCompiler/DM/Expressions/Builtins.cs +++ b/DMCompiler/DM/Expressions/Builtins.cs @@ -505,6 +505,13 @@ public override void EmitPushValue(ExpressionContext ctx) { return; } + if (Expression is Arglist arglist) { + // This happens silently in BYOND + ctx.Compiler.Emit(WarningCode.PointlessBuiltinCall, Location, "calling initial() on arglist() returns the current value"); + arglist.EmitPushArglist(ctx); + return; + } + ctx.Compiler.Emit(WarningCode.BadArgument, Expression.Location, $"can't get initial value of {Expression}"); ctx.Proc.Error(); } From 621460e3131a5bd0bb40ff2119b1b9b8e784fbfa Mon Sep 17 00:00:00 2001 From: ike709 Date: Sun, 8 Dec 2024 21:36:12 -0600 Subject: [PATCH 09/25] Emit location of original var def for duplicate var errors (#2126) Co-authored-by: ike709 --- DMCompiler/DM/DMCodeTree.Vars.cs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/DMCompiler/DM/DMCodeTree.Vars.cs b/DMCompiler/DM/DMCodeTree.Vars.cs index b259047fb3..3c399703de 100644 --- a/DMCompiler/DM/DMCodeTree.Vars.cs +++ b/DMCompiler/DM/DMCodeTree.Vars.cs @@ -174,9 +174,16 @@ private bool AlreadyExists(DMCompiler compiler, DMObject dmObject) { $"Duplicate definition of static var \"{VarName}\""); return true; } else if (dmObject.HasLocalVariable(VarName)) { - if (!varDef.Location.InDMStandard) // Duplicate instance vars are not an error in DMStandard - compiler.Emit(WarningCode.InvalidVarDefinition, varDef.Location, + if (!varDef.Location.InDMStandard) { // Duplicate instance vars are not an error in DMStandard + var variable = dmObject.GetVariable(VarName); + if(variable!.Value is not null) + compiler.Emit(WarningCode.InvalidVarDefinition, varDef.Location, + $"Duplicate definition of var \"{VarName}\". Previous definition at {variable.Value.Location}"); + else + compiler.Emit(WarningCode.InvalidVarDefinition, varDef.Location, $"Duplicate definition of var \"{VarName}\""); + } + return true; } else if (IsStatic && VarName == "vars" && dmObject == compiler.DMObjectTree.Root) { compiler.Emit(WarningCode.InvalidVarDefinition, varDef.Location, "Duplicate definition of global.vars"); From 54bd2430cf65c0a9703a4a266aba45e088dff304 Mon Sep 17 00:00:00 2001 From: ike709 Date: Sun, 8 Dec 2024 21:48:02 -0600 Subject: [PATCH 10/25] Compile error when outputting to const vars in `for()` loops (#2124) Co-authored-by: ike709 Co-authored-by: wixoa --- .../Statements/For/reuse_decl_const.dm | 2 +- .../Tests/Statements/For/reuse_decl_const2.dm | 18 ++++++++++++++++++ DMCompiler/DM/Builders/DMProcBuilder.cs | 9 ++++++++- DMCompiler/DM/Expressions/LValue.cs | 1 + 4 files changed, 28 insertions(+), 2 deletions(-) rename Content.Tests/DMProject/{Broken Tests => Tests}/Statements/For/reuse_decl_const.dm (89%) create mode 100644 Content.Tests/DMProject/Tests/Statements/For/reuse_decl_const2.dm diff --git a/Content.Tests/DMProject/Broken Tests/Statements/For/reuse_decl_const.dm b/Content.Tests/DMProject/Tests/Statements/For/reuse_decl_const.dm similarity index 89% rename from Content.Tests/DMProject/Broken Tests/Statements/For/reuse_decl_const.dm rename to Content.Tests/DMProject/Tests/Statements/For/reuse_decl_const.dm index 4b404cb05c..2f39dc4f12 100644 --- a/Content.Tests/DMProject/Broken Tests/Statements/For/reuse_decl_const.dm +++ b/Content.Tests/DMProject/Tests/Statements/For/reuse_decl_const.dm @@ -1,4 +1,4 @@ -// COMPILE ERROR +// COMPILE ERROR OD0501 /datum var/const/idx = 0 diff --git a/Content.Tests/DMProject/Tests/Statements/For/reuse_decl_const2.dm b/Content.Tests/DMProject/Tests/Statements/For/reuse_decl_const2.dm new file mode 100644 index 0000000000..a8144a4359 --- /dev/null +++ b/Content.Tests/DMProject/Tests/Statements/For/reuse_decl_const2.dm @@ -0,0 +1,18 @@ +// COMPILE ERROR OD0501 + +/datum + var/const/idx = 0 + var/c = 0 + proc/do_loop() + for (idx in list(1,2,3)) + c += idx + +/proc/RunTest() + var/datum/d = new + d.do_loop() + + var/const/idx = 0 + var/c = 0 + for (idx in list(1,2,3)) + c += idx + diff --git a/DMCompiler/DM/Builders/DMProcBuilder.cs b/DMCompiler/DM/Builders/DMProcBuilder.cs index e4fcd52a2d..38f2c0d729 100644 --- a/DMCompiler/DM/Builders/DMProcBuilder.cs +++ b/DMCompiler/DM/Builders/DMProcBuilder.cs @@ -489,6 +489,10 @@ public void ProcessStatementFor(DMASTProcStatementFor statementFor) { var outputVar = _exprBuilder.Create(outputExpr); + if (outputVar is Local { LocalVar: DMProc.LocalConstVariable } or Field { IsConst: true }) { + compiler.Emit(WarningCode.WriteToConstant, outputExpr.Location, "Cannot change constant value"); + } + var start = _exprBuilder.Create(exprRange.StartRange); var end = _exprBuilder.Create(exprRange.EndRange); var step = exprRange.Step != null @@ -520,7 +524,10 @@ public void ProcessStatementFor(DMASTProcStatementFor statementFor) { if (outputVar is Local outputLocal) { outputLocal.LocalVar.ExplicitValueType = statementFor.DMTypes; - } + if(outputLocal.LocalVar is DMProc.LocalConstVariable) + compiler.Emit(WarningCode.WriteToConstant, outputExpr.Location, "Cannot change constant value"); + } else if (outputVar is Field { IsConst: true }) + compiler.Emit(WarningCode.WriteToConstant, outputExpr.Location, "Cannot change constant value"); ProcessStatementForList(list, outputVar, statementFor.DMTypes, statementFor.Body); break; diff --git a/DMCompiler/DM/Expressions/LValue.cs b/DMCompiler/DM/Expressions/LValue.cs index a32902f4f9..d5fd05f468 100644 --- a/DMCompiler/DM/Expressions/LValue.cs +++ b/DMCompiler/DM/Expressions/LValue.cs @@ -122,6 +122,7 @@ public override void EmitPushInitial(ExpressionContext ctx) { // Identifier of field internal sealed class Field(Location location, DMVariable variable, DMComplexValueType valType) : LValue(location, variable.Type) { + public bool IsConst { get; } = variable.IsConst; public override DMComplexValueType ValType => valType; public override void EmitPushInitial(ExpressionContext ctx) { From ac33d8652d8a137d4fac1fc3ac97405849be3743 Mon Sep 17 00:00:00 2001 From: ike709 Date: Mon, 9 Dec 2024 22:40:33 -0600 Subject: [PATCH 11/25] Fix off-by-one in peephole optimizer (#2131) --- DMCompiler/Optimizer/PeepholeOptimizer.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/DMCompiler/Optimizer/PeepholeOptimizer.cs b/DMCompiler/Optimizer/PeepholeOptimizer.cs index aad7cd7287..4da09e52fb 100644 --- a/DMCompiler/Optimizer/PeepholeOptimizer.cs +++ b/DMCompiler/Optimizer/PeepholeOptimizer.cs @@ -141,7 +141,7 @@ int AttemptCurrentOpt(int i) { var bytecode = input[i]; if (bytecode is not AnnotatedBytecodeInstruction instruction) { i -= AttemptCurrentOpt(i); - i = Math.Max(i, 0); + i = Math.Max(i, -1); // i++ brings -1 back to 0 continue; } @@ -160,7 +160,7 @@ int AttemptCurrentOpt(int i) { } i -= AttemptCurrentOpt(i); - i = Math.Max(i, 0); + i = Math.Max(i, -1); // i++ brings -1 back to 0 } AttemptCurrentOpt(input.Count); From ed5173a04b40d5b5264da92683293fde92363473 Mon Sep 17 00:00:00 2001 From: ike709 Date: Tue, 10 Dec 2024 00:05:27 -0600 Subject: [PATCH 12/25] `IndexRefWithString` peephole opt --- DMCompiler/Bytecode/DreamProcOpcode.cs | 2 ++ DMCompiler/Optimizer/PeepholeOptimizations.cs | 28 +++++++++++++++++++ OpenDreamRuntime/Procs/DMOpcodeHandlers.cs | 11 ++++++++ OpenDreamRuntime/Procs/DMProc.cs | 1 + OpenDreamRuntime/Procs/ProcDecoder.cs | 1 + 5 files changed, 43 insertions(+) diff --git a/DMCompiler/Bytecode/DreamProcOpcode.cs b/DMCompiler/Bytecode/DreamProcOpcode.cs index 3d5d36dfaa..054a27b89c 100644 --- a/DMCompiler/Bytecode/DreamProcOpcode.cs +++ b/DMCompiler/Bytecode/DreamProcOpcode.cs @@ -295,6 +295,8 @@ public enum DreamProcOpcode : byte { ReturnReferenceValue = 0x97, [OpcodeMetadata(0, OpcodeArgType.Float)] ReturnFloat = 0x98, + [OpcodeMetadata(1, OpcodeArgType.Reference, OpcodeArgType.String)] + IndexRefWithString = 0x99, } // ReSharper restore MissingBlankLines diff --git a/DMCompiler/Optimizer/PeepholeOptimizations.cs b/DMCompiler/Optimizer/PeepholeOptimizations.cs index 56f6f4577f..27a9105017 100644 --- a/DMCompiler/Optimizer/PeepholeOptimizations.cs +++ b/DMCompiler/Optimizer/PeepholeOptimizations.cs @@ -116,6 +116,34 @@ public void Apply(DMCompiler compiler, List input, int index } } +// PushReferenceValue [ref] +// PushString [string] +// DereferenceIndex [value] [string] +// -> IndexRefWithString [ref, string] +internal sealed class IndexRefWithString : IOptimization { + public OptPass OptimizationPass => OptPass.PeepholeOptimization; + + public ReadOnlySpan GetOpcodes() { + return [ + DreamProcOpcode.PushReferenceValue, + DreamProcOpcode.PushString, + DreamProcOpcode.DereferenceIndex + ]; + } + + public void Apply(DMCompiler compiler, List input, int index) { + AnnotatedBytecodeInstruction firstInstruction = (AnnotatedBytecodeInstruction)(input[index]); + AnnotatedBytecodeReference pushVal = firstInstruction.GetArg(0); + + AnnotatedBytecodeInstruction secondInstruction = (AnnotatedBytecodeInstruction)(input[index + 1]); + AnnotatedBytecodeString strIndex = secondInstruction.GetArg(0); + + input.RemoveRange(index, 3); + input.Insert(index, new AnnotatedBytecodeInstruction(DreamProcOpcode.IndexRefWithString, -1, + [pushVal, strIndex])); + } +} + // PushReferenceValue [ref] // Return // -> ReturnReferenceValue [ref] diff --git a/OpenDreamRuntime/Procs/DMOpcodeHandlers.cs b/OpenDreamRuntime/Procs/DMOpcodeHandlers.cs index 29f372e2e1..51a05c774d 100644 --- a/OpenDreamRuntime/Procs/DMOpcodeHandlers.cs +++ b/OpenDreamRuntime/Procs/DMOpcodeHandlers.cs @@ -2556,6 +2556,17 @@ public static ProcStatus DereferenceIndex(DMProcState state) { return ProcStatus.Continue; } + public static ProcStatus IndexRefWithString(DMProcState state) { + DreamReference reference = state.ReadReference(); + var refValue = state.GetReferenceValue(reference); + + var index = new DreamValue(state.ReadString()); + var indexResult = state.GetIndex(refValue, index, state); + + state.Push(indexResult); + return ProcStatus.Continue; + } + public static ProcStatus DereferenceCall(DMProcState state) { string name = state.ReadString(); var argumentInfo = state.ReadProcArguments(); diff --git a/OpenDreamRuntime/Procs/DMProc.cs b/OpenDreamRuntime/Procs/DMProc.cs index 1ab4d5c0dd..81f869c924 100644 --- a/OpenDreamRuntime/Procs/DMProc.cs +++ b/OpenDreamRuntime/Procs/DMProc.cs @@ -279,6 +279,7 @@ public sealed class DMProcState : ProcState { {DreamProcOpcode.JumpIfFalseReference, DMOpcodeHandlers.JumpIfFalseReference}, {DreamProcOpcode.DereferenceField, DMOpcodeHandlers.DereferenceField}, {DreamProcOpcode.DereferenceIndex, DMOpcodeHandlers.DereferenceIndex}, + {DreamProcOpcode.IndexRefWithString, DMOpcodeHandlers.IndexRefWithString}, {DreamProcOpcode.DereferenceCall, DMOpcodeHandlers.DereferenceCall}, {DreamProcOpcode.PopReference, DMOpcodeHandlers.PopReference}, {DreamProcOpcode.BitShiftLeftReference,DMOpcodeHandlers.BitShiftLeftReference}, diff --git a/OpenDreamRuntime/Procs/ProcDecoder.cs b/OpenDreamRuntime/Procs/ProcDecoder.cs index 2b0ac676f6..2fd2d2f41f 100644 --- a/OpenDreamRuntime/Procs/ProcDecoder.cs +++ b/OpenDreamRuntime/Procs/ProcDecoder.cs @@ -119,6 +119,7 @@ public ITuple DecodeInstruction() { return (opcode, ReadReference(), ReadReference()); case DreamProcOpcode.PushRefAndDereferenceField: + case DreamProcOpcode.IndexRefWithString: return (opcode, ReadReference(), ReadString()); case DreamProcOpcode.CallStatement: From f6bbf4b9f414b491295b08aa2c9dbb0658573ed0 Mon Sep 17 00:00:00 2001 From: ike709 Date: Tue, 10 Dec 2024 01:09:22 -0600 Subject: [PATCH 13/25] Optimize the peephole optimizer --- DMCompiler/Optimizer/PeepholeOptimizer.cs | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/DMCompiler/Optimizer/PeepholeOptimizer.cs b/DMCompiler/Optimizer/PeepholeOptimizer.cs index 4da09e52fb..42765f6127 100644 --- a/DMCompiler/Optimizer/PeepholeOptimizer.cs +++ b/DMCompiler/Optimizer/PeepholeOptimizer.cs @@ -65,21 +65,17 @@ static PeepholeOptimizer() { /// Setup for each private static void GetOptimizations(DMCompiler compiler) { - var possibleTypes = typeof(IOptimization).Assembly.GetTypes(); - var optimizationTypes = new List(possibleTypes.Length); - - foreach (var type in possibleTypes) { - if (typeof(IOptimization).IsAssignableFrom(type) && type is { IsClass: true, IsAbstract: false }) { - optimizationTypes.Add(type); - } - } + foreach (var optType in typeof(IOptimization).Assembly.GetTypes()) { + if (!typeof(IOptimization).IsAssignableFrom(optType) || + optType is not { IsClass: true, IsAbstract: false }) + continue; - foreach (var optType in optimizationTypes) { var opt = (IOptimization)(Activator.CreateInstance(optType)!); var opcodes = opt.GetOpcodes(); if (opcodes.Length < 2) { - compiler.ForcedError(Location.Internal, $"Peephole optimization {optType} must have at least 2 opcodes"); + compiler.ForcedError(Location.Internal, + $"Peephole optimization {optType} must have at least 2 opcodes"); continue; } From 42ea745e1958a4c4c1803251a500e6aca6315b07 Mon Sep 17 00:00:00 2001 From: ike709 Date: Tue, 10 Dec 2024 01:24:04 -0600 Subject: [PATCH 14/25] stop instantiating bytecode optimizers and remove statics --- DMCompiler/DM/DMProc.cs | 3 +-- DMCompiler/DMCompiler.cs | 3 +++ DMCompiler/Optimizer/BytecodeOptimizer.cs | 14 ++++++++------ DMCompiler/Optimizer/PeepholeOptimizer.cs | 22 ++++++++++++---------- 4 files changed, 24 insertions(+), 18 deletions(-) diff --git a/DMCompiler/DM/DMProc.cs b/DMCompiler/DM/DMProc.cs index 376fc8a2bd..465394322a 100644 --- a/DMCompiler/DM/DMProc.cs +++ b/DMCompiler/DM/DMProc.cs @@ -170,10 +170,9 @@ public void ValidateReturnType(DMExpression expr) { } public ProcDefinitionJson GetJsonRepresentation() { - var optimizer = new BytecodeOptimizer(); var serializer = new AnnotatedBytecodeSerializer(_compiler); - optimizer.Optimize(_compiler, AnnotatedBytecode.GetAnnotatedBytecode()); + _compiler.BytecodeOptimizer.Optimize(AnnotatedBytecode.GetAnnotatedBytecode()); List? arguments = null; if (_parameters.Count > 0) { diff --git a/DMCompiler/DMCompiler.cs b/DMCompiler/DMCompiler.cs index 65cac0c3b9..d386a8e69f 100644 --- a/DMCompiler/DMCompiler.cs +++ b/DMCompiler/DMCompiler.cs @@ -14,6 +14,7 @@ using DMCompiler.Compiler.DM.AST; using DMCompiler.DM.Builders; using DMCompiler.Json; +using DMCompiler.Optimizer; namespace DMCompiler; @@ -31,11 +32,13 @@ public class DMCompiler { internal readonly DMCodeTree DMCodeTree; internal readonly DMObjectTree DMObjectTree; internal readonly DMProc GlobalInitProc; + internal readonly BytecodeOptimizer BytecodeOptimizer; public DMCompiler() { DMCodeTree = new(this); DMObjectTree = new(this); GlobalInitProc = new(this, -1, DMObjectTree.Root, null); + BytecodeOptimizer = new BytecodeOptimizer(this); } public bool Compile(DMCompilerSettings settings) { diff --git a/DMCompiler/Optimizer/BytecodeOptimizer.cs b/DMCompiler/Optimizer/BytecodeOptimizer.cs index 89389db81b..dc1ffb7c7f 100644 --- a/DMCompiler/Optimizer/BytecodeOptimizer.cs +++ b/DMCompiler/Optimizer/BytecodeOptimizer.cs @@ -2,8 +2,10 @@ namespace DMCompiler.Optimizer; -public class BytecodeOptimizer { - internal void Optimize(DMCompiler compiler, List input) { +public class BytecodeOptimizer(DMCompiler compiler) { + private readonly PeepholeOptimizer _peepholeOptimizer = new(compiler); + + internal void Optimize(List input) { if (input.Count == 0) return; @@ -11,10 +13,10 @@ internal void Optimize(DMCompiler compiler, List input) { JoinAndForwardLabels(input); RemoveUnreferencedLabels(input); - PeepholeOptimizer.RunPeephole(compiler, input); + _peepholeOptimizer.RunPeephole(input); } - private static void RemoveUnreferencedLabels(List input) { + private void RemoveUnreferencedLabels(List input) { Dictionary labelReferences = new(); for (int i = 0; i < input.Count; i++) { if (input[i] is AnnotatedBytecodeLabel label) { @@ -38,7 +40,7 @@ private static void RemoveUnreferencedLabels(List input) { } } - private static void JoinAndForwardLabels(List input) { + private void JoinAndForwardLabels(List input) { Dictionary labelAliases = new(); for (int i = 0; i < input.Count; i++) { if (input[i] is AnnotatedBytecodeLabel label) { @@ -74,7 +76,7 @@ private static void JoinAndForwardLabels(List input) { } } - private static bool TryGetLabelName(AnnotatedBytecodeInstruction instruction, [NotNullWhen(true)] out string? labelName) { + private bool TryGetLabelName(AnnotatedBytecodeInstruction instruction, [NotNullWhen(true)] out string? labelName) { foreach (var arg in instruction.GetArgs()) { if (arg is not AnnotatedBytecodeLabel label) continue; diff --git a/DMCompiler/Optimizer/PeepholeOptimizer.cs b/DMCompiler/Optimizer/PeepholeOptimizer.cs index 42765f6127..f16fcfb391 100644 --- a/DMCompiler/Optimizer/PeepholeOptimizer.cs +++ b/DMCompiler/Optimizer/PeepholeOptimizer.cs @@ -40,6 +40,7 @@ internal enum OptPass : byte { // ReSharper disable once ClassNeverInstantiated.Global internal sealed class PeepholeOptimizer { + private readonly DMCompiler _compiler; private class OptimizationTreeEntry { public IOptimization? Optimization; public Dictionary? Children; @@ -48,14 +49,15 @@ private class OptimizationTreeEntry { /// /// The optimization passes in the order that they run /// - private static readonly OptPass[] Passes; + private readonly OptPass[] Passes; /// /// Trees matching chains of opcodes to peephole optimizations /// - private static readonly Dictionary[] OptimizationTrees; + private readonly Dictionary[] OptimizationTrees; - static PeepholeOptimizer() { + public PeepholeOptimizer(DMCompiler compiler) { + _compiler = compiler; Passes = (OptPass[])Enum.GetValues(typeof(OptPass)); OptimizationTrees = new Dictionary[Passes.Length]; for (int i = 0; i < OptimizationTrees.Length; i++) { @@ -64,7 +66,7 @@ static PeepholeOptimizer() { } /// Setup for each - private static void GetOptimizations(DMCompiler compiler) { + private void GetOptimizations() { foreach (var optType in typeof(IOptimization).Assembly.GetTypes()) { if (!typeof(IOptimization).IsAssignableFrom(optType) || optType is not { IsClass: true, IsAbstract: false }) @@ -74,7 +76,7 @@ private static void GetOptimizations(DMCompiler compiler) { var opcodes = opt.GetOpcodes(); if (opcodes.Length < 2) { - compiler.ForcedError(Location.Internal, + _compiler.ForcedError(Location.Internal, $"Peephole optimization {optType} must have at least 2 opcodes"); continue; } @@ -103,14 +105,14 @@ private static void GetOptimizations(DMCompiler compiler) { } } - public static void RunPeephole(DMCompiler compiler, List input) { - GetOptimizations(compiler); + public void RunPeephole(List input) { + GetOptimizations(); foreach (var optPass in Passes) { - RunPass(compiler, (byte)optPass, input); + RunPass((byte)optPass, input); } } - private static void RunPass(DMCompiler compiler, byte pass, List input) { + private void RunPass(byte pass, List input) { OptimizationTreeEntry? currentOpt = null; int optSize = 0; @@ -121,7 +123,7 @@ int AttemptCurrentOpt(int i) { int offset; if (currentOpt.Optimization?.CheckPreconditions(input, i - optSize) is true) { - currentOpt.Optimization.Apply(compiler, input, i - optSize); + currentOpt.Optimization.Apply(_compiler, input, i - optSize); offset = (optSize + 2); // Run over the new opcodes for potential further optimization } else { // This chain of opcodes did not lead to a valid optimization. From 4527ea14514b354570c91b88a4ca048b3de3c645 Mon Sep 17 00:00:00 2001 From: ike709 Date: Tue, 10 Dec 2024 01:53:54 -0600 Subject: [PATCH 15/25] appease resharper --- DMCompiler/Optimizer/PeepholeOptimizer.cs | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/DMCompiler/Optimizer/PeepholeOptimizer.cs b/DMCompiler/Optimizer/PeepholeOptimizer.cs index f16fcfb391..73eb36c776 100644 --- a/DMCompiler/Optimizer/PeepholeOptimizer.cs +++ b/DMCompiler/Optimizer/PeepholeOptimizer.cs @@ -41,6 +41,7 @@ internal enum OptPass : byte { // ReSharper disable once ClassNeverInstantiated.Global internal sealed class PeepholeOptimizer { private readonly DMCompiler _compiler; + private class OptimizationTreeEntry { public IOptimization? Optimization; public Dictionary? Children; @@ -49,23 +50,23 @@ private class OptimizationTreeEntry { /// /// The optimization passes in the order that they run /// - private readonly OptPass[] Passes; + private readonly OptPass[] _passes; /// /// Trees matching chains of opcodes to peephole optimizations /// - private readonly Dictionary[] OptimizationTrees; + private readonly Dictionary[] _optimizationTrees; public PeepholeOptimizer(DMCompiler compiler) { _compiler = compiler; - Passes = (OptPass[])Enum.GetValues(typeof(OptPass)); - OptimizationTrees = new Dictionary[Passes.Length]; - for (int i = 0; i < OptimizationTrees.Length; i++) { - OptimizationTrees[i] = new Dictionary(); + _passes = (OptPass[])Enum.GetValues(typeof(OptPass)); + _optimizationTrees = new Dictionary[_passes.Length]; + for (int i = 0; i < _optimizationTrees.Length; i++) { + _optimizationTrees[i] = new Dictionary(); } } - /// Setup for each + /// Setup for each private void GetOptimizations() { foreach (var optType in typeof(IOptimization).Assembly.GetTypes()) { if (!typeof(IOptimization).IsAssignableFrom(optType) || @@ -81,12 +82,12 @@ private void GetOptimizations() { continue; } - if (!OptimizationTrees[(byte)opt.OptimizationPass].TryGetValue(opcodes[0], out var treeEntry)) { + if (!_optimizationTrees[(byte)opt.OptimizationPass].TryGetValue(opcodes[0], out var treeEntry)) { treeEntry = new() { Children = new() }; - OptimizationTrees[(byte)opt.OptimizationPass].Add(opcodes[0], treeEntry); + _optimizationTrees[(byte)opt.OptimizationPass].Add(opcodes[0], treeEntry); } for (int i = 1; i < opcodes.Length; i++) { @@ -107,7 +108,7 @@ private void GetOptimizations() { public void RunPeephole(List input) { GetOptimizations(); - foreach (var optPass in Passes) { + foreach (var optPass in _passes) { RunPass((byte)optPass, input); } } @@ -147,7 +148,7 @@ int AttemptCurrentOpt(int i) { if (currentOpt == null) { optSize = 1; - OptimizationTrees[pass].TryGetValue(opcode, out currentOpt); + _optimizationTrees[pass].TryGetValue(opcode, out currentOpt); continue; } From 5e627f0c57b2c2e0803e121bedc83325eccf3cb5 Mon Sep 17 00:00:00 2001 From: wixoa Date: Wed, 11 Dec 2024 11:12:26 -0500 Subject: [PATCH 16/25] Update DMCompiler/Optimizer/PeepholeOptimizations.cs --- DMCompiler/Optimizer/PeepholeOptimizations.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DMCompiler/Optimizer/PeepholeOptimizations.cs b/DMCompiler/Optimizer/PeepholeOptimizations.cs index 27a9105017..03802f675c 100644 --- a/DMCompiler/Optimizer/PeepholeOptimizations.cs +++ b/DMCompiler/Optimizer/PeepholeOptimizations.cs @@ -118,7 +118,7 @@ public void Apply(DMCompiler compiler, List input, int index // PushReferenceValue [ref] // PushString [string] -// DereferenceIndex [value] [string] +// DereferenceIndex // -> IndexRefWithString [ref, string] internal sealed class IndexRefWithString : IOptimization { public OptPass OptimizationPass => OptPass.PeepholeOptimization; From 5e0f8f32cf6780e8483858fb91e7337e918a90d0 Mon Sep 17 00:00:00 2001 From: ike709 Date: Wed, 11 Dec 2024 17:05:17 -0600 Subject: [PATCH 17/25] Update DMDisassembler/Program.cs Co-authored-by: wixoa --- DMDisassembler/Program.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/DMDisassembler/Program.cs b/DMDisassembler/Program.cs index d2d517fa9b..921bcdc312 100644 --- a/DMDisassembler/Program.cs +++ b/DMDisassembler/Program.cs @@ -127,8 +127,7 @@ private static void PrintHelp([CanBeNull] string command) { } } - void AllCommands() - { + void AllCommands() { Console.WriteLine("DM Disassembler for OpenDream"); Console.WriteLine("Commands and arguments:"); Console.WriteLine("help [command] : Show additional help for [command] if applicable"); From 4bff0871d25d41b28d28ce01f0854f14deb82de2 Mon Sep 17 00:00:00 2001 From: ike709 Date: Wed, 11 Dec 2024 17:05:22 -0600 Subject: [PATCH 18/25] Update DMDisassembler/Program.cs Co-authored-by: wixoa --- DMDisassembler/Program.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/DMDisassembler/Program.cs b/DMDisassembler/Program.cs index 921bcdc312..456674cee1 100644 --- a/DMDisassembler/Program.cs +++ b/DMDisassembler/Program.cs @@ -172,10 +172,8 @@ void ProcsByType() { } } - var sorted = typeIdToProcCount.OrderByDescending(kvp => kvp.Value).ToList(); Console.WriteLine("Type: Proc Declarations"); - for (int i = 0; i < sorted.Count; i++) { - var pair = sorted[i]; + foreach (var pair in typeIdToProcCount.OrderByDescending(kvp => kvp.Value)) { var type = TypesById[pair.Key]; if (pair.Key == 0) { From 5ead2e30fca5aad5bebe93ac541a91b241efb798 Mon Sep 17 00:00:00 2001 From: ike709 Date: Wed, 11 Dec 2024 17:05:29 -0600 Subject: [PATCH 19/25] Update DMDisassembler/Program.cs Co-authored-by: wixoa --- DMDisassembler/Program.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DMDisassembler/Program.cs b/DMDisassembler/Program.cs index 456674cee1..49dc010c52 100644 --- a/DMDisassembler/Program.cs +++ b/DMDisassembler/Program.cs @@ -131,7 +131,7 @@ void AllCommands() { Console.WriteLine("DM Disassembler for OpenDream"); Console.WriteLine("Commands and arguments:"); Console.WriteLine("help [command] : Show additional help for [command] if applicable"); - Console.WriteLine("quit|q : Exits the disassembler"); + Console.WriteLine("exit|quit|q : Exits the disassembler"); Console.WriteLine("search type|proc [name] : Search for a particular typepath or a proc on a selected type"); Console.WriteLine("select|sel : Select a typepath to run further commands on"); Console.WriteLine("list procs|globals : List all globals, or all procs on a selected type"); From 3877a83fbf8e819c4756b50d15f565f873adf618 Mon Sep 17 00:00:00 2001 From: Penelope Haze Date: Thu, 12 Dec 2024 17:05:08 -0500 Subject: [PATCH 20/25] Fix more cases where inferredPath isn't passed --- DMCompiler/DM/Builders/DMExpressionBuilder.cs | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/DMCompiler/DM/Builders/DMExpressionBuilder.cs b/DMCompiler/DM/Builders/DMExpressionBuilder.cs index 159deccf70..9e25dc50fe 100644 --- a/DMCompiler/DM/Builders/DMExpressionBuilder.cs +++ b/DMCompiler/DM/Builders/DMExpressionBuilder.cs @@ -75,12 +75,12 @@ private DMExpression BuildExpression(DMASTExpression expression, DreamPath? infe case DMASTDereference deref: result = BuildDereference(deref, inferredPath); break; case DMASTLocate locate: result = BuildLocate(locate, inferredPath); break; case DMASTImplicitIsType implicitIsType: result = BuildImplicitIsType(implicitIsType, inferredPath); break; - case DMASTList list: result = BuildList(list); break; + case DMASTList list: result = BuildList(list, inferredPath); break; case DMASTDimensionalList dimensionalList: result = BuildDimensionalList(dimensionalList, inferredPath); break; case DMASTNewList newList: result = BuildNewList(newList, inferredPath); break; case DMASTAddText addText: result = BuildAddText(addText, inferredPath); break; - case DMASTInput input: result = BuildInput(input); break; - case DMASTPick pick: result = BuildPick(pick); break; + case DMASTInput input: result = BuildInput(input, inferredPath); break; + case DMASTPick pick: result = BuildPick(pick, inferredPath); break; case DMASTLog log: result = BuildLog(log, inferredPath); break; case DMASTCall call: result = BuildCall(call, inferredPath); break; case DMASTExpressionWrapped wrapped: result = BuildExpression(wrapped.Value, inferredPath); break; @@ -327,10 +327,10 @@ private DMExpression BuildExpression(DMASTExpression expression, DreamPath? infe break; case DMASTGradient gradient: result = new Gradient(gradient.Location, - BuildArgumentList(gradient.Location, gradient.Parameters)); + BuildArgumentList(gradient.Location, gradient.Parameters, inferredPath)); break; case DMASTRgb rgb: - result = new Rgb(rgb.Location, BuildArgumentList(rgb.Location, rgb.Parameters)); + result = new Rgb(rgb.Location, BuildArgumentList(rgb.Location, rgb.Parameters, inferredPath)); break; case DMASTLocateCoordinates locateCoordinates: result = new LocateCoordinates(locateCoordinates.Location, @@ -435,7 +435,7 @@ private DMExpression BuildExpression(DMASTExpression expression, DreamPath? infe case DMASTVarDeclExpression varDeclExpr: var declIdentifier = new DMASTIdentifier(expression.Location, varDeclExpr.DeclPath.Path.LastElement); - result = BuildIdentifier(declIdentifier); + result = BuildIdentifier(declIdentifier, inferredPath); break; case DMASTVoid: result = BadExpression(WarningCode.BadExpression, expression.Location, "Attempt to use a void expression"); @@ -885,7 +885,7 @@ private DMExpression BuildDereference(DMASTDereference deref, DreamPath? inferre return UnknownReference(callOperation.Location, $"Could not find a global proc named \"{callOperation.Identifier}\""); - var argumentList = BuildArgumentList(deref.Expression.Location, callOperation.Parameters); + var argumentList = BuildArgumentList(deref.Expression.Location, callOperation.Parameters, inferredPath); var globalProcExpr = new GlobalProc(expr.Location, globalProc); expr = new ProcCall(expr.Location, globalProcExpr, argumentList, DMValueType.Anything); @@ -1023,7 +1023,7 @@ private DMExpression BuildDereference(DMASTDereference deref, DreamPath? inferre case DMASTDereference.CallOperation callOperation: { var field = callOperation.Identifier; - var argumentList = BuildArgumentList(deref.Expression.Location, callOperation.Parameters); + var argumentList = BuildArgumentList(deref.Expression.Location, callOperation.Parameters, inferredPath); if (!callOperation.NoSearch && !pathIsFuzzy) { if (prevPath == null) { @@ -1081,7 +1081,7 @@ private DMExpression BuildImplicitIsType(DMASTImplicitIsType isType, DreamPath? return new IsTypeInferred(isType.Location, expr, expr.Path.Value); } - private DMExpression BuildList(DMASTList list) { + private DMExpression BuildList(DMASTList list, DreamPath? inferredPath) { (DMExpression? Key, DMExpression Value)[] values = []; if (list.Values != null) { @@ -1089,8 +1089,8 @@ private DMExpression BuildList(DMASTList list) { for (int i = 0; i < list.Values.Length; i++) { DMASTCallParameter value = list.Values[i]; - DMExpression? key = (value.Key != null) ? BuildExpression(value.Key) : null; - DMExpression listValue = BuildExpression(value.Value); + DMExpression? key = (value.Key != null) ? BuildExpression(value.Key, inferredPath) : null; + DMExpression listValue = BuildExpression(value.Value, inferredPath); values[i] = (key, listValue); } @@ -1151,7 +1151,7 @@ private DMExpression BuildAddText(DMASTAddText addText, DreamPath? inferredPath) return new AddText(addText.Location, expArr); } - private DMExpression BuildInput(DMASTInput input) { + private DMExpression BuildInput(DMASTInput input, DreamPath? inferredPath) { DMExpression[] arguments = new DMExpression[input.Parameters.Length]; for (int i = 0; i < input.Parameters.Length; i++) { DMASTCallParameter parameter = input.Parameters[i]; @@ -1161,12 +1161,12 @@ private DMExpression BuildInput(DMASTInput input) { "input() does not take named arguments"); } - arguments[i] = BuildExpression(parameter.Value); + arguments[i] = BuildExpression(parameter.Value, inferredPath); } DMExpression? list = null; if (input.List != null) { - list = BuildExpression(input.List); + list = BuildExpression(input.List, inferredPath); DMValueType objectTypes = DMValueType.Null |DMValueType.Obj | DMValueType.Mob | DMValueType.Turf | DMValueType.Area; @@ -1188,13 +1188,13 @@ private DMExpression BuildInput(DMASTInput input) { return new Input(input.Location, arguments, input.Types.Value, list); } - private DMExpression BuildPick(DMASTPick pick) { + private DMExpression BuildPick(DMASTPick pick, DreamPath? inferredPath) { Pick.PickValue[] pickValues = new Pick.PickValue[pick.Values.Length]; for (int i = 0; i < pickValues.Length; i++) { DMASTPick.PickValue pickValue = pick.Values[i]; - DMExpression? weight = (pickValue.Weight != null) ? BuildExpression(pickValue.Weight) : null; - DMExpression value = BuildExpression(pickValue.Value); + DMExpression? weight = (pickValue.Weight != null) ? BuildExpression(pickValue.Weight, inferredPath) : null; + DMExpression value = BuildExpression(pickValue.Value, inferredPath); if (weight is Prob prob) // pick(prob(50);x, prob(200);y) format weight = prob.P; From 1c7325162ea5938923a1edebbea5e6db4f4015a7 Mon Sep 17 00:00:00 2001 From: Amy <3855802+amylizzle@users.noreply.github.com> Date: Wed, 18 Dec 2024 19:55:09 +0000 Subject: [PATCH 21/25] Don't run lint on draft PRs (#2140) --- .github/workflows/lint.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index cdc9c261c4..e4d9503b32 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -3,9 +3,11 @@ name: Lint on: pull_request: branches: [master] + types: [opened, synchronize, reopened, ready_for_review] jobs: lint: + if: github.event.pull_request.draft == false runs-on: ubuntu-latest steps: From 64c56845b29a9f2f14dd1ad40da898b573e4ea90 Mon Sep 17 00:00:00 2001 From: ike709 Date: Wed, 18 Dec 2024 13:55:55 -0600 Subject: [PATCH 22/25] Remove `Jump` after `Return` (#2133) Co-authored-by: ike709 --- DMCompiler/Optimizer/PeepholeOptimizations.cs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/DMCompiler/Optimizer/PeepholeOptimizations.cs b/DMCompiler/Optimizer/PeepholeOptimizations.cs index 03802f675c..6b2b9ed3e1 100644 --- a/DMCompiler/Optimizer/PeepholeOptimizations.cs +++ b/DMCompiler/Optimizer/PeepholeOptimizations.cs @@ -214,6 +214,24 @@ public void Apply(DMCompiler compiler, List input, int index } } +// Return +// Jump [label] +// -> Return +internal sealed class RemoveJumpAfterReturn : IOptimization { + public OptPass OptimizationPass => OptPass.PeepholeOptimization; + + public ReadOnlySpan GetOpcodes() { + return [ + DreamProcOpcode.Return, + DreamProcOpcode.Jump + ]; + } + + public void Apply(DMCompiler compiler, List input, int index) { + input.RemoveRange(index + 1, 1); + } +} + // PushFloat [float] // SwitchCase [label] // -> SwitchOnFloat [float] [label] From 5483541b2be22074c591f1097ae28473ee0cc74e Mon Sep 17 00:00:00 2001 From: Amy <3855802+amylizzle@users.noreply.github.com> Date: Wed, 18 Dec 2024 20:06:25 +0000 Subject: [PATCH 23/25] Fix off-by-one in regex replace and add test (#2144) Co-authored-by: amylizzle Co-authored-by: wixoa --- .../Tests/Regex/regex_find_replace.dm | 20 +++++++++++++++++++ .../Procs/Native/DreamProcNativeRegex.cs | 2 +- 2 files changed, 21 insertions(+), 1 deletion(-) create mode 100644 Content.Tests/DMProject/Tests/Regex/regex_find_replace.dm diff --git a/Content.Tests/DMProject/Tests/Regex/regex_find_replace.dm b/Content.Tests/DMProject/Tests/Regex/regex_find_replace.dm new file mode 100644 index 0000000000..d5463d7498 --- /dev/null +++ b/Content.Tests/DMProject/Tests/Regex/regex_find_replace.dm @@ -0,0 +1,20 @@ +var/models = "/obj/cable/brown{\n\ticon_state = \"2-8\"\n\t},\n/obj/cable/brown{\n\ticon_state = \"4-8\"\n\t},\n/turf/simulated/floor/orangeblack,\n/area/station/devzone" +var/result_match = "/obj/cable/brown{\n\ticon_state = \"1\"\n\t},\n/obj/cable/brown{\n\ticon_state = \"2\"\n\t},\n/turf/simulated/floor/orangeblack,\n/area/station/devzone" + +/proc/RunTest() + var/list/originalStrings = list() + var/regex/noStrings = regex(@{"(["])(?:(?=(\\?))\2(.|\n))*?\1"}) + var/stringIndex = 1 + var/found + do + found = noStrings.Find(models, noStrings.next) + if(found) + var indexText = {""[stringIndex]""} + stringIndex++ + var match = copytext(noStrings.match, 2, -1) // Strip quotes + models = noStrings.Replace(models, indexText, found) + originalStrings[indexText] = (match) + while(found) + ASSERT(models == result_match) + ASSERT(originalStrings ~= list("\"1\"" = "2-8", "\"2\"" = "4-8")) + ASSERT(stringIndex == 3) \ No newline at end of file diff --git a/OpenDreamRuntime/Procs/Native/DreamProcNativeRegex.cs b/OpenDreamRuntime/Procs/Native/DreamProcNativeRegex.cs index a06a823d57..09898b5d1d 100644 --- a/OpenDreamRuntime/Procs/Native/DreamProcNativeRegex.cs +++ b/OpenDreamRuntime/Procs/Native/DreamProcNativeRegex.cs @@ -117,7 +117,7 @@ DreamValue DoTextReplace(string replacement) { if(!regex.IsGlobal) { var match = regex.Regex.Match(haystackString, Math.Clamp(start - 1, 0, haystackSubstring.Length)); if (!match.Success) return new DreamValue(haystackString); - regexInstance.SetVariable("next", new DreamValue(match.Index + Math.Max(replacement.Length, 1))); + regexInstance.SetVariable("next", new DreamValue(match.Index + Math.Max(replacement.Length, 1) + 1)); } string replaced = regex.Regex.Replace(haystackSubstring, replacement, regex.IsGlobal ? -1 : 1, From 4b8dc3a197e91b146b28e7a1a5231a8f32c54def Mon Sep 17 00:00:00 2001 From: wixoa Date: Wed, 18 Dec 2024 15:09:00 -0500 Subject: [PATCH 24/25] Implement the `screen_loc` edge anchors (#2106) --- OpenDreamClient/Rendering/DreamViewOverlay.cs | 3 +- OpenDreamShared/Dream/ScreenLocation.cs | 65 +++++++++++++------ 2 files changed, 46 insertions(+), 22 deletions(-) diff --git a/OpenDreamClient/Rendering/DreamViewOverlay.cs b/OpenDreamClient/Rendering/DreamViewOverlay.cs index 55eb129c44..9fa9171912 100644 --- a/OpenDreamClient/Rendering/DreamViewOverlay.cs +++ b/OpenDreamClient/Rendering/DreamViewOverlay.cs @@ -672,7 +672,8 @@ private void CollectVisibleSprites(ViewAlgorithm.Tile?[,] tiles, EntityUid gridU if (sprite.ScreenLocation.MapControl != null) // Don't render screen objects meant for other map controls continue; - Vector2 position = sprite.ScreenLocation.GetViewPosition(worldAABB.BottomLeft, _interfaceManager.View, EyeManager.PixelsPerMeter); + Vector2i dmiIconSize = sprite.Icon.DMI?.IconSize ?? new(EyeManager.PixelsPerMeter, EyeManager.PixelsPerMeter); + Vector2 position = sprite.ScreenLocation.GetViewPosition(worldAABB.BottomLeft, _interfaceManager.View, EyeManager.PixelsPerMeter, dmiIconSize); Vector2 iconSize = sprite.Icon.DMI == null ? Vector2.Zero : sprite.Icon.DMI.IconSize / (float)EyeManager.PixelsPerMeter; for (int x = 0; x < sprite.ScreenLocation.RepeatX; x++) { for (int y = 0; y < sprite.ScreenLocation.RepeatY; y++) { diff --git a/OpenDreamShared/Dream/ScreenLocation.cs b/OpenDreamShared/Dream/ScreenLocation.cs index 27cef2ec09..2b0b13d553 100644 --- a/OpenDreamShared/Dream/ScreenLocation.cs +++ b/OpenDreamShared/Dream/ScreenLocation.cs @@ -6,18 +6,23 @@ using System.Linq; using System.Text; using Robust.Shared.Log; +using Robust.Shared.Maths; namespace OpenDreamShared.Dream; public enum HorizontalAnchor { + West, Left, Center, + East, Right } public enum VerticalAnchor { + South, Bottom, Center, + North, Top } @@ -35,13 +40,13 @@ public sealed class ScreenLocation { private static ISawmill Sawmill => Logger.GetSawmill("opendream.screen_loc_parser"); - private static string[] _keywords = { + private static string[] _keywords = [ "CENTER", "WEST", "EAST", "LEFT", "RIGHT", "NORTH", "SOUTH", "TOP", "BOTTOM", "TOPLEFT", "TOPRIGHT", "BOTTOMLEFT", "BOTTOMRIGHT" - }; + ]; public ScreenLocation(int x, int y, int pixelOffsetX, int pixelOffsetY, ScreenLocation? range = null) { X = x - 1; @@ -66,20 +71,24 @@ public ScreenLocation(string screenLocation) { ParseScreenLoc(screenLocation); } - public Vector2 GetViewPosition(Vector2 viewOffset, ViewRange view, float iconSize) { - float x = (X + PixelOffsetX / iconSize); + public Vector2 GetViewPosition(Vector2 viewOffset, ViewRange view, float tileSize, Vector2i iconSize) { + // TODO: LEFT/RIGHT/TOP/BOTTOM need to stick to the edge of the visible map if the map's container is smaller than the map itself + + float x = (X + PixelOffsetX / tileSize); x += HorizontalAnchor switch { - HorizontalAnchor.Left => 0, + HorizontalAnchor.West or HorizontalAnchor.Left => 0, HorizontalAnchor.Center => view.CenterX, - HorizontalAnchor.Right => view.Width - 1, + HorizontalAnchor.East => view.Width - 1, + HorizontalAnchor.Right => view.Width - (iconSize.X / tileSize), _ => throw new Exception($"Invalid horizontal anchor {HorizontalAnchor}") }; - float y = (Y + PixelOffsetY / iconSize); + float y = (Y + PixelOffsetY / tileSize); y += VerticalAnchor switch { - VerticalAnchor.Bottom => 0, + VerticalAnchor.South or VerticalAnchor.Bottom => 0, VerticalAnchor.Center => view.CenterY, - VerticalAnchor.Top => view.Height - 1, + VerticalAnchor.North => view.Height - 1, + VerticalAnchor.Top => view.Height - (iconSize.Y / tileSize), _ => throw new Exception($"Invalid vertical anchor {VerticalAnchor}") }; @@ -109,7 +118,7 @@ private void ParseScreenLoc(string screenLoc) { if (mapControlSplitIndex > 0) { string mapControl = rangeSplit[0].Substring(0, mapControlSplitIndex); - if (char.IsAsciiLetter(mapControl[0]) && mapControl.IndexOfAny(new[] { '+', '-' }) == -1 && !_keywords.Contains(mapControl)) { + if (char.IsAsciiLetter(mapControl[0]) && mapControl.IndexOfAny(['+', '-']) == -1 && !_keywords.Contains(mapControl)) { MapControl = mapControl; coordinateSplit[0] = coordinateSplit[0].Substring(mapControlSplitIndex + 1); } @@ -121,10 +130,14 @@ private void ParseScreenLoc(string screenLoc) { (HorizontalAnchor, VerticalAnchor) = coordinateSplit[0].Trim() switch { "CENTER" => (HorizontalAnchor.Center, VerticalAnchor.Center), - "NORTHWEST" or "TOPLEFT" => (HorizontalAnchor.Left, VerticalAnchor.Top), - "NORTHEAST" or "TOPRIGHT" => (HorizontalAnchor.Right, VerticalAnchor.Top), - "SOUTHWEST" or "BOTTOMLEFT" => (HorizontalAnchor.Left, VerticalAnchor.Bottom), - "SOUTHEAST" or "BOTTOMRIGHT" => (HorizontalAnchor.Right, VerticalAnchor.Bottom), + "NORTHWEST" => (HorizontalAnchor.West, VerticalAnchor.North), + "TOPLEFT" => (HorizontalAnchor.Left, VerticalAnchor.Top), + "NORTHEAST" => (HorizontalAnchor.East, VerticalAnchor.North), + "TOPRIGHT" => (HorizontalAnchor.Right, VerticalAnchor.Top), + "SOUTHWEST" => (HorizontalAnchor.West, VerticalAnchor.South), + "BOTTOMLEFT" => (HorizontalAnchor.Left, VerticalAnchor.Bottom), + "SOUTHEAST" => (HorizontalAnchor.East, VerticalAnchor.South), + "BOTTOMRIGHT" => (HorizontalAnchor.Right, VerticalAnchor.Bottom), _ => throw new Exception($"Invalid screen_loc {screenLoc}") }; @@ -143,9 +156,7 @@ private bool ParseScreenLocCoordinate(string coordinate, bool isHorizontal) { List pieces = new(); StringBuilder currentPiece = new(); - for (int i = 0; i < coordinate.Length; i++) { - char c = coordinate[i]; - + foreach (var c in coordinate) { switch (c) { case ' ' or '\t': continue; @@ -193,23 +204,35 @@ private bool ParseScreenLocCoordinate(string coordinate, bool isHorizontal) { break; case "WEST": - case "LEFT": // Yes, this sets the horizontal anchor regardless of the isHorizontal arg. // Every macro sets their respective axis regardless of which coordinate it's in - HorizontalAnchor = HorizontalAnchor.Left; + HorizontalAnchor = HorizontalAnchor.West; settingHorizontal = true; break; case "EAST": + HorizontalAnchor = HorizontalAnchor.East; + settingHorizontal = true; + break; + case "NORTH": + VerticalAnchor = VerticalAnchor.North; + settingHorizontal = false; + break; + case "SOUTH": + VerticalAnchor = VerticalAnchor.South; + settingHorizontal = false; + break; + case "LEFT": + HorizontalAnchor = HorizontalAnchor.Left; + settingHorizontal = true; + break; case "RIGHT": HorizontalAnchor = HorizontalAnchor.Right; settingHorizontal = true; break; - case "NORTH": case "TOP": VerticalAnchor = VerticalAnchor.Top; settingHorizontal = false; break; - case "SOUTH": case "BOTTOM": VerticalAnchor = VerticalAnchor.Bottom; settingHorizontal = false; From 2e5ad950bf07245b4cdefa47c740c093b5125eeb Mon Sep 17 00:00:00 2001 From: ike709 Date: Wed, 18 Dec 2024 14:22:59 -0600 Subject: [PATCH 25/25] Fix a bad proc param test (#2113) Co-authored-by: ike709 --- .../Expression/proccall_keyword_param.dm | 13 ------------- .../Tests/Expression/proccall_keyword_param.dm | 11 +++++++++++ 2 files changed, 11 insertions(+), 13 deletions(-) delete mode 100644 Content.Tests/DMProject/Broken Tests/Expression/proccall_keyword_param.dm create mode 100644 Content.Tests/DMProject/Tests/Expression/proccall_keyword_param.dm diff --git a/Content.Tests/DMProject/Broken Tests/Expression/proccall_keyword_param.dm b/Content.Tests/DMProject/Broken Tests/Expression/proccall_keyword_param.dm deleted file mode 100644 index 87f90f6e81..0000000000 --- a/Content.Tests/DMProject/Broken Tests/Expression/proccall_keyword_param.dm +++ /dev/null @@ -1,13 +0,0 @@ - -//# issue 655 -//# issue 265 - -// TODO: We error on this when BYOND doesn't. Revisit this test when we can selectively disable/enable errors with pragmas - -/obj/proc/nullproc(null, temp) - ASSERT(null == 1) - ASSERT(temp == 2) - -/proc/RunTest() - var/obj/o = new - o.nullproc(1,2) diff --git a/Content.Tests/DMProject/Tests/Expression/proccall_keyword_param.dm b/Content.Tests/DMProject/Tests/Expression/proccall_keyword_param.dm new file mode 100644 index 0000000000..89b4065260 --- /dev/null +++ b/Content.Tests/DMProject/Tests/Expression/proccall_keyword_param.dm @@ -0,0 +1,11 @@ + +//# issue 655 +//# issue 265 + +/datum/proc/nullproc(null, temp) + ASSERT(isnull(null)) + ASSERT(temp == 2) + +/proc/RunTest() + var/datum/D = new + D.nullproc(1,2)