fix(menu): allow IW4 columns without alignment (#932)

* fix(menu): allow IW4 columns without alignment

* feat: also make alignment optional for iw5 menus

* chore: move menu sequences to anonymous namespace if possible

---------

Co-authored-by: Jan Laupetin <[email protected]>

src/ObjLoading/Parsing/Menu/Sequence/EventHandlerSetScopeSequences.cpp

@@ -14,7 +14,7 @@
 
 using namespace menu;
 
-namespace menu
+namespace
 {
     enum class ExpectedScriptToken
     {
@@ -195,14 +195,9 @@ namespace menu
             });
         }
     };
-} // namespace menu
 
-namespace menu::event_handler_set_scope_sequences
-{
     class SequenceCloseBlock final : public MenuFileParser::sequence_t
     {
-        static constexpr auto CAPTURE_TOKEN = 1;
-
     public:
         SequenceCloseBlock()
         {
@@ -922,9 +917,7 @@ namespace menu::event_handler_set_scope_sequences
             });
         }
     };
-} // namespace menu::event_handler_set_scope_sequences
+} // namespace
-
-using namespace event_handler_set_scope_sequences;
 
 EventHandlerSetScopeSequences::EventHandlerSetScopeSequences(std::vector<std::unique_ptr<MenuFileParser::sequence_t>>& allSequences,
                                                              std::vector<MenuFileParser::sequence_t*>& scopeSequences)

src/ObjLoading/Parsing/Menu/Sequence/FunctionScopeSequences.cpp

@@ -9,7 +9,7 @@
 
 using namespace menu;
 
-namespace menu::function_scope_sequences
+namespace
 {
     class SequenceCloseBlock final : public MenuFileParser::sequence_t
     {
@@ -69,9 +69,7 @@ namespace menu::function_scope_sequences
             }
         }
     };
-} // namespace menu::function_scope_sequences
+} // namespace
-
-using namespace function_scope_sequences;
 
 FunctionScopeSequences::FunctionScopeSequences(std::vector<std::unique_ptr<MenuFileParser::sequence_t>>& allSequences,
                                                std::vector<MenuFileParser::sequence_t*>& scopeSequences)
@@ -79,7 +77,7 @@ FunctionScopeSequences::FunctionScopeSequences(std::vector<std::unique_ptr<MenuF
 {
 }
 
-void FunctionScopeSequences::AddSequences(FeatureLevel featureLevel, bool permissive) const
+void FunctionScopeSequences::AddSequences(const FeatureLevel featureLevel, const bool permissive) const
 {
     AddSequence(std::make_unique<SequenceCloseBlock>());
     AddSequence(std::make_unique<GenericStringPropertySequence>("name",

src/ObjLoading/Parsing/Menu/Sequence/GlobalScopeSequences.cpp

@@ -5,7 +5,7 @@
 
 using namespace menu;
 
-namespace menu::global_scope_sequences
+namespace
 {
     class SequenceCloseBlock final : public MenuFileParser::sequence_t
     {
@@ -98,9 +98,7 @@ namespace menu::global_scope_sequences
             state->m_menus_to_load.emplace_back(menuNameToken.StringValue());
         }
     };
-} // namespace menu::global_scope_sequences
+} // namespace
-
-using namespace global_scope_sequences;
 
 GlobalScopeSequences::GlobalScopeSequences(std::vector<std::unique_ptr<MenuFileParser::sequence_t>>& allSequences,
                                            std::vector<MenuFileParser::sequence_t*>& scopeSequences)
@@ -108,7 +106,7 @@ GlobalScopeSequences::GlobalScopeSequences(std::vector<std::unique_ptr<MenuFileP
 {
 }
 
-void GlobalScopeSequences::AddSequences(FeatureLevel featureLevel, bool permissive) const
+void GlobalScopeSequences::AddSequences(const FeatureLevel featureLevel, const bool permissive) const
 {
     AddSequence(std::make_unique<SequenceCloseBlock>());
     AddSequence(std::make_unique<SequenceFunctionDef>());

src/ObjLoading/Parsing/Menu/Sequence/ItemScopeSequences.cpp

@@ -12,11 +12,14 @@
 #include "Parsing/Menu/MenuFileCommonOperations.h"
 
 #include <cassert>
+#include <format>
 #include <string>
 #include <vector>
 
 using namespace menu;
 
+namespace
+{
     constexpr auto ITEM_TYPE_OWNERDRAW = 8;
 
     class ItemScopeOperations
@@ -152,8 +155,6 @@ public:
         }
     };
 
-namespace menu::item_scope_sequences
-{
     class SequenceCloseBlock final : public MenuFileParser::sequence_t
     {
     public:
@@ -449,94 +450,106 @@ namespace menu::item_scope_sequences
 
     class SequenceColumns final : public MenuFileParser::sequence_t
     {
-        static constexpr auto TAG_COLUMN = 1;
+        static constexpr auto TAG_VALUE = 1;
 
         static constexpr auto CAPTURE_FIRST_TOKEN = 1;
         static constexpr auto CAPTURE_COLUMN_COUNT = 2;
 
+        static constexpr const char* SYNTAX_IW4 = "<xpos> <width> <maxChars> [alignment]";
+        static constexpr auto BASE_COLUMN_COUNT_IW4 = 3;
+
+        static constexpr const char* SYNTAX_IW5 = "<xpos> <ypos> <width> <height> <maxChars> [alignment]";
+        static constexpr auto BASE_COLUMN_COUNT_IW5 = 5;
+
     public:
-        explicit SequenceColumns(const FeatureLevel featureLevel)
+        explicit SequenceColumns()
         {
             const MenuMatcherFactory create(this);
             AddLabeledMatchers(MenuExpressionMatchers().Expression(this), MenuExpressionMatchers::LABEL_EXPRESSION);
 
-            if (featureLevel == FeatureLevel::IW5)
-            {
             AddMatchers({
                 create.KeywordIgnoreCase("columns").Capture(CAPTURE_FIRST_TOKEN),
                 create.Integer().Capture(CAPTURE_COLUMN_COUNT),
-                    create.Loop(create.And({
+                create.Loop(create.IntExpression().Tag(TAG_VALUE)),
-                        create.True().Tag(TAG_COLUMN),
-                        create.IntExpression(), // xpos
-                        create.IntExpression(), // ypos
-                        create.IntExpression(), // width
-                        create.IntExpression(), // height
-                        create.IntExpression(), // maxChars
-                        create.IntExpression(), // alignment
-                    })),
             });
         }
-            else
-            {
-                AddMatchers({
-                    create.KeywordIgnoreCase("columns").Capture(CAPTURE_FIRST_TOKEN),
-                    create.Integer().Capture(CAPTURE_COLUMN_COUNT),
-                    create.Loop(create.And({
-                        create.True().Tag(TAG_COLUMN),
-                        create.IntExpression(), // xpos
-                        create.IntExpression(), // width
-                        create.IntExpression(), // maxChars
-                        create.IntExpression(), // alignment
-                    })),
-                });
-            }
-        }
 
     protected:
         void ProcessMatch(MenuFileParserState* state, SequenceResult<SimpleParserValue>& result) const override
         {
             assert(state->m_current_item);
-
-            ItemScopeOperations::EnsureHasListboxFeatures(*state->m_current_item, result.NextCapture(CAPTURE_FIRST_TOKEN).GetPos());
-
             assert(state->m_feature_level == FeatureLevel::IW4 || state->m_feature_level == FeatureLevel::IW5);
 
-            const auto& listBoxFeatures = state->m_current_item->m_list_box_features;
+            const auto& firstToken = result.NextCapture(CAPTURE_FIRST_TOKEN);
-            while (result.PeekAndRemoveIfTag(TAG_COLUMN) == TAG_COLUMN)
+            ItemScopeOperations::EnsureHasListboxFeatures(*state->m_current_item, firstToken.GetPos());
-            {
-                int xPos = 0;
-                int yPos = 0;
-                int width = 0;
-                int height = 0;
-                int maxChars = 0;
-                int alignment = 0;
 
+            const auto columnCount = static_cast<size_t>(result.NextCapture(CAPTURE_COLUMN_COUNT).IntegerValue());
+            std::vector<int> columnValues;
+            while (result.PeekAndRemoveIfTag(TAG_VALUE) == TAG_VALUE)
+                columnValues.emplace_back(MenuMatcherFactory::TokenIntExpressionValue(state, result));
+
+            size_t baseColumnCount;
+            const char* syntax;
+            bool hasHeightValues;
             if (state->m_feature_level == FeatureLevel::IW4)
             {
-                    xPos = MenuMatcherFactory::TokenIntExpressionValue(state, result);
+                baseColumnCount = BASE_COLUMN_COUNT_IW4;
-                    width = MenuMatcherFactory::TokenIntExpressionValue(state, result);
+                syntax = SYNTAX_IW4;
-                    maxChars = MenuMatcherFactory::TokenIntExpressionValue(state, result);
+                hasHeightValues = false;
-                    alignment = MenuMatcherFactory::TokenIntExpressionValue(state, result);
             }
-                else if (state->m_feature_level == FeatureLevel::IW5)
+            else
             {
-                    xPos = MenuMatcherFactory::TokenIntExpressionValue(state, result);
+                assert(state->m_feature_level == FeatureLevel::IW5);
-                    yPos = MenuMatcherFactory::TokenIntExpressionValue(state, result);
+                baseColumnCount = BASE_COLUMN_COUNT_IW5;
-                    width = MenuMatcherFactory::TokenIntExpressionValue(state, result);
+                syntax = SYNTAX_IW5;
-                    height = MenuMatcherFactory::TokenIntExpressionValue(state, result);
+                hasHeightValues = true;
-                    maxChars = MenuMatcherFactory::TokenIntExpressionValue(state, result);
-                    alignment = MenuMatcherFactory::TokenIntExpressionValue(state, result);
             }
 
-                CommonItemFeaturesListBox::Column column{
+            bool hasAlignmentSpecified;
-                    xPos,
+            if (columnValues.size() == columnCount * (baseColumnCount + 1))
-                    yPos,
+                hasAlignmentSpecified = true;
-                    width,
+            else if (columnValues.size() == columnCount * baseColumnCount)
-                    height,
+                hasAlignmentSpecified = false;
-                    maxChars,
+            else
-                    alignment,
+                throw ParsingException(firstToken.GetPos(), std::format("Each row must have the following syntax: {}", syntax));
-                };
+
-                listBoxFeatures->m_columns.emplace_back(column);
+            const auto& listBoxFeatures = state->m_current_item->m_list_box_features;
+
+            size_t valueIndex = 0u;
+            for (auto columnIndex = 0u; columnIndex < columnCount; columnIndex++)
+            {
+                const auto xPos = columnValues[valueIndex++];
+
+                int yPos;
+                if (hasHeightValues)
+                    yPos = columnValues[valueIndex++];
+                else
+                    yPos = 0;
+
+                const auto width = columnValues[valueIndex++];
+
+                int height;
+                if (hasHeightValues)
+                    height = columnValues[valueIndex++];
+                else
+                    height = 0;
+
+                const auto maxChars = columnValues[valueIndex++];
+
+                int alignment;
+                if (hasAlignmentSpecified)
+                    alignment = columnValues[valueIndex++];
+                else
+                    alignment = 0;
+
+                listBoxFeatures->m_columns.emplace_back(CommonItemFeaturesListBox::Column{
+                    .m_x_pos = xPos,
+                    .m_y_pos = yPos,
+                    .m_width = width,
+                    .m_height = height,
+                    .m_max_chars = maxChars,
+                    .m_alignment = alignment,
+                });
             }
         }
     };
@@ -573,7 +586,7 @@ namespace menu::item_scope_sequences
             auto newEventHandlerSet = std::make_unique<CommonEventHandlerSet>();
             state->m_current_event_handler_set = newEventHandlerSet.get();
             state->m_current_nested_event_handler_set = newEventHandlerSet.get();
-            state->m_current_item->m_key_handlers.emplace(std::make_pair(key, std::move(newEventHandlerSet)));
+            state->m_current_item->m_key_handlers.emplace(key, std::move(newEventHandlerSet));
         }
     };
 
@@ -607,12 +620,10 @@ namespace menu::item_scope_sequences
             auto newEventHandlerSet = std::make_unique<CommonEventHandlerSet>();
             state->m_current_event_handler_set = newEventHandlerSet.get();
             state->m_current_nested_event_handler_set = newEventHandlerSet.get();
-            state->m_current_item->m_key_handlers.emplace(std::make_pair(keyValue, std::move(newEventHandlerSet)));
+            state->m_current_item->m_key_handlers.emplace(keyValue, std::move(newEventHandlerSet));
         }
     };
-} // namespace menu::item_scope_sequences
+} // namespace
-
-using namespace item_scope_sequences;
 
 ItemScopeSequences::ItemScopeSequences(std::vector<std::unique_ptr<MenuFileParser::sequence_t>>& allSequences,
                                        std::vector<MenuFileParser::sequence_t*>& scopeSequences)
@@ -620,7 +631,7 @@ ItemScopeSequences::ItemScopeSequences(std::vector<std::unique_ptr<MenuFileParse
 {
 }
 
-void ItemScopeSequences::AddSequences(FeatureLevel featureLevel, bool permissive) const
+void ItemScopeSequences::AddSequences(const FeatureLevel featureLevel, const bool permissive) const
 {
     AddSequence(std::make_unique<SequenceCloseBlock>());
     AddSequence(std::make_unique<GenericStringPropertySequence>("name",
@@ -746,32 +757,32 @@ void ItemScopeSequences::AddSequences(FeatureLevel featureLevel, bool permissive
                                                                  state->m_current_item->m_text_font = value;
                                                              }));
     AddSequence(std::make_unique<GenericColorPropertySequence>("backcolor",
-                                                               [](const MenuFileParserState* state, const TokenPos&, const CommonColor value)
+                                                               [](const MenuFileParserState* state, const TokenPos&, const CommonColor& value)
                                                                {
                                                                    state->m_current_item->m_back_color = value;
                                                                }));
     AddSequence(std::make_unique<GenericColorPropertySequence>("forecolor",
-                                                               [](const MenuFileParserState* state, const TokenPos&, const CommonColor value)
+                                                               [](const MenuFileParserState* state, const TokenPos&, const CommonColor& value)
                                                                {
                                                                    state->m_current_item->m_fore_color = value;
                                                                }));
     AddSequence(std::make_unique<GenericColorPropertySequence>("bordercolor",
-                                                               [](const MenuFileParserState* state, const TokenPos&, const CommonColor value)
+                                                               [](const MenuFileParserState* state, const TokenPos&, const CommonColor& value)
                                                                {
                                                                    state->m_current_item->m_border_color = value;
                                                                }));
     AddSequence(std::make_unique<GenericColorPropertySequence>("outlinecolor",
-                                                               [](const MenuFileParserState* state, const TokenPos&, const CommonColor value)
+                                                               [](const MenuFileParserState* state, const TokenPos&, const CommonColor& value)
                                                                {
                                                                    state->m_current_item->m_outline_color = value;
                                                                }));
     AddSequence(std::make_unique<GenericColorPropertySequence>("disablecolor",
-                                                               [](const MenuFileParserState* state, const TokenPos&, const CommonColor value)
+                                                               [](const MenuFileParserState* state, const TokenPos&, const CommonColor& value)
                                                                {
                                                                    state->m_current_item->m_disable_color = value;
                                                                }));
     AddSequence(std::make_unique<GenericColorPropertySequence>("glowcolor",
-                                                               [](const MenuFileParserState* state, const TokenPos&, const CommonColor value)
+                                                               [](const MenuFileParserState* state, const TokenPos&, const CommonColor& value)
                                                                {
                                                                    state->m_current_item->m_glow_color = value;
                                                                }));
@@ -1060,7 +1071,7 @@ void ItemScopeSequences::AddSequences(FeatureLevel featureLevel, bool permissive
     }
 
     // ============== ListBox ==============
-    AddSequence(std::make_unique<SequenceColumns>(featureLevel));
+    AddSequence(std::make_unique<SequenceColumns>());
     AddSequence(std::make_unique<GenericKeywordPropertySequence>("notselectable",
                                                                  [](const MenuFileParserState* state, const TokenPos& pos)
                                                                  {

src/ObjLoading/Parsing/Menu/Sequence/MenuScopeSequences.cpp

@@ -17,7 +17,7 @@
 
 using namespace menu;
 
-namespace menu::menu_scope_sequences
+namespace
 {
     class SequenceCloseBlock final : public MenuFileParser::sequence_t
     {
@@ -72,8 +72,6 @@ namespace menu::menu_scope_sequences
 
     class SequenceItemDef final : public MenuFileParser::sequence_t
     {
-        static constexpr auto CAPTURE_TOKEN = 1;
-
     public:
         SequenceItemDef()
         {
@@ -208,9 +206,7 @@ namespace menu::menu_scope_sequences
             state->m_current_menu->m_key_handlers.emplace(std::make_pair(keyValue, std::move(newEventHandlerSet)));
         }
     };
-} // namespace menu::menu_scope_sequences
+} // namespace
-
-using namespace menu_scope_sequences;
 
 MenuScopeSequences::MenuScopeSequences(std::vector<std::unique_ptr<MenuFileParser::sequence_t>>& allSequences,
                                        std::vector<MenuFileParser::sequence_t*>& scopeSequences)
@@ -218,7 +214,7 @@ MenuScopeSequences::MenuScopeSequences(std::vector<std::unique_ptr<MenuFileParse
 {
 }
 
-void MenuScopeSequences::AddSequences(FeatureLevel featureLevel, bool permissive) const
+void MenuScopeSequences::AddSequences(const FeatureLevel featureLevel, const bool permissive) const
 {
     AddSequence(std::make_unique<SequenceCloseBlock>());
     AddSequence(std::make_unique<SequenceItemDef>());
@@ -294,27 +290,27 @@ void MenuScopeSequences::AddSequences(FeatureLevel featureLevel, bool permissive
                                                                            state->m_current_menu->m_border_size = value;
                                                                        }));
     AddSequence(std::make_unique<GenericColorPropertySequence>("backcolor",
-                                                               [](const MenuFileParserState* state, const TokenPos&, const CommonColor value)
+                                                               [](const MenuFileParserState* state, const TokenPos&, const CommonColor& value)
                                                                {
                                                                    state->m_current_menu->m_back_color = value;
                                                                }));
     AddSequence(std::make_unique<GenericColorPropertySequence>("forecolor",
-                                                               [](const MenuFileParserState* state, const TokenPos&, const CommonColor value)
+                                                               [](const MenuFileParserState* state, const TokenPos&, const CommonColor& value)
                                                                {
                                                                    state->m_current_menu->m_fore_color = value;
                                                                }));
     AddSequence(std::make_unique<GenericColorPropertySequence>("bordercolor",
-                                                               [](const MenuFileParserState* state, const TokenPos&, const CommonColor value)
+                                                               [](const MenuFileParserState* state, const TokenPos&, const CommonColor& value)
                                                                {
                                                                    state->m_current_menu->m_border_color = value;
                                                                }));
     AddSequence(std::make_unique<GenericColorPropertySequence>("focuscolor",
-                                                               [](const MenuFileParserState* state, const TokenPos&, const CommonColor value)
+                                                               [](const MenuFileParserState* state, const TokenPos&, const CommonColor& value)
                                                                {
                                                                    state->m_current_menu->m_focus_color = value;
                                                                }));
     AddSequence(std::make_unique<GenericColorPropertySequence>("outlinecolor",
-                                                               [](const MenuFileParserState* state, const TokenPos&, const CommonColor value)
+                                                               [](const MenuFileParserState* state, const TokenPos&, const CommonColor& value)
                                                                {
                                                                    state->m_current_menu->m_outline_color = value;
                                                                }));

src/ObjLoading/Parsing/Menu/Sequence/NoScopeSequences.cpp

@@ -5,7 +5,7 @@
 
 using namespace menu;
 
-namespace menu::no_scope_sequences
+namespace
 {
     class SequenceOpenGlobalScope final : public MenuFileParser::sequence_t
     {
@@ -25,9 +25,7 @@ namespace menu::no_scope_sequences
             state->m_in_global_scope = true;
         }
     };
-} // namespace menu::no_scope_sequences
+} // namespace
-
-using namespace no_scope_sequences;
 
 NoScopeSequences::NoScopeSequences(std::vector<std::unique_ptr<MenuFileParser::sequence_t>>& allSequences,
                                    std::vector<MenuFileParser::sequence_t*>& scopeSequences)
@@ -35,7 +33,7 @@ NoScopeSequences::NoScopeSequences(std::vector<std::unique_ptr<MenuFileParser::s
 {
 }
 
-void NoScopeSequences::AddSequences(FeatureLevel featureLevel, bool permissive) const
+void NoScopeSequences::AddSequences(const FeatureLevel featureLevel, const bool permissive) const
 {
     AddSequence(std::make_unique<SequenceOpenGlobalScope>());
 }

test/ObjLoadingTests/Game/IW4/Menu/LoaderMenuListIW4Test.cpp

@@ -331,6 +331,59 @@ namespace test::game::iw4::menu::parsing::it
         REQUIRE(item->dvarFlags == ITEM_DVAR_FLAG_FOCUS);
     }
 
+    TEST_CASE("MenuParsingIW4IT: Can omit list box column alignment", "[parsing][converting][menu][it]")
+    {
+        MenuParsingItHelper helper;
+
+        helper.AddFile(R"testmenu(
+{
+    menuDef
+    {
+        name "ListBoxColumns"
+        itemDef
+        {
+            type 6
+            // x, width, max chars
+            columns 1 22 190 64
+        }
+        itemDef
+        {
+            type 6
+            // x, width, max chars, alignment
+            columns 2 0 18 3 2 18 25 10 1
+        }
+    }
+}
+            )testmenu");
+
+        const auto result = helper.RunIntegrationTest();
+        REQUIRE(result.HasBeenSuccessful());
+
+        const auto* menu = helper.GetMenuAsset("ListBoxColumns");
+        REQUIRE(menu->itemCount == 2);
+        REQUIRE(menu->items != nullptr);
+
+        const auto* columnsWithoutAlignment = menu->items[0]->typeData.listBox;
+        REQUIRE(columnsWithoutAlignment != nullptr);
+        REQUIRE(columnsWithoutAlignment->numColumns == 1);
+        CHECK(columnsWithoutAlignment->columnInfo[0].pos == 22);
+        CHECK(columnsWithoutAlignment->columnInfo[0].width == 190);
+        CHECK(columnsWithoutAlignment->columnInfo[0].maxChars == 64);
+        CHECK(columnsWithoutAlignment->columnInfo[0].alignment == 0);
+
+        const auto* columnsWithAlignment = menu->items[1]->typeData.listBox;
+        REQUIRE(columnsWithAlignment != nullptr);
+        REQUIRE(columnsWithAlignment->numColumns == 2);
+        CHECK(columnsWithAlignment->columnInfo[0].pos == 0);
+        CHECK(columnsWithAlignment->columnInfo[0].width == 18);
+        CHECK(columnsWithAlignment->columnInfo[0].maxChars == 3);
+        CHECK(columnsWithAlignment->columnInfo[0].alignment == 2);
+        CHECK(columnsWithAlignment->columnInfo[1].pos == 18);
+        CHECK(columnsWithAlignment->columnInfo[1].width == 25);
+        CHECK(columnsWithAlignment->columnInfo[1].maxChars == 10);
+        CHECK(columnsWithAlignment->columnInfo[1].alignment == 1);
+    }
+
     TEST_CASE("MenuParsingIW4IT: Can specify event handler multiple times", "[parsing][converting][menu][it]")
     {
         MenuParsingItHelper helper;