Merge branch 'master' into opcodes-unique

Content.Tests/DMProject/Tests/Statements/Switch/weird_when_clause.dm

@@ -0,0 +1,15 @@
+
+//Issue OD#996, kinda: https://github.com/OpenDreamProject/OpenDream/issues/996
+
+/proc/RunTest()
+	var/x = 5
+	switch(x)
+		if(1)
+			CRASH("Strange branch chosen in switch statement")
+		if(4)
+			CRASH("Strange branch chosen in switch statement")
+		else if(x == 3)
+			CRASH("Parser failed to understand 'else if' in switch block")
+		else
+			return
+	CRASH("Parser failed to understand 'else if' in switch block")

Content.Tests/DMProject/Tests/Statements/Switch/weird_when_clause_pragma.dm

@@ -0,0 +1,16 @@
+//COMPILE ERROR
+//test to make sure SuspiciousSwitchCase is working
+
+#pragma SuspiciousSwitchCase error
+
+/proc/RunTest()
+	var/x = 5
+	switch(x)
+		if(1)
+			return
+		if(4)
+			return
+		else if(x == 3)
+			return
+		else
+			return

DMCompiler/Compiler/DM/DMParser.cs

@@ -8,14 +8,9 @@
 
 namespace DMCompiler.Compiler.DM {
     public partial class DMParser : Parser<Token> {
-
         private DreamPath _currentPath = DreamPath.Root;
-
         private bool _allowVarDeclExpression = false;
 
-        public DMParser(DMLexer lexer) : base(lexer) {
-        }
-
         private static readonly TokenType[] AssignTypes = {
             TokenType.DM_Equals,
             TokenType.DM_PlusEquals,
@@ -149,6 +144,9 @@ public DMParser(DMLexer lexer) : base(lexer) {
             TokenType.DM_XorEquals,
         };
 
+        public DMParser(DMLexer lexer) : base(lexer) {
+        }
+
         public DMASTFile File() {
             var loc = Current().Location;
             List<DMASTStatement> statements = new();
@@ -1301,7 +1299,7 @@ DMASTProcBlockInner GetForBody() {
 
                 DMASTProcStatementSwitch.SwitchCase[]? switchCases = SwitchCases();
 
-                if (switchCases == null) Error("Expected switch cases");
+                if (switchCases == null) Error(WarningCode.BadExpression, "Expected switch cases");
                 return new DMASTProcStatementSwitch(loc, value, switchCases);
             }
 
@@ -1352,7 +1350,7 @@ public DMASTProcStatementSwitch.SwitchCase[] SwitchInner() {
             List<DMASTProcStatementSwitch.SwitchCase> switchCases = new();
             DMASTProcStatementSwitch.SwitchCase? switchCase = SwitchCase();
 
-            while (switchCase != null) {
+            while (switchCase is not null) {
                 switchCases.Add(switchCase);
                 Newline();
                 Whitespace();
@@ -1374,19 +1372,20 @@ public DMASTProcStatementSwitch.SwitchCase[] SwitchInner() {
 
                     DMASTExpression? expression = Expression();
                     if (expression == null) {
-                        if (expressions.Count == 0) {
-                            Error("Expected an expression");
-                        } else {
-                            //Eat a trailing comma if there's at least 1 expression
-                            break;
-                        }
+                        if (expressions.Count == 0)
+                            DMCompiler.Emit(WarningCode.BadExpression, Current().Location, "Expected an expression");
+
+                        break;
                     }
 
                     if (Check(TokenType.DM_To)) {
-                        var loc = Current().Location;
                         Whitespace();
+                        var loc = Current().Location;
                         DMASTExpression? rangeEnd = Expression();
-                        if (rangeEnd == null) Error("Expected an upper limit");
+                        if (rangeEnd == null) {
+                            DMCompiler.Emit(WarningCode.BadExpression, loc, "Expected an upper limit");
+                            rangeEnd = new DMASTConstantNull(loc); // Fallback to null
+                        }
 
                         expressions.Add(new DMASTSwitchCaseRange(loc, expression, rangeEnd));
                     } else {
@@ -1402,22 +1401,25 @@ public DMASTProcStatementSwitch.SwitchCase[] SwitchInner() {
 
                 if (body == null) {
                     DMASTProcStatement? statement = ProcStatement();
-                    var loc = Current().Location;
 
                     if (statement != null) {
-                        body = new DMASTProcBlockInner(loc,statement);
+                        body = new DMASTProcBlockInner(statement.Location, statement);
                     } else {
-                        body = new DMASTProcBlockInner(loc);
+                        body = new DMASTProcBlockInner(Current().Location);
                     }
                 }
 
                 return new DMASTProcStatementSwitch.SwitchCaseValues(expressions.ToArray(), body);
             } else if (Check(TokenType.DM_Else)) {
-                var loc = Current().Location;
                 Whitespace();
+                var loc = Current().Location;
                 if (Current().Type == TokenType.DM_If) {
-                    Error("Expected \"if\" or \"else\", \"else if\" is not permitted as a switch case");
+                    //From now on, all ifs/elseifs/elses are actually part of this if's chain, not the switch's.
+                    //Ambiguous, but that is parity behaviour. Ergo, the following emission.
+                    DMCompiler.Emit(WarningCode.SuspiciousSwitchCase, loc,
+                        "Expected \"if\" or \"else\" - \"else if\" is ambiguous as a switch case and may cause unintended flow");
                 }
+
                 DMASTProcBlockInner? body = ProcBlock();
 
                 if (body == null) {

DMCompiler/Compiler/DM/DMParserHelper.cs

@@ -1,14 +1,12 @@
 using OpenDreamShared.Compiler;
-using DMCompiler.Compiler.DMPreprocessor;
 using System;
 using System.Collections.Generic;
 using System.Linq;
 using System.Text;
 using DMCompiler.Bytecode;
 
 namespace DMCompiler.Compiler.DM {
-    public partial class DMParser : Parser<Token> {
-
+    public partial class DMParser {
         /// <summary>
         /// A special override of Error() since, for DMParser, we know we are in a compilation context and can make use of error codes.
         /// </summary>
@@ -23,6 +21,12 @@ protected bool Error(WarningCode code, string message) {
             return level == ErrorLevel.Error;
         }
 
+        /// <inheritdoc cref="Parser{SourceType}.Error(string, bool)"/>
+        [Obsolete("This is not a desirable way for DMParser to emit an error, as errors should emit an error code and not cause unnecessary throws. Use DMParser's overrides of this method, instead.")]
+        new protected void Error(string message, bool throwException = true) {
+            base.Error(message, throwException);
+        }
+
         protected bool PeekDelimiter() {
             return Current().Type == TokenType.Newline || Current().Type == TokenType.DM_Semicolon;
         }

DMCompiler/DMStandard/DefaultPragmaConfig.dm

@@ -27,6 +27,7 @@
 #pragma DanglingVarType warning
 #pragma MissingInterpolatedExpression warning
 #pragma AmbiguousResourcePath warning
+#pragma SuspiciousSwitchCase warning
 
 //3000-3999
 #pragma EmptyBlock notice

OpenDreamClient/States/DreamUserInterfaceStateManager.cs

@@ -31,8 +31,14 @@ public void Initialize() {
                 // When we disconnect from the server:
                 case ClientRunLevel.Error:
                 case ClientRunLevel.Initialize when args.OldLevel >= ClientRunLevel.Connected:
-                    if (_gameController.LaunchState.FromLauncher) {
-                        _stateManager.RequestStateChange<ConnectingState>();
+                    // TODO: Reconnect without returning to the launcher
+                    // The client currently believes its still connected at this point and will refuse
+                    if (_gameController.LaunchState is {
+                            FromLauncher: true,
+                            Ss14Address: not null
+                        }) {
+                        _gameController.Redial(_gameController.LaunchState.Ss14Address, "Connection lost; attempting reconnect");
+
                         break;
                     }
 

OpenDreamRuntime/AtomManager.cs

@@ -192,13 +192,6 @@ public EntityUid CreateMovableEntity(DreamObjectMovable movable) {
         DMISpriteComponent sprite = _entityManager.AddComponent<DMISpriteComponent>(entity);
         sprite.SetAppearance(GetAppearanceFromDefinition(movable.ObjectDefinition));
 
-        if (_entityManager.TryGetComponent(entity, out MetaDataComponent? metaData)) {
-            _metaDataSystem ??= _entitySystemManager.GetEntitySystem<MetaDataSystem>();
-
-            _metaDataSystem.SetEntityName(entity, movable.GetDisplayName(), metaData);
-            _metaDataSystem.SetEntityDescription(entity, movable.Desc ?? string.Empty, metaData);
-        }
-
         _entityToAtom.Add(entity, movable);
         return entity;
     }

OpenDreamRuntime/Objects/DreamObjectTree.cs

@@ -76,6 +76,7 @@ public void LoadJson(DreamCompiledJson json) {
         _entitySystemManager.TryGetEntitySystem(out _appearanceSystem);
         _entitySystemManager.TryGetEntitySystem(out _transformSystem);
         _entitySystemManager.TryGetEntitySystem(out _pvsOverrideSystem);
+        _entitySystemManager.TryGetEntitySystem(out _metaDataSystem);
 
         Strings = json.Strings ?? new();
 

OpenDreamRuntime/Objects/Types/DreamObjectAtom.cs

@@ -1,4 +1,5 @@
-using OpenDreamShared.Dream;
+using OpenDreamRuntime.Procs;
+using OpenDreamShared.Dream;
 
 namespace OpenDreamRuntime.Objects.Types;
 
@@ -14,9 +15,6 @@ public class DreamObjectAtom : DreamObject {
     public DreamList? VisLocs; // TODO: Implement
 
     public DreamObjectAtom(DreamObjectDefinition objectDefinition) : base(objectDefinition) {
-        ObjectDefinition.Variables["name"].TryGetValueAsString(out Name);
-        ObjectDefinition.Variables["desc"].TryGetValueAsString(out Desc);
-
         Overlays = new(ObjectTree.List.ObjectDefinition, this, AppearanceSystem, false);
         Underlays = new(ObjectTree.List.ObjectDefinition, this, AppearanceSystem, true);
         VisContents = new(ObjectTree.List.ObjectDefinition, PvsOverrideSystem, this);
@@ -26,6 +24,11 @@ public DreamObjectAtom(DreamObjectDefinition objectDefinition) : base(objectDefi
         AtomManager.AddAtom(this);
     }
 
+    public override void Initialize(DreamProcArguments args) {
+        ObjectDefinition.Variables["name"].TryGetValueAsString(out Name);
+        ObjectDefinition.Variables["desc"].TryGetValueAsString(out Desc);
+    }
+
     protected override void HandleDeletion() {
         AtomManager.RemoveAtom(this);
 

OpenDreamRuntime/Objects/Types/DreamObjectMovable.cs

@@ -2,7 +2,6 @@
 using OpenDreamRuntime.Rendering;
 using OpenDreamShared.Dream;
 using Robust.Shared.Map;
-using Robust.Shared.Utility;
 
 namespace OpenDreamRuntime.Objects.Types;
 
@@ -40,14 +39,19 @@ public DreamObjectMovable(DreamObjectDefinition objectDefinition) : base(objectD
         Entity = AtomManager.CreateMovableEntity(this);
         SpriteComponent = EntityManager.GetComponent<DMISpriteComponent>(Entity);
         _transformComponent = EntityManager.GetComponent<TransformComponent>(Entity);
-
-        objectDefinition.Variables["screen_loc"].TryGetValueAsString(out var screenLoc);
-        ScreenLoc = screenLoc;
     }
 
     public override void Initialize(DreamProcArguments args) {
         base.Initialize(args);
 
+        ObjectDefinition.Variables["screen_loc"].TryGetValueAsString(out var screenLoc);
+        ScreenLoc = screenLoc;
+
+        if (EntityManager.TryGetComponent(Entity, out MetaDataComponent? metaData)) {
+            MetaDataSystem?.SetEntityName(Entity, GetDisplayName(), metaData);
+            MetaDataSystem?.SetEntityDescription(Entity, Desc ?? string.Empty, metaData);
+        }
+
         args.GetArgument(0).TryGetValueAsDreamObject<DreamObjectAtom>(out var loc);
         SetLoc(loc); //loc is set before /New() is ever called
     }

OpenDreamShared/Compiler/CompilerError.cs

@@ -57,7 +57,8 @@ public enum WarningCode {
         // 3000 - 3999 are reserved for stylistic configuration.
         EmptyBlock = 3100,
         EmptyProc = 3101,
-        UnsafeClientAccess = 3200
+        UnsafeClientAccess = 3200,
+        SuspiciousSwitchCase = 3201, // "else if" cases are actually valid DM, they just spontaneously end the switch context and begin an if-else ladder within the else case of the switch
 
         // 4000 - 4999 are reserved for runtime configuration. (TODO: Runtime doesn't know about configs yet!)
     }