Moved locale validation to a compile time test, instead of a runtime check performed by MSE itself.

This also removes perl as a build dependency for people who don't want to run the test suite.

CMakeLists.txt

@@ -34,14 +34,6 @@ target_precompile_headers(magicseteditor PRIVATE src/util/prec.hpp)
 
 # resource file
 
-set(locale_keys_file "${PROJECT_SOURCE_DIR}/resource/expected_locale_keys")
-add_custom_command(
-  COMMAND perl "${PROJECT_SOURCE_DIR}/tools/locale/locale.pl" "${PROJECT_SOURCE_DIR}/src" ${locale_keys_file}
-  OUTPUT ${locale_keys_file}
-  DEPENDS ${sources}
-)
-add_custom_target(generate_expected_locale_keys DEPENDS ${locale_keys_file})
-add_dependencies(magicseteditor generate_expected_locale_keys)
 target_sources(magicseteditor PRIVATE resource/win32_res.rc)
 
 # magicseteditor.com: wrapper to enable command line executable on windows
@@ -63,3 +55,6 @@ endif()
 
 # visual studio debugger
 set_property(DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR} PROPERTY VS_STARTUP_PROJECT magicseteditor)
+
+# Test suite
+include(test/tests.cmake)

README.md

@@ -17,7 +17,6 @@ On windows, the program can be compiled with Visual Studio (recommended) or with
 
  * Download and install [Visual Studio Community edition](https://visualstudio.microsoft.com/vs/community/)
  * Download and install [vcpkg](https://github.com/microsoft/vcpkg)
- * Download and install perl (For example [Strawberry perl](http://strawberryperl.com/) or using [MSYS2](https://www.msys2.org/))
  * Use vcpkg to install wxwidgets, boost, hunspell
 ````
 vcpkg install wxwidgets
@@ -31,13 +30,16 @@ vcpkg integrate install
 Notes:
  * You may need to work around [this bug](https://github.com/microsoft/vcpkg/issues/4756) by replacing `$VCPATH\IDE\CommonExtensions\Microsoft\CMake\CMake\share\cmake-3.16\Modules\FindwxWidgets.cmake` with the file from  https://github.com/CaeruleusAqua/vcpkg-wx-find
  * vcpkg by default installs 32 bit versions of libraries, use `vcpkg install PACKAGENAME:x64-windows` if you want to enable a 64 bit build.
+ 
+For running tests you will also need to
+ * Download and install perl (For example [Strawberry perl](http://strawberryperl.com/) or using [MSYS2](https://www.msys2.org/))
+The tests can be run from inside visual studio
 
 ## Building on windows with GCC (MSYS2)
 
  * Download and install [msys2](https://www.msys2.org/)
  * Install a recent version of the gcc compiler, cmake, and wxWidgets libraries:
 ````
-pacman -S mingw32/mingw-w64-i686-perl
 pacman -S mingw32/mingw-w64-i686-gcc
 pacman -S mingw32/mingw-w64-i686-wxWidgets
 pacman -S mingw32/mingw-w64-i686-boost
@@ -47,7 +49,7 @@ pacman -S cmake
    Use `mingw64/mingw-w64-x86_64-...` instead of for the 64bit build
  * Build
 ````
-cmake -G "MSYS Makefiles" -H. -Bbuild
+cmake -G "MSYS Makefiles" -H. -Bbuild -DCMAKE_BUILD_TYPE=Release
 cmake --build build
 ````
 
@@ -60,9 +62,10 @@ sudo apt install libboost-dev libwxgtk3.0-gtk3-dev libhunspell-dev
 ````
 Then use cmake to build
 ````
-cmake build
+cmake build -DCMAKE_BUILD_TYPE=Release
 cmake --build build
 ````
+Use `CMAKE_BUILD_TYPE=Debug` for a debug build
 
 ## Building on Mac
 

resource/win32_res.rc

@@ -4,7 +4,7 @@
 //| License:      GNU General Public License 2 or later (see file COPYING)     |
 //+----------------------------------------------------------------------------+
 
-#include <windows.h> // include for version info constants
+#include <winresrc.h> // include for version info constants
 
 // -------------------------------------------------------- : Icons
 
@@ -190,22 +190,18 @@ message_error IMAGE "message_error.png"
 //wxBITMAP_STD_COLOURS BITMAP "wx/msw/colours.bmp"
 //WXCURSOR_HAND CURSOR DISCARDABLE "wx/msw/hand.cur"
 
-// -------------------------------------------------------- : Other
-
-expected_locale_keys TEXT "expected_locale_keys"
-
 // -------------------------------------------------------- : Version info
 
 1 VERSIONINFO
-FILEVERSION    2,0,0,0
+FILEVERSION    2,0,2,0
-PRODUCTVERSION 2,0,0,0
+PRODUCTVERSION 2,0,2,0
 FILETYPE VFT_APP
 {
   BLOCK "StringFileInfo"
   {
     BLOCK "040904E4"
     {
-      VALUE "FileVersion",     "2.0.0"
+      VALUE "FileVersion",     "2.0.2"
       VALUE "License",         "GNU General Public License 2 or later; This is free software, and you are welcome to redistribute it under certain conditions; See the help file for details"
       VALUE "FileDescription", "Magic Set Editor"
       VALUE "InternalName",    "magicseteditor"

src/data/locale.cpp

@@ -131,163 +131,3 @@ String tr(const Package& pkg, const String& subcat, const String& key, DefaultLo
   }
   return loc->tr(subcat, key, def);
 }
-
-// ----------------------------------------------------------------------------- : Validation
-
-DECLARE_POINTER_TYPE(SubLocaleValidator);
-
-class KeyValidator {
-  public:
-  int  args;
-  bool optional;
-  DECLARE_REFLECTION();
-};
-
-class SubLocaleValidator : public IntrusivePtrBase<SubLocaleValidator> {
-  public:
-  map<String,KeyValidator> keys; ///< Arg count for each key
-  DECLARE_REFLECTION();
-};
-
-/// Validation information for locales
-class LocaleValidator {
-  public:
-  map<String, SubLocaleValidatorP> sublocales;
-  DECLARE_REFLECTION();
-};
-
-template <> void Reader::handle(KeyValidator& k) {
-  String v = getValue();
-  if (starts_with(v, _("optional, "))) {
-    k.optional = true;
-    v = v.substr(10);
-  } else {
-    k.optional = false;
-  }
-  long l = 0;
-  v.ToLong(&l);
-  k.args = l;
-}
-template <> void Writer::handle(const KeyValidator& v) {
-  assert(false);
-}
-IMPLEMENT_REFLECTION_NO_SCRIPT(SubLocaleValidator) {
-  REFLECT_NAMELESS(keys);
-}
-IMPLEMENT_REFLECTION_NO_SCRIPT(LocaleValidator) {
-  REFLECT_NAMELESS(sublocales);
-}
-
-/// Count "%s" in str
-int string_format_args(const String& str) {
-  int count = 0;
-  bool in_percent = false;
-  FOR_EACH_CONST(c, str) {
-    if (in_percent) {
-      if (c == _('s')) {
-        count++;
-      }
-      in_percent = false;
-    } else if (c == _('%')) {
-      in_percent = true;
-    }
-  }
-  return count;
-}
-
-/// Load a text file from a resource
-/** TODO: Move me
- */
-InputStreamP load_resource_text(const String& name);
-InputStreamP load_resource_text(const String& name) {
-  #if defined(__WXMSW__) && !defined(__GNUC__)
-    HRSRC hResource = ::FindResource(wxGetInstance(), name.wc_str(), _("TEXT"));
-    if ( hResource == 0 ) throw InternalError(String::Format(_("Resource not found: %s"), name));
-    HGLOBAL hData = ::LoadResource(wxGetInstance(), hResource);
-    if ( hData == 0 ) throw InternalError(String::Format(_("Resource not text: %s"), name));
-    char* data = (char *)::LockResource(hData);
-    if ( !data ) throw InternalError(String::Format(_("Resource cannot be locked: %s"), name));
-    int len = ::SizeofResource(wxGetInstance(), hResource);
-    return shared(new wxMemoryInputStream(data, len));
-  #else
-    static String path = wxStandardPaths::Get().GetDataDir() + _("/resource/");
-    static String local_path = wxStandardPaths::Get().GetUserDataDir() + _("/resource/");
-    if (wxFileExists(path + name)) {
-      return shared(new wxFileInputStream(path + name));
-    } else {
-      return shared(new wxFileInputStream(local_path + name));
-    }
-  #endif
-}
-
-
-DECLARE_TYPEOF(map<String COMMA String>);
-DECLARE_TYPEOF(map<String COMMA KeyValidator>);
-
-void Locale::validate(Version ver) {
-  Packaged::validate(ver);
-  // load locale validator
-  LocaleValidator v;
-  Reader r(load_resource_text(_("expected_locale_keys")), nullptr, _("expected_locale_keys"));
-  r.handle_greedy(v);
-  // validate
-  String errors;
-  errors += translations[LOCALE_CAT_MENU   ].validate(_("menu"),    v.sublocales[_("menu")   ]);
-  errors += translations[LOCALE_CAT_HELP   ].validate(_("help"),    v.sublocales[_("help")   ]);
-  errors += translations[LOCALE_CAT_TOOL   ].validate(_("tool"),    v.sublocales[_("tool")   ]);
-  errors += translations[LOCALE_CAT_TOOLTIP].validate(_("tooltip"), v.sublocales[_("tooltip")]);
-  errors += translations[LOCALE_CAT_LABEL  ].validate(_("label"),   v.sublocales[_("label")  ]);
-  errors += translations[LOCALE_CAT_BUTTON ].validate(_("button"),  v.sublocales[_("button") ]);
-  errors += translations[LOCALE_CAT_TITLE  ].validate(_("title"),   v.sublocales[_("title")  ]);
-  errors += translations[LOCALE_CAT_ACTION ].validate(_("action"),  v.sublocales[_("action") ]);
-  errors += translations[LOCALE_CAT_ERROR  ].validate(_("error"),   v.sublocales[_("error")  ]);
-  errors += translations[LOCALE_CAT_TYPE   ].validate(_("type"),    v.sublocales[_("type")   ]);
-  // errors?
-  if (!errors.empty()) {
-    if (ver != file_version_locale) {
-      errors = _("Errors in locale file ") + short_name + _(":") + errors;
-    } else {
-      errors = _("Errors in locale file ") + short_name +
-               _("\nThis is probably because the locale was made for a different version of MSE.") + errors;
-    }
-  } else if (ver != file_version_locale) {
-      errors = _("Errors in locale file ") + short_name + _(":")
-             + _("\n  Locale file out of date, expected: mse version: ") + file_version_locale.toString()
-             + _("\n  found: ") + ver.toString();
-  }
-  if (!errors.empty()) {
-    queue_message(MESSAGE_WARNING, errors);
-  }
-}
-
-String SubLocale::validate(const String& name, const SubLocaleValidatorP& v) const {
-  if (!v) {
-    return _("\nInternal error validating local file: expected keys file missing for \"") + name + _("\" section.");
-  }
-  String errors;
-  // 1. keys in v but not in this, check arg count
-  FOR_EACH_CONST(kc, v->keys) {
-    map<String,String>::const_iterator it = translations.find(kc.first);
-    if (it == translations.end()) {
-      if (!kc.second.optional) {
-        errors += _("\n   Missing key:\t\t\t") + name + _(": ") + kc.first;
-      }
-    } else if (string_format_args(it->second) != kc.second.args) {
-      errors += _("\n   Incorrect number of arguments for:\t") + name + _(": ") + kc.first
-             +  String::Format(_("\t  expected: %d, found %d"), kc.second.args, string_format_args(it->second));
-    }
-  }
-  // 2. keys in this but not in v
-  FOR_EACH_CONST(kv, translations) {
-    map<String,KeyValidator>::const_iterator it = v->keys.find(kv.first);
-    if (it == v->keys.end() && !kv.second.empty()) {
-      // allow extra keys with empty values as a kind of documentation
-      // for example in the help stirngs:
-      //   help:
-      //       file:
-      //       new set: blah blah
-      errors += _("\n   Unexpected key:\t\t\t") + name + _(": ") + kv.first;
-    }
-  }
-  return errors;
-}

src/data/locale.hpp

@@ -22,22 +22,19 @@ DECLARE_POINTER_TYPE(SubLocaleValidator);
 
 /// Translations of the texts of a game/stylesheet/symbolfont
 class SubLocale : public IntrusivePtrBase<SubLocale> {
-  public:
+public:
   map<String,String> translations;
   
   /// Translate a key, if not found, apply the default function to the key
   String tr(const String& key, DefaultLocaleFun def);
   String tr(const String& subcat, const String& key, DefaultLocaleFun def);
   
-  /// Is this a valid sublocale? Returns errors
-  String validate(const String& name, const SubLocaleValidatorP&) const;
-  
   DECLARE_REFLECTION();
 };
 
 /// A collection of translations of messages
 class Locale : public Packaged {
-  public:
+public:
   /// Translations of UI strings in each category
   SubLocale              translations[LOCALE_CAT_MAX];
   /// Translations of Package specific texts, by relativeFilename
@@ -46,10 +43,7 @@ class Locale : public Packaged {
   /// Open a locale with the given name
   static LocaleP byName(const String& name);
   
-  /// Validate that the locale is valid for this MSE version
+protected:
-  virtual void validate(Version = app_version);
-  
-  protected:
   String typeName() const;
   Version fileVersion() const;
   DECLARE_REFLECTION();

test/locale/validate_locale.pl

@@ -0,0 +1,198 @@
+#! /usr/bin/perl
+
+# Validate a .mse-locale file by checking that the keys match those used in the source code, and that they have the right number of arguments.
+
+use strict;
+use File::Find;
+
+# ------------------------------------------------------------------------------
+# Step 0: arguments
+# ------------------------------------------------------------------------------
+
+if (scalar @ARGV < 1) {
+  die "Usage: $0 <SRCDIR> <LOCALE> [<LOCALE>...]"
+}
+my $indir = shift @ARGV;
+
+# ------------------------------------------------------------------------------
+# Step 1: find keys used in source code
+# ------------------------------------------------------------------------------
+
+# Determine the keys that should be in the locale file,
+# and the number of arguments the keys should have
+
+# Array of locale keys: maps type -> key -> info
+# where info is {argc:arity, opt:optional(only used in commented code)}
+
+our %locale_keys;
+our $i=0;
+
+sub arg_count {
+  return scalar split(/,/,$_[0]);
+}
+
+sub make_comment {
+  my $input = $_[0];
+  $input =~ s/(_[A-Z])/_COMMENT$1/g;
+  return $input;
+}
+
+# for each .cpp/.hpp file, collect locale calls
+sub gather_locale_keys {
+  my $filename = $_;
+  my $full_name = $File::Find::name;
+
+  if (!($filename =~ /\..pp$/)) {
+    return;
+  }
+  
+  # Read file
+  open(my $fh, "<", $filename) or die "Failed to open source file $full_name";
+  my $body = join('',<$fh>);
+  close $fh;
+  
+  # Custom argument expansion
+  my $inparen;
+  $inparen = qr/[^()]|\((??{$inparen})*\)/; # recursive paren matching
+  $body =~ s/(?:String::Format|format_string)\((_[A-Z]+)(_\([^)]+\)),($inparen+)/
+              $1 . "_" . arg_count($3) . $2
+            /ge;
+  $body =~ s/action_name_for[(][^,]*,\s*(_[A-Z]+)(_\([^)]+\))/$1_1$2/g;
+  
+  # Drop comments, mark found items as 'optional'
+  $body =~ s{//[^\n]*} {find_locale_calls($&, 1)}ge;
+  $body =~ s{/\*.*?\*/}{find_locale_calls($&, 1)}ge;
+  
+  find_locale_calls($body, 0);
+}
+
+sub find_locale_calls {
+  my $body       = shift;
+  my $in_comment = shift;
+  
+  # Find calls to locale functions
+  while ($body =~ /_(COMMENT_)?(MENU|HELP|TOOL|TOOLTIP|LABEL|BUTTON|TITLE|TYPE|ACTION|ERROR)_(?:([1-9])_)?\(\s*\"([^\"]+)\"/g) {
+    my $type = $2;
+    my $argc = $3 ? $3 : 0;
+    my $key = $4;
+    if (defined($locale_keys{$type}{$key}{'argc'}) && $locale_keys{$type}{$key}{'argc'} != $argc) {
+      die "ERROR: locale key _${type}_($key) used with different arities";
+    }
+    $locale_keys{$type}{$key}{'opt'}  = defined($locale_keys{$type}{$key}{'opt'}) ? ($locale_keys{$type}{$key}{'opt'} && $in_comment) : $in_comment;
+    $locale_keys{$type}{$key}{'argc'} = $argc;
+  }
+  # addPanel uses multiple types
+  while ($body =~ m{
+    ( addPanel \((?:[^,]+,){6} # gui/set/window.cpp
+    )
+    \s* _ \(\" ([^\"]+) \"\)
+  }xg) {
+    my $key = $2;
+    $key =~ s/_/ /g;
+    foreach my $type ("MENU","HELP","TOOL","TOOLTIP") {
+      $locale_keys{$type}{$key}{'opt'}  = $in_comment;
+      $locale_keys{$type}{$key}{'argc'} = 0;
+    }
+  }
+}
+
+#my $filename = 'src/code_template.cpp';
+#open(my $fh, "<", $filename) or die "WTF Failed to open source file $filename".length($filename);
+
+find(\&gather_locale_keys, $indir);
+if (scalar(%locale_keys) == 0) {
+  die "Did not find any source files with locale keys"
+}
+print $locale_keys{'ERROR'}{'successful instalal'} . "\n";
+
+# ------------------------------------------------------------------------------
+# Step 2: validate a locale
+# ------------------------------------------------------------------------------
+
+sub parse_locale {
+  # Load and parse a .mse-locale file
+  # Return a mapping  type -> key -> value
+  my $locale_file = shift;
+  open(my $fh, "<", "$locale_file/locale") or
+    die "Error: locale file not found: $locale_file/locale";
+  # Get lines from file
+  my $type = undef;
+  my $key = undef;
+  my %locale;
+  for my $line (<$fh>) {
+    if ($line =~ /^\s*#|^\s*$/) {
+      # comment
+    } elsif ($line =~ /^([^:\t]+):/) {
+      $type = uc $1;
+      $key = undef;
+    } elsif ($line =~ /^\t([^:\t]+):(.*)/) {
+      $key = $1;
+      if (defined($locale{$type}{$key})) {
+        die "Locale key already defined: $type: $key";
+      }
+      $locale{$type}{$key} = $2;
+    } elsif ($line =~ /^\t\t(.*)/) {
+      $locale{$type}{$key} .= $1;
+    } else {
+      die "Unknown line in locale file: $line\n";
+    }
+  }
+  close $fh;
+  return %locale
+}
+
+sub validate_locale {
+  my $locale_file = shift;
+  my %locale = parse_locale($locale_file);
+  # Validate locale:
+  #  The set of keys should match exactly, so every key expected by the program should be in the locale file, and vice-versa
+  my $ok = 1;
+  foreach my $type (sort keys %locale_keys) {
+    if (!defined($locale{$type})) {
+      print STDERR "Missing key in locale: $type\n have keys " . "[" . join(', ', keys %locale) . "]\n";
+      $ok = 0;
+      next;
+    }
+    foreach my $key (sort keys %{$locale_keys{$type}}) {
+      if (!defined($locale{$type}{$key})) {
+        if (!$locale_keys{$type}{$key}{'opt'}) {
+          print STDERR "Missing key in locale: $type: $key\n";
+          $ok = 0;
+        }
+        next;
+      }
+      # Count the number of printf style arguments in the value
+      my @args = $locale{$type}{$key} =~ /%[sd]/g;
+      my $argc = scalar(@args);
+      if ($argc != $locale_keys{$type}{$key}{'argc'}) {
+        print STDERR "Incorrect number of arguments for $type: $key. Expected $locale_keys{$type}{$key}{'argc'}, got $argc\n";
+        $ok = 0;
+      }
+    }
+  }
+  foreach my $type (sort keys %locale) {
+    next if $type eq 'PACKAGE'; # Ignore package specific locale keys
+    if (!defined($locale_keys{$type})) {
+      print STDERR "Unknown key in locale: $type\n expected keys " . "[" . join(', ', keys %locale_keys) . "]\n";
+      $ok = 0;
+      next;
+    }
+    foreach my $key (sort keys %{$locale{$type}}) {
+      if (!defined($locale_keys{$type}{$key})) {
+        print STDERR "Unknown key in locale: $type: $key\n";
+        $ok = 0;
+      }
+    }
+  }
+  return $ok;
+}
+
+if (scalar @ARGV < 1) {
+  die "No locales found to validate";
+}
+
+for my $locale (@ARGV) {
+  if (!validate_locale($locale)) {
+    exit 1;
+  }
+}

test/tests.cmake

@@ -0,0 +1,17 @@
+# Use CTest for tests
+enable_testing()
+set(test_dir "${PROJECT_SOURCE_DIR}/test")
+set(CTEST_OUTPUT_ON_FAILURE 1)
+
+# Validate locales
+file(GLOB locales data/*.mse-locale)
+add_test(
+  NAME "validate-locales"
+  COMMAND perl "${test_dir}/locale/validate_locale.pl" "${PROJECT_SOURCE_DIR}/src" ${locales}
+)
+
+# Scripting language tests
+# TODO
+
+# Rendering tests
+# TODO