Cleaned up the reflection code a bit

* Renamed 'tag' variable to 'handler'
* Removed addAlias stuff, instead check for matching names with if statements
* Added after_reading function that is called by Reader after reading a complete object. This generalizes Packaged::validate, which is now also called via this mechanism.
* Removed some backwards compatibility with <0.3.0 for templates

src/util/io/get_member.cpp

@@ -36,7 +36,7 @@ GetMember::GetMember(const String& name)
   : target_name(name)
 {}
 
-// caused by the pattern: if (!tag.isComplex()) { REFLECT_NAMELESS(stuff) }
+// caused by the pattern: if (!handler.isCompound()) { REFLECT_NAMELESS(stuff) }
 template <> void GetMember::handle(const String& v) {
   throw InternalError(_("GetDefaultMember::handle"));
 }

src/util/io/get_member.hpp

@@ -22,13 +22,13 @@ template <typename T> class Scriptable;
 /// Find a member without a name using reflection
 /** The member is wrapped in a ScriptValue */
 class GetDefaultMember {
-  public:
-  /// Tell the reflection code we are not reading
-  inline bool reading()   const { return false; }
-  inline bool scripting() const { return true; }
-  inline bool isComplex() const { return false; }
-  inline void addAlias(int, const Char*, const Char*) {}
-  inline void handleIgnore(int, const Char*) {}
+public:
+  /// Tell the reflection code we are not looking at members
+  static constexpr bool isReading = false;
+  static constexpr bool isWriting = false;
+  static constexpr bool isScripting = false;
+  inline bool isCompound() const { return true; }
+  inline Version formatVersion() const { return app_version; }
   
   /// The result, or script_nil if the member was not found
   inline ScriptValueP result() { return value; } 
@@ -57,26 +57,26 @@ class GetDefaultMember {
   template <typename T>             void handle(const intrusive_ptr<T>& p) { value = to_script(p); }
   void handle(const ScriptValueP&);
   void handle(const ScriptP&);
-  private:
-  ScriptValueP  value;    ///< The value we found (if any)
+private:
+  ScriptValueP value;    ///< The value we found (if any)
 };
 
 // ----------------------------------------------------------------------------- : GetMember
 
 /// Find a member with a specific name using reflection
 /** The member is wrapped in a ScriptValue */
-class GetMember : private GetDefaultMember {
-  public:
+class GetMember {
+public:
   /// Construct a member getter that looks for the given name
   GetMember(const String& name);
   
-  /// Tell the reflection code we are not reading
-  inline bool reading()   const { return false; }
-  inline bool scripting() const { return true; }
-  inline bool isComplex() const { return true; }
-  inline void addAlias(int, const Char*, const Char*) {}
-  inline void handleIgnore(int, const Char*) {}
-  
+  /// Tell the reflection code we are getting a member for scripting purposes
+  static constexpr bool isReading = true;
+  static constexpr bool isWriting = false;
+  static constexpr bool isScripting = false;
+  inline bool isCompound() const { return true; }
+  inline Version formatVersion() const { return app_version; }
+
   /// The result, or script_nil if the member was not found
   inline ScriptValueP result() { return gdm.result(); } 
   
@@ -106,7 +106,7 @@ class GetMember : private GetDefaultMember {
   }
   template <typename K, typename V> void handle(const DelayedIndexMaps<K,V>&);
   template <typename K, typename V> void handle(const DelayedIndexMapsData<K,V>&);
-    
+  
   private:
   const String& target_name;  ///< The name we are looking for
   GetDefaultMember gdm;    ///< Object to store and retrieve the value
@@ -120,17 +120,17 @@ class GetMember : private GetDefaultMember {
 #define REFLECT_OBJECT_GET_DEFAULT_MEMBER_NOT(Cls)  REFLECT_WRITE_NO(Cls,GetDefaultMember)
 #define REFLECT_OBJECT_GET_MEMBER_NOT(Cls)      REFLECT_WRITE_NO(Cls,GetMember)
 
-#define REFLECT_WRITE_YES(Cls, Tag) \
-  template<> void Tag::handle<Cls>(const Cls& object) { \
+#define REFLECT_WRITE_YES(Cls, Handler) \
+  template<> void Handler::handle<Cls>(const Cls& object) { \
     const_cast<Cls&>(object).reflect(*this); \
   } \
-  void Cls::reflect(Tag& tag) { \
-    reflect_impl(tag); \
+  void Cls::reflect(Handler& handler) { \
+    reflect_impl(handler); \
   }
 
-#define REFLECT_WRITE_NO(Cls, Tag) \
-  template<> void Tag::handle<Cls>(const Cls& object) {} \
-  void Cls::reflect(Tag& tag) {}
+#define REFLECT_WRITE_NO(Cls, Handler) \
+  template<> void Handler::handle<Cls>(const Cls& object) {} \
+  void Cls::reflect(Handler& handler) {}
 
 // ----------------------------------------------------------------------------- : Reflection for enumerations
 
@@ -141,7 +141,7 @@ class GetMember : private GetDefaultMember {
     reflect_ ## Enum(const_cast<Enum&>(enum_), egm); \
   }
 
-/// 'Tag' to be used when reflecting enumerations for GetMember
+/// Handler to be used when reflecting enumerations for GetMember
 class EnumGetMember {
   public:
   inline EnumGetMember(GetDefaultMember& gdm)

src/util/io/package.cpp

@@ -458,7 +458,7 @@ DateTime Package::modificationTime(const pair<String, FileInfo>& fi) const {
 // ----------------------------------------------------------------------------- : Packaged
 
 template <> void Reader::handle(PackageDependency& dep) {
-  if (!isComplex()) {
+  if (!isCompound()) {
     handle(dep.package);
     size_t pos = dep.package.find_first_of(_(' '));
     if (pos != String::npos) {
@@ -504,7 +504,7 @@ unique_ptr<wxInputStream> Packaged::openIconFile() {
   }
 }
 
-// proxy object, that reads just WITHOUT using the overloaded behaviour
+// proxy object, that reads just the package header WITHOUT using the overloaded behaviour
 struct JustAsPackageProxy {
   JustAsPackageProxy(Packaged* that) : that(that) {}
   Packaged* that;
@@ -524,7 +524,6 @@ void Packaged::open(const String& package, bool just_header) {
     try {
       JustAsPackageProxy proxy(this);
       reader.handle_greedy(proxy);
-      Packaged::validate(reader.file_app_version);
     } catch (const ParseError& err) {
       throw FileParseError(err.what(), absoluteFilename() + _("/") + typeName()); // more detailed message
     }
@@ -539,7 +538,6 @@ void Packaged::loadFully() {
   Reader reader(*stream, this, absoluteFilename() + _("/") + typeName());
   try {
     reader.handle_greedy(*this);
-    validate(reader.file_app_version);
     fully_loaded = true; // only after loading and validating succeeded, be careful with recursion!
   } catch (const ParseError& err) {
     throw FileParseError(err.what(), absoluteFilename() + _("/") + typeName()); // more detailed message

src/util/io/package.hpp

@@ -255,13 +255,18 @@ class Packaged : public Package {
   virtual Version fileVersion() const = 0;
 
   DECLARE_REFLECTION_VIRTUAL();
-
+  friend void after_reading(Packaged& p, Version file_app_version);
+  
   private:
   bool   fully_loaded;  ///< Is the package fully loaded?
   friend struct JustAsPackageProxy;
   friend class Installer;
 };
 
+inline void after_reading(Packaged& p, Version file_app_version) {
+  p.validate(file_app_version);
+}
+
 // ----------------------------------------------------------------------------- : IncludePackage
 
 /// A package that just contains a bunch of files that are used from other packages

src/util/io/reader.cpp

@@ -37,12 +37,6 @@ unique_ptr<wxInputStream> Reader::openIncludedFile() {
   return package_manager.openFileFromPackage(package, value);
 }
 
-void Reader::addAlias(Version end_version, const Char* a, const Char* b) {
-  Alias& alias = aliasses[a];
-  alias.new_key     = b;
-  alias.end_version = end_version;
-}
-
 void Reader::handleIgnore(int end_version, const Char* a) {
   if (file_app_version < end_version) {
     if (enterBlock(a)) exitBlock();
@@ -243,26 +237,6 @@ void Reader::unknownKey() {
     } while (indent > expected_indent);
     return;
   }
-  // aliasses?
-  map<String,Alias>::const_iterator it = aliasses.find(key);
-  if (it != aliasses.end()) {
-    if (aliasses.find(it->second.new_key) != aliasses.end()) {
-      // alias points to another alias, don't follow it, there is the risk of infinite loops
-    } else if (it->second.end_version <= file_app_version) {
-      // alias not used for this version, use in warning
-      if (indent == expected_indent) {
-        warning(_("Unexpected key: '") + key + _("'  use  '") + it->second.new_key + _("'"), 0, false);
-        do {
-          moveNext();
-        } while (indent > expected_indent);
-        return;
-      }
-    } else {
-      // try this key instead
-      key = it->second.new_key;
-      return;
-    }
-  }
   if (indent >= expected_indent) {
     warning(_("Unexpected key: '") + key + _("'"), 0, false);
     do {

src/util/io/reader.hpp

@@ -28,30 +28,25 @@ class Packaged;
  *  object that was just read.
  */
 class Reader {
-  private:
-  /// Construct a reader that reads a file in a package
-  /** Used for "include file" keys.
-   *  package can be nullptr
-   */
-  //Reader(Reader* parent, Packaged* package, const String& filename, bool ignore_invalid = false);
   public:
   /// Construct a reader that reads from the given input stream
   /** filename is used only for error messages
+   *  package is used for looking up included files.
    */
   Reader(wxInputStream& input, Packaged* package = nullptr, const String& filename = wxEmptyString, bool ignore_invalid = false);
   
   ~Reader() { showWarnings(); }
   
   /// Tell the reflection code we are reading
-  inline bool reading() const { return true; }
-  /// Tell the reflection code we are not related to scripting
-  inline bool scripting() const { return false; }
+  static constexpr bool isReading = true;
+  static constexpr bool isWriting = false;
+  static constexpr bool isScripting = false;
   /// Is the thing currently being read 'complex', i.e. does it have children
-  inline bool isComplex() const { return indent != expected_indent - 1 || value.empty(); }
-  /// Add a as an alias for b, all keys a will be replaced with b, only if file_app_version < end_version
-  void addAlias(Version end_version, const Char* a, const Char* b);
+  inline bool isCompound() const { return indent != expected_indent - 1 || value.empty(); }
   /// Ignore old keys
   void handleIgnore(int, const Char*);
+  /// Get the version of the format we are reading
+  inline Version formatVersion() const { return file_app_version; }
   
   /// Read and check the application version
   void handleAppVersion();
@@ -70,6 +65,7 @@ class Reader {
       if (state != HANDLED) unknownKey(object);
       state = OUTSIDE;
     } while (indent >= expected_indent);
+    after_reading(object, file_app_version);
   }
   
   /// Handle an object: read it if it's name matches
@@ -112,10 +108,10 @@ class Reader {
   /// The package being read from
   inline Packaged* getPackage() const { return package; }
   
+private:
   // --------------------------------------------------- : Data
   /// App version this file was made with
   Version file_app_version;
-private:
   /// The line we read
   String line;
   /// The key and value of the last line we read
@@ -133,13 +129,6 @@ private:
     HANDLED,    ///< We have handled a value, and moved to the next line, previous_value is the value we just handled
     UNHANDLED,    ///< Something has been 'unhandled()'
   } state;
-  /// Aliasses for compatability
-  struct Alias {
-    String  new_key;
-    Version end_version;
-  };
-  /// Aliasses for compatability
-  map<String, Alias> aliasses;
   /// Should all invalid keys be ignored?
   bool ignore_invalid;
   
@@ -194,6 +183,16 @@ private:
   unique_ptr<wxInputStream> openIncludedFile();
 };
 
+// ----------------------------------------------------------------------------- : After reading hook
+
+// Overload to perform extra stuff after reading
+template <typename T> inline void after_reading(T&, Version) {}
+
+template <typename T>
+inline void after_reading(intrusive_ptr<T>& x, Version ver) {
+  after_reading(*x, ver);
+}
+
 // ----------------------------------------------------------------------------- : Container types
 
 /// Construct a new type, possibly reading something in the process.
@@ -243,33 +242,33 @@ void Reader::handle(IndexMap<K,V>& m) {
 // ----------------------------------------------------------------------------- : Reflection
 
 /// Implement reflection as used by Reader
-#define REFLECT_OBJECT_READER(Cls) \
-  template<> void Reader::handle<Cls>(Cls& object) { \
-    object.reflect(*this); \
-  } \
-  void Cls::reflect(Reader& reader) { \
-    reflect_impl(reader); \
+#define REFLECT_OBJECT_READER(Cls) \
+  template<> void Reader::handle<Cls>(Cls& object) { \
+    object.reflect(*this); \
+  } \
+  void Cls::reflect(Reader& reader) { \
+    reflect_impl(reader); \
   }
 
 // ----------------------------------------------------------------------------- : Reflection for enumerations
 
 /// Implement enum reflection as used by Reader
-#define REFLECT_ENUM_READER(Enum) \
-  template<> void Reader::handle<Enum>(Enum& enum_) { \
-    EnumReader reader(getValue()); \
-    reflect_ ## Enum(enum_, reader); \
-    reader.warnIfNotDone(this); \
-  } \
-  void parse_enum(const String& value, Enum& out) { \
-    EnumReader reader(value); \
-    reflect_ ## Enum(out, reader); \
-    reader.errorIfNotDone(); \
+#define REFLECT_ENUM_READER(Enum) \
+  template<> void Reader::handle<Enum>(Enum& enum_) { \
+    EnumReader reader(getValue()); \
+    reflect_ ## Enum(enum_, reader); \
+    reader.warnIfNotDone(this); \
+  } \
+  void parse_enum(const String& value, Enum& out) { \
+    EnumReader reader(value); \
+    reflect_ ## Enum(out, reader); \
+    reader.errorIfNotDone(); \
   }
 
-/// 'Tag' to be used when reflecting enumerations for Reader
+/// 'Handler' to be used when reflecting enumerations for Reader
 class EnumReader {
   public:
-  inline EnumReader(String read)
+  inline EnumReader(String const& read)
     : read(read), first(nullptr), done(false) {}
   
   /// Handle a possible value for the enum, if the name matches the name in the input

src/util/io/writer.hpp

@@ -25,11 +25,11 @@ class Writer {
   Writer(OutputStream& output, Version file_app_version);
   
   /// Tell the reflection code we are not reading
-  inline bool reading()   const { return false; }
-  inline bool scripting() const { return false; }
-  inline bool isComplex() const { return true; }
-  inline void addAlias(int, const Char*, const Char*) {}
-  inline void handleIgnore(int, const Char*) {}
+  static constexpr bool isReading = false;
+  static constexpr bool isWriting = true;
+  static constexpr bool isScripting = false;
+  inline bool isCompound() const { return true; }
+  inline Version formatVersion() const { return app_version; }
   
   // --------------------------------------------------- : Handling objects
   /// Handle an object: write it under the given name
@@ -145,7 +145,7 @@ void Writer::handle(const IndexMap<K,V>& m) {
     reflect_ ## Enum(const_cast<Enum&>(enum_), writer); \
   }
 
-/// 'Tag' to be used when reflecting enumerations for Writer
+/// Handler to be used when reflecting enumerations for Writer
 class EnumWriter {
   public:
   inline EnumWriter(Writer& writer)