Merge branch 'printf' into 'master'

Bring messagebox format parsing more in line with printf/vanilla

See merge request OpenMW/openmw!4878

apps/openmw_tests/mwscript/testscripts.cpp

@@ -1,4 +1,6 @@
 #include <gtest/gtest.h>
+
+#include <array>
 #include <sstream>
 
 #include "testutils.hpp"
@@ -126,6 +128,31 @@ player -> addSpell "fire_bite", 645
 
 PositionCell "Rabenfels, Taverne" 4480.000 3968.000 15820.000 0
 
+End)mwscript";
+
+    const std::string sScript5 = R"mwscript(Begin messagebox_format_script
+
+float fVal
+
+set fVal to 12.34
+
+MessageBox "hello world"
+MessageBox "%.0f" fVal
+MessageBox "a %03.0f b" fVal
+MessageBox "%+04.0f" fVal
+MessageBox "%+4.0f" fVal
+MessageBox "%+ 4.0f" fVal
+MessageBox "%0+ 4.0f" fVal
+MessageBox "%0+ #4.0f" fVal
+MessageBox "%- 5.0f" fVal
+
+MessageBox "%g" fVal
+MessageBox "%.3g" fVal
+MessageBox "%.5g" fVal
+MessageBox "%#.5g" fVal
+MessageBox "%-5g" fVal
+MessageBox "%- 5g" fVal
+
 End)mwscript";
 
     const std::string sIssue587 = R"mwscript(Begin stalresetScript
@@ -579,6 +606,43 @@ End)mwscript";
         EXPECT_FALSE(!compile(sScript4));
     }
 
+    TEST_F(MWScriptTest, mwscript_test_messagebox_format)
+    {
+        if (const auto script = compile(sScript5))
+        {
+            TestInterpreterContext context;
+            run(*script, context);
+            using std::string_view_literals::operator""sv;
+            constexpr std::array expected{
+                "hello world"sv,
+                "12"sv,
+                "a 012 b"sv,
+                "+012"sv,
+                " +12"sv,
+                " +12"sv,
+                "+012"sv,
+                "+12."sv,
+                " 12  "sv,
+
+                "12.34"sv,
+                "12.3"sv,
+                "12.34"sv,
+                "12.340"sv,
+                "12.34"sv,
+                " 12.34"sv,
+            };
+            for (std::size_t i = 0; i < context.getMessages().size(); i++)
+            {
+                std::string_view message = context.getMessages()[i];
+                EXPECT_EQ(expected.at(i), message);
+            }
+        }
+        else
+        {
+            FAIL();
+        }
+    }
+
     TEST_F(MWScriptTest, mwscript_test_587)
     {
         EXPECT_FALSE(!compile(sIssue587));

apps/openmw_tests/mwscript/testutils.hpp

@@ -145,8 +145,11 @@ namespace
     {
         LocalVariables mLocals;
         std::map<ESM::RefId, GlobalVariables> mMembers;
+        std::vector<std::string> mMessages;
 
     public:
+        const std::vector<std::string>& getMessages() { return mMessages; }
+
         ESM::RefId getTarget() const override { return ESM::RefId(); }
 
         int getLocalShort(int index) const override { return mLocals.getShort(index); }
@@ -161,7 +164,10 @@ namespace
 
         void setLocalFloat(int index, float value) override { mLocals.setFloat(index, value); }
 
-        void messageBox(std::string_view message, const std::vector<std::string>& buttons) override {}
+        void messageBox(std::string_view message, const std::vector<std::string>& buttons) override
+        {
+            mMessages.emplace_back(message);
+        }
 
         void report(const std::string& message) override {}
 

components/compiler/lineparser.cpp

@@ -481,7 +481,7 @@ namespace Compiler
     }
 
     void GetArgumentsFromMessageFormat::visitedPlaceholder(
-        Placeholder placeholder, char /*padding*/, int /*width*/, int /*precision*/, Notation /*notation*/)
+        Placeholder placeholder, int /*flags*/, int /*width*/, int /*precision*/, Notation /*notation*/)
     {
         switch (placeholder)
         {

components/compiler/lineparser.hpp

@@ -88,7 +88,7 @@ namespace Compiler
 
     protected:
         void visitedPlaceholder(
-            Placeholder placeholder, char padding, int width, int precision, Notation notation) override;
+            Placeholder placeholder, int flags, int width, int precision, Notation notation) override;
         void visitedCharacter(char c) override {}
 
     public:

components/interpreter/miscopcodes.hpp

@@ -2,8 +2,7 @@
 #define INTERPRETER_MISCOPCODES_H_INCLUDED
 
 #include <algorithm>
-#include <sstream>
+#include <format>
-#include <stdexcept>
 #include <string>
 #include <vector>
 
@@ -23,78 +22,79 @@ namespace Interpreter
 
     protected:
         void visitedPlaceholder(
-            Placeholder placeholder, char padding, int width, int precision, Notation notation) override
+            Placeholder placeholder, int flags, int width, int precision, Notation notation) override
         {
-            std::ostringstream out;
+            std::string formatString;
-            out.fill(padding);
-            if (width != -1)
-                out.width(width);
-            if (precision != -1)
-                out.precision(precision);
 
-            switch (placeholder)
+            if (placeholder == StringPlaceholder)
             {
-                case StringPlaceholder:
+                int index = mRuntime[0].mInteger;
-                {
+                mRuntime.pop();
-                    int index = mRuntime[0].mInteger;
-                    mRuntime.pop();
 
-                    out << mRuntime.getStringLiteral(index);
+                std::string_view value = mRuntime.getStringLiteral(index);
-                    mFormattedMessage += out.str();
+                if (precision >= 0)
-                }
+                    value = value.substr(0, static_cast<std::size_t>(precision));
-                break;
+                if (width < 0)
-                case IntegerPlaceholder:
+                    mFormattedMessage += value;
+                else
                 {
-                    Type_Integer value = mRuntime[0].mInteger;
+                    formatString = "{:";
-                    mRuntime.pop();
+                    if (flags & PrependZero)
-
+                        formatString += '0';
-                    out << value;
+                    if (flags & AlignLeft)
-                    mFormattedMessage += out.str();
+                        formatString += '<';
+                    else
+                        formatString += '>';
+                    formatString += "{}}";
+                    mFormattedMessage += std::vformat(formatString, std::make_format_args(value, width));
                 }
-                break;
+            }
-                case FloatPlaceholder:
+            else
+            {
+                formatString = "{:";
+                if (flags & AlignLeft)
+                    formatString += '<';
+                if (flags & PositiveSign)
+                    formatString += '+';
+                else if (flags & PositiveSpace)
+                    formatString += ' ';
+                if (flags & AlternateForm)
+                    formatString += '#';
+                if (flags & PrependZero)
+                    formatString += '0';
+                if (width >= 0)
+                    formatString += "{}";
+                if (placeholder == FloatPlaceholder)
+                {
+                    if (precision >= 0)
+                        formatString += ".{}";
+                    formatString += static_cast<char>(notation);
+                }
+                else
+                    precision = -1;
+                formatString += '}';
+                const auto appendMessage = [&](auto value) {
+                    if (width >= 0 && precision >= 0)
+                        mFormattedMessage += std::vformat(formatString, std::make_format_args(value, width, precision));
+                    else if (width >= 0)
+                        mFormattedMessage += std::vformat(formatString, std::make_format_args(value, width));
+                    else if (precision >= 0)
+                        mFormattedMessage += std::vformat(formatString, std::make_format_args(value, precision));
+                    else
+                        mFormattedMessage += std::vformat(formatString, std::make_format_args(value));
+                };
+                if (placeholder == FloatPlaceholder)
                 {
                     float value = mRuntime[0].mFloat;
                     mRuntime.pop();
-
+                    appendMessage(value);
-                    if (notation == Notation::Fixed)
+                }
-                    {
+                else
-                        out << std::fixed << value;
+                {
-                        mFormattedMessage += out.str();
+                    Type_Integer value = mRuntime[0].mInteger;
-                    }
+                    mRuntime.pop();
-                    else if (notation == Notation::Shortest)
+                    appendMessage(value);
-                    {
-                        out << value;
-                        std::string standard = out.str();
-
-                        out.str(std::string());
-                        out.clear();
-
-                        out << std::scientific << value;
-                        std::string scientific = out.str();
-
-                        mFormattedMessage += standard.length() < scientific.length() ? standard : scientific;
-                    }
-                    // TODO switch to std::format so the precision argument applies to these two
-                    else if (notation == Notation::HexLower)
-                    {
-                        out << std::hexfloat << value;
-                        mFormattedMessage += out.str();
-                    }
-                    else if (notation == Notation::HexUpper)
-                    {
-                        out << std::uppercase << std::hexfloat << value;
-                        mFormattedMessage += out.str();
-                    }
-                    else
-                    {
-                        out << std::scientific << value;
-                        mFormattedMessage += out.str();
-                    }
                 }
-                break;
-                default:
-                    break;
             }
         }
 

components/misc/messageformatparser.cpp

@@ -27,58 +27,71 @@ namespace Misc
     {
         for (std::size_t i = 0; i < m.size(); ++i)
         {
-            if (m[i] == '%')
+            if (m[i] != '%')
-            {
-                if (++i < m.size())
-                {
-                    if (m[i] == '%')
-                        visitedCharacter('%');
-                    else
-                    {
-                        char pad = ' ';
-                        if (m[i] == '0' || m[i] == ' ')
-                        {
-                            pad = m[i];
-                            ++i;
-                        }
-
-                        int width = parseNumber(i, m, -1);
-
-                        if (i < m.size())
-                        {
-                            int precision = -1;
-                            if (m[i] == '.')
-                            {
-                                ++i;
-                                precision = parseNumber(i, m, 0);
-                            }
-
-                            if (i < m.size())
-                            {
-                                if (m[i] == 'S' || m[i] == 's')
-                                    visitedPlaceholder(StringPlaceholder, pad, width, precision, Notation::Fixed);
-                                else if (m[i] == 'd' || m[i] == 'i')
-                                    visitedPlaceholder(IntegerPlaceholder, pad, width, precision, Notation::Fixed);
-                                else if (m[i] == 'f' || m[i] == 'F')
-                                    visitedPlaceholder(FloatPlaceholder, pad, width, precision, Notation::Fixed);
-                                else if (m[i] == 'e' || m[i] == 'E')
-                                    visitedPlaceholder(FloatPlaceholder, pad, width, precision, Notation::Scientific);
-                                else if (m[i] == 'g' || m[i] == 'G')
-                                    visitedPlaceholder(FloatPlaceholder, pad, width, precision, Notation::Shortest);
-                                else if (m[i] == 'a')
-                                    visitedPlaceholder(FloatPlaceholder, pad, width, precision, Notation::HexLower);
-                                else if (m[i] == 'A')
-                                    visitedPlaceholder(FloatPlaceholder, pad, width, precision, Notation::HexUpper);
-                                else
-                                    visitedCharacter(m[i]);
-                            }
-                        }
-                    }
-                }
-            }
-            else
             {
                 visitedCharacter(m[i]);
+                continue;
+            }
+            if (++i == m.size())
+                break;
+            if (m[i] == '%')
+            {
+                visitedCharacter('%');
+                continue;
+            }
+
+            int flags = None;
+            while (i < m.size())
+            {
+                if (m[i] == '-')
+                    flags |= AlignLeft;
+                else if (m[i] == '+')
+                    flags |= PositiveSign;
+                else if (m[i] == ' ')
+                    flags |= PositiveSpace;
+                else if (m[i] == '0')
+                    flags |= PrependZero;
+                else if (m[i] == '#')
+                    flags |= AlternateForm;
+                else
+                    break;
+                ++i;
+            }
+
+            int width = parseNumber(i, m, -1);
+
+            if (i < m.size())
+            {
+                int precision = -1;
+                if (m[i] == '.')
+                {
+                    ++i;
+                    precision = parseNumber(i, m, 0);
+                }
+
+                if (i < m.size())
+                {
+                    if (m[i] == 'S' || m[i] == 's')
+                        visitedPlaceholder(StringPlaceholder, flags, width, precision, Notation::Fixed);
+                    else if (m[i] == 'd' || m[i] == 'i')
+                        visitedPlaceholder(IntegerPlaceholder, flags, width, precision, Notation::Fixed);
+                    else if (m[i] == 'f' || m[i] == 'F')
+                        visitedPlaceholder(FloatPlaceholder, flags, width, precision, Notation::Fixed);
+                    else if (m[i] == 'e')
+                        visitedPlaceholder(FloatPlaceholder, flags, width, precision, Notation::ScientificLower);
+                    else if (m[i] == 'E')
+                        visitedPlaceholder(FloatPlaceholder, flags, width, precision, Notation::ScientificUpper);
+                    else if (m[i] == 'g')
+                        visitedPlaceholder(FloatPlaceholder, flags, width, precision, Notation::ShortestLower);
+                    else if (m[i] == 'G')
+                        visitedPlaceholder(FloatPlaceholder, flags, width, precision, Notation::ShortestUpper);
+                    else if (m[i] == 'a')
+                        visitedPlaceholder(FloatPlaceholder, flags, width, precision, Notation::HexLower);
+                    else if (m[i] == 'A')
+                        visitedPlaceholder(FloatPlaceholder, flags, width, precision, Notation::HexUpper);
+                    else
+                        visitedCharacter(m[i]);
+                }
             }
         }
     }

components/misc/messageformatparser.hpp

@@ -15,17 +15,28 @@ namespace Misc
             FloatPlaceholder
         };
 
-        enum class Notation
+        enum Flags
         {
-            Fixed,
+            None = 0,
-            Scientific,
+            PositiveSpace = 1,
-            Shortest,
+            PositiveSign = 2,
-            HexUpper,
+            AlignLeft = 4,
-            HexLower
+            PrependZero = 8,
+            AlternateForm = 16
         };
 
-        virtual void visitedPlaceholder(
+        enum class Notation : char
-            Placeholder placeholder, char padding, int width, int precision, Notation notation)
+        {
+            Fixed = 'f',
+            ScientificUpper = 'E',
+            ScientificLower = 'e',
+            ShortestUpper = 'G',
+            ShortestLower = 'g',
+            HexUpper = 'A',
+            HexLower = 'a'
+        };
+
+        virtual void visitedPlaceholder(Placeholder placeholder, int flags, int width, int precision, Notation notation)
             = 0;
         virtual void visitedCharacter(char c) = 0;