Use LocalFileName class for file names inside a package.

src/data/action/value.cpp

@@ -87,10 +87,10 @@ unique_ptr<ValueAction> value_action(const ChoiceValueP& value, const Defaultabl
 unique_ptr<ValueAction> value_action(const ColorValueP& value, const Defaultable<Color>& new_value) {
   return make_unique<SimpleValueAction<ColorValue, true>>(value, new_value);
 }
-unique_ptr<ValueAction> value_action(const ImageValueP& value, const FileName& new_value) {
+unique_ptr<ValueAction> value_action(const ImageValueP& value, const LocalFileName& new_value) {
   return make_unique<SimpleValueAction<ImageValue, false>>(value, new_value);
 }
-unique_ptr<ValueAction> value_action(const SymbolValueP& value, const FileName& new_value) {
+unique_ptr<ValueAction> value_action(const SymbolValueP& value, const LocalFileName& new_value) {
   return make_unique<SimpleValueAction<SymbolValue, false>>(value, new_value);
 }
 unique_ptr<ValueAction> value_action(const PackageChoiceValueP& value, const String& new_value) {

src/data/action/value.hpp

@@ -19,6 +19,7 @@
 
 class Card;
 class StyleSheet;
+class LocalFileName;
 DECLARE_POINTER_TYPE(Set);
 DECLARE_POINTER_TYPE(Value);
 DECLARE_POINTER_TYPE(Style);
@@ -57,8 +58,8 @@ class ValueAction : public Action {
 unique_ptr<ValueAction> value_action(const ChoiceValueP&         value, const Defaultable<String>& new_value);
 unique_ptr<ValueAction> value_action(const MultipleChoiceValueP& value, const Defaultable<String>& new_value, const String& last_change);
 unique_ptr<ValueAction> value_action(const ColorValueP&          value, const Defaultable<Color>&  new_value);
-unique_ptr<ValueAction> value_action(const ImageValueP&          value, const FileName&            new_value);
+unique_ptr<ValueAction> value_action(const ImageValueP&          value, const LocalFileName&       new_value);
-unique_ptr<ValueAction> value_action(const SymbolValueP&         value, const FileName&            new_value);
+unique_ptr<ValueAction> value_action(const SymbolValueP&         value, const LocalFileName&       new_value);
 unique_ptr<ValueAction> value_action(const PackageChoiceValueP&  value, const String&              new_value);
 
 // ----------------------------------------------------------------------------- : Text

src/data/field/image.hpp

@@ -12,6 +12,7 @@
 #include <data/field.hpp>
 #include <script/scriptable.hpp>
 #include <script/image.hpp>
+#include <util/io/package.hpp>
 
 // ----------------------------------------------------------------------------- : ImageField
 
@@ -45,7 +46,7 @@ class ImageStyle : public Style {
 class ImageValue : public Value {
   public:
   inline ImageValue(const ImageFieldP& field) : Value(field) {}
-  DECLARE_VALUE_TYPE(Image, FileName);
+  DECLARE_VALUE_TYPE(Image, LocalFileName);
   
   ValueType filename;    ///< Filename of the image (in the current package), or ""
   Age       last_update; ///< When was the image last changed?

src/data/field/symbol.cpp

@@ -51,6 +51,8 @@ String SymbolValue::toString() const {
   return filename.empty() ? wxEmptyString : _("<symbol>");
 }
 
-IMPLEMENT_REFLECTION_NAMELESS(SymbolValue) {
+IMPLEMENT_REFLECTION_NO_GET_MEMBER(SymbolValue) {
   if (fieldP->save_value || !handler.isWriting) REFLECT_NAMELESS(filename);
 }
+void SymbolValue::reflect(GetMember& handler) {}
+void SymbolValue::reflect(GetDefaultMember& handler) {}

src/data/field/symbol.hpp

@@ -11,6 +11,7 @@
 #include <util/prec.hpp>
 #include <data/field.hpp>
 #include <script/scriptable.hpp>
+#include <util/io/package.hpp>
 
 DECLARE_POINTER_TYPE(SymbolFilter);
 DECLARE_POINTER_TYPE(SymbolVariation);
@@ -65,7 +66,7 @@ class SymbolVariation : public IntrusivePtrBase<SymbolVariation> {
 class SymbolValue : public Value {
   public:
   inline SymbolValue(const SymbolFieldP& field) : Value(field) {}
-  DECLARE_VALUE_TYPE(Symbol, FileName);
+  DECLARE_VALUE_TYPE(Symbol, LocalFileName);
   
   ValueType filename;    ///< Filename of the symbol (in the current package)
   Age       last_update; ///< When was the symbol last changed?

src/data/format/mse1.cpp

@@ -112,16 +112,16 @@ void read_mse1_card(Set& set, wxFileInputStream& f, wxTextInputStream& file) {
         card->value<TextValue>(_("name"))        .value.assign(line);
         break;
       } case 'C': case 'D': { // image filename
-        String image_file = set.newFileName(_("image"),_("")); // a new unique name in the package
+        LocalFileName image_file = set.newFileName(_("image"),_("")); // a new unique name in the package
         if (wxCopyFile(line, set.nameOut(image_file), true)) {
-          card->value<ImageValue>(_("image"))  .filename = image_file;
+          card->value<ImageValue>(_("image")).filename = image_file;
         }
         break;
       } case 'E':  {  // super type
-        card->value<TextValue>(_("super type"))  .value.assign(line);
+        card->value<TextValue>(_("super type")).value.assign(line);
         break;
       } case 'F': {  // sub type
-        card->value<TextValue>(_("sub type"))    .value.assign(line);
+        card->value<TextValue>(_("sub type")).value.assign(line);
         break;
       } case 'G': {  // casting cost
         card->value<TextValue>(_("casting cost")).value.assign(line);

src/data/format/mtg_editor.cpp

@@ -152,7 +152,7 @@ SetP MtgEditorFileFormat::importSet(const String& filename) {
       }
       // copy image into set
       if (wxFileExists(line)) {
-        String image_file = set->newFileName(_("image"),_(""));
+        LocalFileName image_file = set->newFileName(_("image"),_(""));
         if (wxCopyFile(line, set->nameOut(image_file), true)) {
           current_card->value<ImageValue>(_("image")).filename = image_file;
         }

src/gfx/generated_image.cpp

@@ -444,7 +444,7 @@ bool BuiltInImage::operator == (const GeneratedImage& that) const {
 
 // ----------------------------------------------------------------------------- : SymbolToImage
 
-SymbolToImage::SymbolToImage(bool is_local, const String& filename, Age age, const SymbolVariationP& variation)
+SymbolToImage::SymbolToImage(bool is_local, const LocalFileName& filename, Age age, const SymbolVariationP& variation)
   : is_local(is_local), filename(filename), age(age), variation(variation)
 {}
 SymbolToImage::~SymbolToImage() {}
@@ -480,7 +480,7 @@ bool SymbolToImage::operator == (const GeneratedImage& that) const {
 
 // ----------------------------------------------------------------------------- : ImageValueToImage
 
-ImageValueToImage::ImageValueToImage(const String& filename, Age age)
+ImageValueToImage::ImageValueToImage(const LocalFileName& filename, Age age)
   : filename(filename), age(age)
 {}
 ImageValueToImage::~ImageValueToImage() {}

src/gfx/generated_image.hpp

@@ -10,6 +10,7 @@
 
 #include <util/prec.hpp>
 #include <util/age.hpp>
+#include <util/io/package.hpp>
 #include <gfx/gfx.hpp>
 #include <script/value.hpp>
 
@@ -352,7 +353,7 @@ private:
 /// Use a symbol as an image
 class SymbolToImage : public GeneratedImage {
 public:
-  SymbolToImage(bool is_local, const String& filename, Age age, const SymbolVariationP& variation);
+  SymbolToImage(bool is_local, const LocalFileName& filename, Age age, const SymbolVariationP& variation);
   ~SymbolToImage();
   Image generate(const Options& opt) const override;
   bool operator == (const GeneratedImage& that) const override;
@@ -364,7 +365,7 @@ public:
 private:
   SymbolToImage(const SymbolToImage&); // copy ctor
   bool             is_local; ///< Use local package?
-  String           filename;
+  LocalFileName    filename;
   Age              age;      ///< Age the symbol was last updated
   SymbolVariationP variation;
 };
@@ -374,14 +375,14 @@ private:
 /// Use an image from an ImageValue as an image
 class ImageValueToImage : public GeneratedImage {
 public:
-  ImageValueToImage(const String& filename, Age age);
+  ImageValueToImage(const LocalFileName& filename, Age age);
   ~ImageValueToImage();
   Image generate(const Options& opt) const override;
   bool operator == (const GeneratedImage& that) const override;
   bool local() const override { return true; }
 private:
   ImageValueToImage(const ImageValueToImage&); // copy ctor
-  String filename;
+  LocalFileName filename;
-  Age    age; ///< Age the symbol was last updated
+  Age age; ///< Age the image was last updated
 };
 

src/gui/control/image_card_list.cpp

@@ -50,10 +50,10 @@ ImageFieldP ImageCardList::findImageField() {
 /// A request for a thumbnail of a card image
 class CardThumbnailRequest : public ThumbnailRequest {
   public:
-  CardThumbnailRequest(ImageCardList* parent, const String& filename)
+  CardThumbnailRequest(ImageCardList* parent, const LocalFileName& filename)
     : ThumbnailRequest(
       parent,
-      _("card") + parent->set->absoluteFilename() + _("-") + filename,
+      _("card") + parent->set->absoluteFilename() + _("-") + filename.toStringForKey(),
       wxDateTime::Now())  // TODO: Find mofication time of card image
     , filename(filename)
   {}
@@ -78,23 +78,23 @@ class CardThumbnailRequest : public ThumbnailRequest {
     if (img.Ok()) {
       wxImageList* il = parent->GetImageList(wxIMAGE_LIST_SMALL);
       int id = il->Add(wxBitmap(img));
-      parent->thumbnails.insert(make_pair(filename, id));
+      parent->thumbnails.insert(make_pair(filename.toStringForKey(), id));
       parent->Refresh(false);
     }
   }
 
   virtual bool threadSafe() const {return true;}
-  private:
+private:
-  String filename;
+  LocalFileName filename;
 };
 
 int ImageCardList::OnGetItemImage(long pos) const {
   if (image_field) {
     // Image = thumbnail of first image field of card
     ImageValue& val = static_cast<ImageValue&>(*getCard(pos)->data[image_field]);
-    if (!val.filename) return -1; // no image
+    if (val.filename.empty()) return -1; // no image
     // is there already a thumbnail?
-    map<String,int>::const_iterator it = thumbnails.find(val.filename);
+    map<String,int>::const_iterator it = thumbnails.find(val.filename.toStringForKey());
     if (it != thumbnails.end()) {
       return it->second;
     } else {

src/gui/symbol/window.cpp

@@ -252,7 +252,7 @@ void SymbolWindow::onFileStore(wxCommandEvent& ev) {
   if (performer) {
     SymbolValueP value = static_pointer_cast<SymbolValue>(performer->value);
     Package& package = performer->getLocalPackage();
-    FileName new_filename = package.newFileName(value->field().name,_(".mse-symbol")); // a new unique name in the package
+    LocalFileName new_filename = package.newFileName(value->field().name,_(".mse-symbol")); // a new unique name in the package
     auto stream = package.openOut(new_filename);
     Writer writer(*stream, file_version_symbol);
     writer.handle(control->getSymbol());

src/gui/value/image.cpp

@@ -45,7 +45,7 @@ void ImageValueEditor::sliceImage(const Image& image) {
   // clicked ok?
   if (s.ShowModal() == wxID_OK) {
     // store the image into the set
-    FileName new_image_file = getLocalPackage().newFileName(field().name,_("")); // a new unique name in the package
+    LocalFileName new_image_file = getLocalPackage().newFileName(field().name,_("")); // a new unique name in the package
     Image img = s.getImage();
     img.SaveFile(getLocalPackage().nameOut(new_image_file), wxBITMAP_TYPE_PNG); // always use PNG images, see #69. Disk space is cheap anyway.
     addAction(value_action(valueP(), new_image_file));
@@ -88,7 +88,7 @@ bool ImageValueEditor::doPaste() {
 }
 
 bool ImageValueEditor::doDelete() {
-  addAction(value_action(valueP(), FileName()));
+  addAction(value_action(valueP(), LocalFileName()));
   return true;
 }
 

src/script/functions/image.cpp

@@ -148,13 +148,13 @@ SCRIPT_FUNCTION(symbol_variation) {
   SCRIPT_PARAM(ScriptValueP, symbol); // TODO: change to input?
   ScriptObject<ValueP>* valueO = dynamic_cast<ScriptObject<ValueP>*>(symbol.get());
   SymbolValue* value = valueO ? dynamic_cast<SymbolValue*>(valueO->getValue().get()) : nullptr;
-  String filename;
+  LocalFileName filename;
   if (value) {
     filename = value->filename;
   } else if (valueO) {
     throw ScriptErrorConversion(valueO->typeName(), _TYPE_("symbol" ));
   } else {
-    filename = from_script<String>(symbol);
+    filename = LocalFileName::fromReadString(symbol->toString());
   }
   // known variation?
   SCRIPT_OPTIONAL_PARAM_(String, variation)

src/util/io/get_member.cpp

@@ -18,7 +18,6 @@ using boost::tribool;
 
             void GetDefaultMember::handle(const Char*         v) { value = to_script(v); }
 template <> void GetDefaultMember::handle(const String&       v) { value = to_script(v); }
-template <> void GetDefaultMember::handle(const FileName&     v) { value = to_script(v); }
 template <> void GetDefaultMember::handle(const int&          v) { value = to_script(v); }
 template <> void GetDefaultMember::handle(const unsigned int& v) { value = to_script((int)v); }
 template <> void GetDefaultMember::handle(const double&       v) { value = to_script(v); }

src/util/io/package.cpp

@@ -245,7 +245,7 @@ String Package::nameOut(const String& file) {
   }
 }
 
-FileName Package::newFileName(const String& prefix, const String& suffix) {
+LocalFileName Package::newFileName(const String& prefix, const String& suffix) {
   assert(wxThread::IsMain()); // Writing should only be done from the main thread
   String name;
   UInt infix = 0;
@@ -273,21 +273,23 @@ void Package::referenceFile(const String& file) {
   it->second.keep = true;
 }
 
-String Package::absoluteName(const String& file) {
+// ----------------------------------------------------------------------------- : LocalFileNames and absolute file references
+
+String Package::absoluteName(const LocalFileName& file) {
   assert(wxThread::IsMain());
-  FileInfos::iterator it = files.find(normalize_internal_filename(file));
+  FileInfos::iterator it = files.find(normalize_internal_filename(file.fn));
   if (it == files.end()) {
-    throw FileNotFoundError(file, filename);
+    throw FileNotFoundError(file.fn, filename);
   }
   if (it->second.wasWritten()) {
     // written to this file, return the temp file
     return it->second.tempName;
-  } else if (wxFileExists(filename+_("/")+file)) {
+  } else if (wxFileExists(filename + _("/") + file.fn)) {
     // dir package
-    return filename+_("/")+file;
+    return filename + _("/") + file.fn;
   } else {
     // assume zip package
-    return filename+_("\1")+file;
+    return filename + _("\1") + file.fn;
   }
 }
 // Open a file that is in some package
@@ -306,6 +308,42 @@ unique_ptr<wxInputStream> Package::openAbsoluteFile(const String& name) {
   }
 }
 
+String LocalFileName::toStringForWriting() const {
+  if (!fn.empty() && clipboard_package()) {
+    // use absolute names on clipboard
+    try {
+      return clipboard_package()->absoluteName(fn);
+    } catch (const Error&) {
+      // ignore errors
+      return String();
+    }
+  } else if (!fn.empty() && writing_package()) {
+    writing_package()->referenceFile(fn);
+    return fn;
+  } else {
+    return fn;
+  }
+}
+
+LocalFileName LocalFileName::fromReadString(String const& fn, String const& prefix, String const& suffix) {
+  if (!fn.empty() && clipboard_package()) {
+    // copy file into current package
+    try {
+      LocalFileName local_name = clipboard_package()->newFileName(_("image"),_("")); // a new unique name in the package, assume it's an image
+      auto out_stream = clipboard_package()->openOut(local_name);
+      auto in_stream  = Package::openAbsoluteFile(fn);
+      out_stream->Write(*in_stream); // copy
+      return local_name;
+    } catch (const Error&) {
+      // ignore errors
+      return LocalFileName();
+    }
+  } else {
+    return LocalFileName(fn);
+  }
+}
+
+
 // ----------------------------------------------------------------------------- : Package : private
 
 Package::FileInfo::FileInfo()

src/util/io/package.hpp

@@ -25,6 +25,40 @@ DECLARE_DYNAMIC_ARG(Package*, writing_package);
 /// The package that is being put onto/read from the clipboard
 DECLARE_DYNAMIC_ARG(Package*, clipboard_package);
 
+// ----------------------------------------------------------------------------- : File names
+
+/// A string standing for a filename in a package
+/// Differs from a string because when writing the file should be included in the package
+/// Behaviour is controled by dynamic args:
+///   * clipboard_package
+///   * writing_package
+class LocalFileName {
+public:
+  LocalFileName() {}
+
+  // Convert to a string that can be written to a package.
+  // notifies the package that the underlying file is in use.
+  // when writing to the clipboard, instead returns a global file reference.
+  String toStringForWriting() const;
+  // Construct a LocalFileName based on a string read from a package.
+  // when reading from the clipboard, this will instead be a global file reference, and it is converted at this point.
+  static LocalFileName fromReadString(const String&, const String& prefix = _("image"), String const& suffix = _(""));
+
+  inline bool empty() const {
+    return fn.empty();
+  }
+  inline bool operator == (LocalFileName const& that) const {
+    return this->fn == that.fn;
+  }
+
+  inline String const& toStringForKey() const { return fn; }
+
+private:
+  LocalFileName(const wxString& fn) : fn(fn) {}
+  String fn;
+  friend class Package;
+};
+
 // ----------------------------------------------------------------------------- : Package
 
 /// A package is a container for files. On disk it is either a directory or a zip file.
@@ -100,39 +134,41 @@ class Package : public IntrusivePtrVirtualBase {
 
   /// Open an input stream for a file in the package.
   unique_ptr<wxInputStream> openIn(const String& file);
+  inline unique_ptr<wxInputStream> openIn(const LocalFileName& file) {
+    return openIn(file.fn);
+  }
 
   /// Open an output stream for a file in the package.
   /// (changes are only committed with save())
   unique_ptr<wxOutputStream> openOut(const String& file);
+  inline unique_ptr<wxOutputStream> openOut(const LocalFileName& file) {
+    return openOut(file.fn);
+  }
 
   /// Get a filename that can be written to to modfify a file in the package
   /// (changes are only committed with save())
   String nameOut(const String& file);
+  inline String nameOut(const LocalFileName& file) {
+    return nameOut(file.fn);
+  }
 
   /// Creates a new, unique, filename with the specified prefix and suffix
   /// for example newFileName("image/",".jpg") -> "image/1.jpg"
   /// Returns the name of a temporary file that can be written to.
-  FileName newFileName(const String& prefix, const String& suffix);
+  LocalFileName newFileName(const String& prefix, const String& suffix);
 
   /// Signal that a file is still used by this package.
   /// Must be called for files not opened using openOut/nameOut
   /// If they are to be kept in the package.
   void referenceFile(const String& file);
 
-  /// Get an 'absolute filename' for a file in the package.
-  /// This file can later be opened from anywhere (other process) using openAbsoluteFile()
-  String absoluteName(const String& file);
-
-  /// Open a file given an absolute filename
-  static unique_ptr<wxInputStream> openAbsoluteFile(const String& name);
-
   // --------------------------------------------------- : Managing the inside of the package : Reader/writer
 
   template <typename T>
-  void readFile(const String& file, T& obj);
+  void readFile(const LocalFileName& file, T& obj);
 
   template <typename T>
-  T readFile(const String& file) {
+  T readFile(const LocalFileName& file) {
     T obj;
     readFile(file, obj);
     return obj;
@@ -194,6 +230,14 @@ class Package : public IntrusivePtrVirtualBase {
   void saveToZipfile(const String&,   bool remove_unused, bool is_copy);
   void saveToDirectory(const String&, bool remove_unused, bool is_copy);
   FileInfos::iterator addFile(const String& file);
+
+  /// Get an 'absolute filename' for a file in the package.
+  /// This file can later be opened from anywhere (other process) using openAbsoluteFile()
+  String absoluteName(const LocalFileName& file);
+
+  /// Open a file given an absolute filename
+  static unique_ptr<wxInputStream> openAbsoluteFile(const String& name);
+  friend class LocalFileName;
 };
 
 // ----------------------------------------------------------------------------- : Packaged
@@ -291,13 +335,13 @@ intrusive_ptr<T> open_package(const String& filename) {
 
 // This is here because it uses dynamic_cast and must be to a complete type.
 template <typename T>
-inline void Package::readFile(const String& file, T& obj) {
+inline void Package::readFile(const LocalFileName& file, T& obj) {
   auto stream = openIn(file);
-  Reader reader(*stream, dynamic_cast<Packaged*>(this), absoluteFilename() + _("/") + file);
+  Reader reader(*stream, dynamic_cast<Packaged*>(this), absoluteFilename() + _("/") + file.fn);
   try {
     reader.handle_greedy(obj);
   } catch (const ParseError& err) {
-    throw FileParseError(err.what(), absoluteFilename() + _("/") + file); // more detailed message
+    throw FileParseError(err.what(), absoluteFilename() + _("/") + file.fn); // more detailed message
   }
 }
 

src/util/io/reader.cpp

@@ -354,26 +354,8 @@ template <> void Reader::handle(Vector2D& vec) {
   }
 }
 
-template <> void Reader::handle(FileName& f) {
+template <> void Reader::handle(LocalFileName& f) {
-  if (clipboard_package()) {
+  f = LocalFileName::fromReadString(this->getValue());
-    String str = getValue();
-    if (!str.empty()) {
-      // copy file into current package
-      try {
-        String packaged_name = clipboard_package()->newFileName(_("image"),_("")); // a new unique name in the package, assume it's an image
-        auto out_stream = clipboard_package()->openOut(packaged_name);
-        auto in_stream  = Package::openAbsoluteFile(str);
-        out_stream->Write(*in_stream); // copy
-        f.assign(packaged_name);
-      } catch (Error const&) {
-        // ignore errors
-      }
-    } else {
-      f.assign(str);
-    }
-  } else {
-    handle(static_cast<String&>(f));
-  }
 }
 
 // ----------------------------------------------------------------------------- : EnumReader

src/util/io/writer.cpp

@@ -131,18 +131,6 @@ template <> void Writer::handle(const Vector2D& vec) {
   handle(String::Format(_("(%.10lf,%.10lf)"), vec.x, vec.y));
 }
 
-template <> void Writer::handle(const FileName& value) {
+template <> void Writer::handle(const LocalFileName& value) {
-  if (clipboard_package() && !value.empty()) {
+  handle(value.toStringForWriting());
-    // use absolute names on clipboard
-    try {
-      handle(clipboard_package()->absoluteName(value));
-    } catch (const Error&) {
-      // ignore errors
-    }
-  } else {
-    handle(static_cast<const String&>(value));
-    if (writing_package()) {
-      writing_package()->referenceFile(value);
-    }
-  }
 }

src/util/prec.hpp

@@ -84,13 +84,6 @@ typedef wxOutputStream OutputStream;
 typedef unsigned char Byte;
 typedef unsigned int  UInt;
 
-/// A string standing for a filename, has different behaviour when reading/writing
-class FileName : public wxString {
-  public:
-  FileName()                                {}
-  FileName(const wxString& s) : wxString(s) {}
-};
-
 // ----------------------------------------------------------------------------- : MSE Headers
 
 // MSE utility headers (ones unlikely to change and used everywhere)