Merge branch 'no_one_likes_commas' into 'master'

Treat commas in scripts as whitespace

Closes #6380

See merge request OpenMW/openmw!1388

CHANGELOG.md

@@ -96,6 +96,7 @@
     Feature #6249: Alpha testing support for Collada
     Feature #6251: OpenMW-CS: Set instance movement based on camera zoom
     Feature #6288: Preserve the "blocked" record flag for referenceable objects.
+    Feature #6380: Commas are treated as whitespace in vanilla
     Task #6201: Remove the "Note: No relevant classes found. No output generated" warnings
     Task #6264: Remove the old classes in animation.cpp
 

apps/openmw_test_suite/mwscript/test_scripts.cpp

@@ -427,6 +427,16 @@ set 1 to 42
 
 End)mwscript";
 
+    const std::string sIssue6380 = R"mwscript(,Begin,issue6380,
+
+,short,a
+
+,set,a,to,,,,(a,+1)
+
+messagebox,"this is a %g",a
+
+,End,)mwscript";
+
     TEST_F(MWScriptTest, mwscript_test_invalid)
     {
         EXPECT_THROW(compile("this is not a valid script", true), Compiler::SourceException);
@@ -831,4 +841,9 @@ End)mwscript";
             FAIL();
         }
     }
+
+    TEST_F(MWScriptTest, mwscript_test_6380)
+    {
+        EXPECT_FALSE(!compile(sIssue6380));
+    }
 }

components/compiler/discardparser.cpp

@@ -12,7 +12,7 @@ namespace Compiler
 
     bool DiscardParser::parseInt (int value, const TokenLoc& loc, Scanner& scanner)
     {
-        if (mState==StartState || mState==CommaState || mState==MinusState)
+        if (mState==StartState || mState==MinusState)
         {
             if (isEmpty())
                 mTokenLoc = loc;
@@ -26,7 +26,7 @@ namespace Compiler
 
     bool DiscardParser::parseFloat (float value, const TokenLoc& loc, Scanner& scanner)
     {
-        if (mState==StartState || mState==CommaState || mState==MinusState)
+        if (mState==StartState || mState==MinusState)
         {
             if (isEmpty())
                 mTokenLoc = loc;
@@ -41,7 +41,7 @@ namespace Compiler
     bool DiscardParser::parseName (const std::string& name, const TokenLoc& loc,
         Scanner& scanner)
     {
-        if (mState==StartState || mState==CommaState)
+        if (mState==StartState)
         {
             if (isEmpty())
                 mTokenLoc = loc;
@@ -55,18 +55,7 @@ namespace Compiler
 
     bool DiscardParser::parseSpecial (int code, const TokenLoc& loc, Scanner& scanner)
     {
-        if (code==Scanner::S_comma && mState==StartState)
+        if (code==Scanner::S_minus && mState==StartState)
-        {
-            if (isEmpty())
-                mTokenLoc = loc;
-
-            start();
-
-            mState = CommaState;
-            return true;
-        }
-
-        if (code==Scanner::S_minus && (mState==StartState || mState==CommaState))
         {
             if (isEmpty())
                 mTokenLoc = loc;

components/compiler/discardparser.hpp

@@ -11,7 +11,7 @@ namespace Compiler
     {
             enum State
             {
-                StartState, CommaState, MinusState
+                StartState, MinusState
             };
 
             State mState;

components/compiler/exprparser.cpp

@@ -244,7 +244,6 @@ namespace Compiler
         }
         else
         {
-            // no comma was used between arguments
             scanner.putbackInt (value, loc);
             return false;
         }
@@ -267,7 +266,6 @@ namespace Compiler
         }
         else
         {
-            // no comma was used between arguments
             scanner.putbackFloat (value, loc);
             return false;
         }
@@ -343,7 +341,6 @@ namespace Compiler
         }
         else
         {
-            // no comma was used between arguments
             scanner.putbackName (name, loc);
             return false;
         }
@@ -452,7 +449,6 @@ namespace Compiler
         }
         else
         {
-            // no comma was used between arguments
             scanner.putbackKeyword (keyword, loc);
             return false;
         }
@@ -487,22 +483,6 @@ namespace Compiler
             return Parser::parseSpecial (code, loc, scanner);
         }
 
-        if (code==Scanner::S_comma)
-        {
-            mTokenLoc = loc;
-
-            if (mFirst)
-            {
-                // leading comma
-                mFirst = false;
-                return true;
-            }
-
-            // end marker
-            scanner.putbackSpecial (code, loc);
-            return false;
-        }
-
         mFirst = false;
 
         if (code==Scanner::S_newline)
@@ -539,7 +519,6 @@ namespace Compiler
             }
             else
             {
-                // no comma was used between arguments
                 scanner.putbackSpecial (code, loc);
                 return false;
             }

components/compiler/fileparser.cpp

@@ -121,11 +121,6 @@ namespace Compiler
                 return false;
             }
         }
-        else if (code==Scanner::S_comma && (mState==NameState || mState==EndNameState))
-        {
-            // ignoring comma (for now)
-            return true;
-        }
 
         return Parser::parseSpecial (code, loc, scanner);
     }

components/compiler/lineparser.cpp

@@ -136,7 +136,7 @@ namespace Compiler
             return false;
         }
 
-        if (mState==MessageState || mState==MessageCommaState)
+        if (mState==MessageState)
         {
             GetArgumentsFromMessageFormat processor;
             processor.process(name);
@@ -155,7 +155,7 @@ namespace Compiler
             return true;
         }
 
-        if (mState==MessageButtonState || mState==MessageButtonCommaState)
+        if (mState==MessageButtonState)
         {
             Generator::pushString (mCode, mLiterals, name);
             mState = MessageButtonState;
@@ -198,7 +198,7 @@ namespace Compiler
 
     bool LineParser::parseKeyword (int keyword, const TokenLoc& loc, Scanner& scanner)
     {
-        if (mState==MessageState || mState==MessageCommaState)
+        if (mState==MessageState)
         {
             if (const Extensions *extensions = getContext().getExtensions())
             {
@@ -446,12 +446,6 @@ namespace Compiler
         if (code==Scanner::S_newline && (mState==EndState || mState==BeginState))
             return false;
 
-        if (code==Scanner::S_comma && mState==MessageState)
-        {
-            mState = MessageCommaState;
-            return true;
-        }
-
         if (code==Scanner::S_ref && mState==SetPotentialMemberVarState)
         {
             getErrorHandler().warning ("Stray explicit reference", loc);
@@ -479,12 +473,6 @@ namespace Compiler
             return false;
         }
 
-        if (code==Scanner::S_comma && mState==MessageButtonState)
-        {
-            mState = MessageButtonCommaState;
-            return true;
-        }
-
         if (code==Scanner::S_member && mState==SetPotentialMemberVarState)
         {
             mState = SetMemberVarState;

components/compiler/lineparser.hpp

@@ -24,7 +24,7 @@ namespace Compiler
                 BeginState,
                 SetState, SetLocalVarState, SetGlobalVarState, SetPotentialMemberVarState,
                 SetMemberVarState, SetMemberVarState2,
-                MessageState, MessageCommaState, MessageButtonState, MessageButtonCommaState,
+                MessageState, MessageButtonState,
                 EndState, PotentialExplicitState, ExplicitState, MemberState
             };
 

components/compiler/scanner.cpp

@@ -531,8 +531,6 @@ namespace Compiler
             else
                 special = S_cmpGT;
         }
-        else if (c==',')
-            special = S_comma;
         else if (c=='+')
             special = S_plus;
         else if (c=='*')
@@ -552,8 +550,6 @@ namespace Compiler
             mTolerantNames = tolerant;
             return out;
         }
-        else if (expectName && special == S_comma)
-            mExpectName = true;
 
         TokenLoc loc (mLoc);
         mLoc.mLiteral.clear();

components/compiler/scanner.hpp

@@ -63,7 +63,7 @@ namespace Compiler
 
         bool isWhitespace()
         {
-            return (mData[0]==' ' || mData[0]=='\t') && mData[1]==0 && mData[2]==0 && mData[3]==0;
+            return (mData[0]==' ' || mData[0]=='\t' || mData[0]==',') && mData[1]==0 && mData[2]==0 && mData[3]==0;
         }
 
         bool isDigit()
@@ -214,7 +214,6 @@ namespace Compiler
                 S_open, S_close,
                 S_cmpEQ, S_cmpNE, S_cmpLT, S_cmpLE, S_cmpGT, S_cmpGE,
                 S_plus, S_minus, S_mult, S_div,
-                S_comma,
                 S_ref,
                 S_member
             };

components/compiler/stringparser.cpp

@@ -13,7 +13,7 @@
 namespace Compiler
 {
     StringParser::StringParser (ErrorHandler& errorHandler, const Context& context, Literals& literals)
-    : Parser (errorHandler, context), mLiterals (literals), mState (StartState), mSmashCase (false), mDiscard (false)
+    : Parser (errorHandler, context), mLiterals (literals), mSmashCase (false), mDiscard (false)
     {
 
     }
@@ -21,23 +21,18 @@ namespace Compiler
     bool StringParser::parseName (const std::string& name, const TokenLoc& loc,
         Scanner& scanner)
     {
-        if (mState==StartState || mState==CommaState)
+        start();
-        {
+        mTokenLoc = loc;
-            start();
-            mTokenLoc = loc;
-
-            if (!mDiscard)
-            {
-                if (mSmashCase)
-                    Generator::pushString (mCode, mLiterals, Misc::StringUtils::lowerCase (name));
-                else
-                    Generator::pushString (mCode, mLiterals, name);
-            }
 
-            return false;
+        if (!mDiscard)
+        {
+            if (mSmashCase)
+                Generator::pushString (mCode, mLiterals, Misc::StringUtils::lowerCase (name));
+            else
+                Generator::pushString (mCode, mLiterals, name);
         }
 
-        return Parser::parseName (name, loc, scanner);
+        return false;
     }
 
     bool StringParser::parseKeyword (int keyword, const TokenLoc& loc, Scanner& scanner)
@@ -75,17 +70,6 @@ namespace Compiler
         return Parser::parseKeyword (keyword, loc, scanner);
     }
 
-    bool StringParser::parseSpecial (int code, const TokenLoc& loc, Scanner& scanner)
-    {
-        if (code==Scanner::S_comma && mState==StartState)
-        {
-            mState = CommaState;
-            return true;
-        }
-
-        return Parser::parseSpecial (code, loc, scanner);
-    }
-
     bool StringParser::parseInt (int value, const TokenLoc& loc, Scanner& scanner)
     {
         reportWarning("Treating integer argument as a string", loc);
@@ -99,7 +83,6 @@ namespace Compiler
 
     void StringParser::reset()
     {
-        mState = StartState;
         mCode.clear();
         mSmashCase = false;
         mTokenLoc = TokenLoc();

components/compiler/stringparser.hpp

@@ -14,13 +14,7 @@ namespace Compiler
 
     class StringParser : public Parser
     {
-            enum State
-            {
-                StartState, CommaState
-            };
-
             Literals& mLiterals;
-            State mState;
             std::vector<Interpreter::Type_Code> mCode;
             bool mSmashCase;
             TokenLoc mTokenLoc;
@@ -39,10 +33,6 @@ namespace Compiler
             ///< Handle a keyword token.
             /// \return fetch another token?
 
-            bool parseSpecial (int code, const TokenLoc& loc, Scanner& scanner) override;
-            ///< Handle a special character token.
-            /// \return fetch another token?
-
             bool parseInt (int value, const TokenLoc& loc, Scanner& scanner) override;
             ///< Handle an int token.
             /// \return fetch another token?