From 06ef34cbdf167b1082fedde4caf3c120e7037e17 Mon Sep 17 00:00:00 2001 From: Evil Eye Date: Sat, 23 Aug 2025 17:26:18 +0200 Subject: [PATCH 1/2] Bring messagebox format parsing more in line with printf/vanilla --- components/compiler/lineparser.cpp | 2 +- components/compiler/lineparser.hpp | 2 +- components/interpreter/miscopcodes.hpp | 128 ++++++++++++------------ components/misc/messageformatparser.cpp | 113 ++++++++++++--------- components/misc/messageformatparser.hpp | 27 +++-- 5 files changed, 148 insertions(+), 124 deletions(-) diff --git a/components/compiler/lineparser.cpp b/components/compiler/lineparser.cpp index 90bdac1610..6ea4f9c66a 100644 --- a/components/compiler/lineparser.cpp +++ b/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) { diff --git a/components/compiler/lineparser.hpp b/components/compiler/lineparser.hpp index 68b3198ff2..67ad8e7603 100644 --- a/components/compiler/lineparser.hpp +++ b/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: diff --git a/components/interpreter/miscopcodes.hpp b/components/interpreter/miscopcodes.hpp index 0b49b3c248..c8da745c41 100644 --- a/components/interpreter/miscopcodes.hpp +++ b/components/interpreter/miscopcodes.hpp @@ -2,8 +2,7 @@ #define INTERPRETER_MISCOPCODES_H_INCLUDED #include -#include -#include +#include #include #include @@ -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; - out.fill(padding); - if (width != -1) - out.width(width); - if (precision != -1) - out.precision(precision); + std::string formatString; - 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); - mFormattedMessage += out.str(); - } - break; - case IntegerPlaceholder: + std::string_view value = mRuntime.getStringLiteral(index); + if (precision >= 0) + value = value.substr(0, static_cast(precision)); + if (width < 0) + mFormattedMessage += value; + else { - Type_Integer value = mRuntime[0].mInteger; - mRuntime.pop(); - - out << value; - mFormattedMessage += out.str(); + formatString = "{:"; + if (flags & PrependZero) + formatString += '0'; + if (flags & AlignLeft) + 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(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(); - - if (notation == Notation::Fixed) - { - out << std::fixed << value; - mFormattedMessage += out.str(); - } - else if (notation == Notation::Shortest) - { - 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(); - } + appendMessage(value); + } + else + { + Type_Integer value = mRuntime[0].mInteger; + mRuntime.pop(); + appendMessage(value); } - break; - default: - break; } } diff --git a/components/misc/messageformatparser.cpp b/components/misc/messageformatparser.cpp index 649d0184d3..67ac07011c 100644 --- a/components/misc/messageformatparser.cpp +++ b/components/misc/messageformatparser.cpp @@ -27,58 +27,71 @@ namespace Misc { for (std::size_t i = 0; i < m.size(); ++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 + if (m[i] != '%') { 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]); + } } } } diff --git a/components/misc/messageformatparser.hpp b/components/misc/messageformatparser.hpp index eeef29234d..99a42d2739 100644 --- a/components/misc/messageformatparser.hpp +++ b/components/misc/messageformatparser.hpp @@ -15,17 +15,28 @@ namespace Misc FloatPlaceholder }; - enum class Notation + enum Flags { - Fixed, - Scientific, - Shortest, - HexUpper, - HexLower + None = 0, + PositiveSpace = 1, + PositiveSign = 2, + AlignLeft = 4, + PrependZero = 8, + AlternateForm = 16 }; - virtual void visitedPlaceholder( - Placeholder placeholder, char padding, int width, int precision, Notation notation) + enum class Notation : char + { + 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; From 0ab0e9abd7f72cdedc105f60ef371509ecabdcc5 Mon Sep 17 00:00:00 2001 From: Evil Eye Date: Sat, 23 Aug 2025 19:51:31 +0200 Subject: [PATCH 2/2] Add format tests --- apps/openmw_tests/mwscript/testscripts.cpp | 64 ++++++++++++++++++++++ apps/openmw_tests/mwscript/testutils.hpp | 8 ++- 2 files changed, 71 insertions(+), 1 deletion(-) diff --git a/apps/openmw_tests/mwscript/testscripts.cpp b/apps/openmw_tests/mwscript/testscripts.cpp index b9e422daed..1cc85e40ae 100644 --- a/apps/openmw_tests/mwscript/testscripts.cpp +++ b/apps/openmw_tests/mwscript/testscripts.cpp @@ -1,4 +1,6 @@ #include + +#include #include #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)); diff --git a/apps/openmw_tests/mwscript/testutils.hpp b/apps/openmw_tests/mwscript/testutils.hpp index ba22156f56..8ad571b6ee 100644 --- a/apps/openmw_tests/mwscript/testutils.hpp +++ b/apps/openmw_tests/mwscript/testutils.hpp @@ -145,8 +145,11 @@ namespace { LocalVariables mLocals; std::map mMembers; + std::vector mMessages; public: + const std::vector& 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& buttons) override {} + void messageBox(std::string_view message, const std::vector& buttons) override + { + mMessages.emplace_back(message); + } void report(const std::string& message) override {}