Add warning when loading unsupported property type and mark the file as read-only (fix #3812, fix #3811)

data/strings/en.ini

@@ -41,6 +41,7 @@ non_transformable_reference_layer = Layer '{}' is reference, cannot be transform
 sprite_locked_somewhere = The sprite is locked in other editor
 not_enough_transform_memory = Not enough memory to transform the selection
 not_enough_rotsprite_memory = Not enough memory for RotSprite
+unmodifiable_sprite = Read-only sprite cannot be modified
 
 [alerts]
 applying_filter = FX<<Applying effect...||&Cancel
@@ -217,6 +218,17 @@ Information
 <<Activating Aseprite will give you access to automatic updates.
 ||&OK
 END
+load_file_with_incompatibilities = <<<END
+Incompatibility error found:
+
+
+{0}.
+
+
+This file will be opened as read-only. Please upgrade Aseprite to the latest version to be able to save changes without losing data.
+END
+cannot_overwrite_readonly = Cannot save/overwrite a read-only sprite. Use File > Save As option.
+cannot_modify_readonly = Cannot modify a read-only sprite.
 
 [brightness_contrast]
 title = Brightness/Contrast

src/app/app.cpp

@@ -766,6 +766,9 @@ void App::updateDisplayTitleBar()
   if (docView) {
     // Prepend the document's filename.
     title += docView->document()->name();
+    if (docView->document()->isReadOnly()) {
+      title += " [Read-Only]";
+    }
     title += " - ";
   }
 

src/app/commands/cmd_play_animation.cpp

@@ -41,7 +41,7 @@ PlayAnimationCommand::PlayAnimationCommand()
 
 bool PlayAnimationCommand::onEnabled(Context* ctx)
 {
-  return ctx->checkFlags(ContextFlags::ActiveDocumentIsWritable |
+  return ctx->checkFlags(ContextFlags::ActiveDocumentIsReadable |
                          ContextFlags::HasActiveSprite);
 }
 

src/app/commands/cmd_save_file.cpp

@@ -162,6 +162,12 @@ std::string SaveFileBaseCommand::saveAsDialog(
   if (filename.empty())
     return std::string();
 
+  // Document is being saved under a different path/name, remove
+  // read-only mark then.
+  if (filename != initialFilename) {
+    document->removeReadOnlyMark();
+  }
+
   if (saveInBackground == SaveInBackground::On) {
     saveDocumentInBackground(
       context, document,
@@ -238,8 +244,10 @@ void SaveFileBaseCommand::saveDocumentInBackground(
     console.printf(fop->error().c_str());
 
     // We don't know if the file was saved correctly or not. So mark
-    // it as it should be saved again.
+    // it as it should be saved again. Except for read-only documents,
-    document->impossibleToBackToSavedState();
+    // since we know they must not be saved.
+    if (!document->isReadOnly())
+      document->impossibleToBackToSavedState();
   }
   // If the job was cancelled, mark the document as modified.
   else if (fop->isStop()) {
@@ -315,6 +323,7 @@ public:
 
 protected:
   void onExecute(Context* context) override;
+  bool onEnabled(Context* context) override;
 };
 
 SaveFileAsCommand::SaveFileAsCommand()
@@ -331,6 +340,11 @@ void SaveFileAsCommand::onExecute(Context* context)
                MarkAsSaved::On);
 }
 
+bool SaveFileAsCommand::onEnabled(Context* context)
+{
+  return context->checkFlags(ContextFlags::ActiveDocumentIsReadable);
+}
+
 class SaveFileCopyAsCommand : public SaveFileBaseCommand {
 public:
   SaveFileCopyAsCommand();

src/app/context_flags.cpp

@@ -44,7 +44,7 @@ void ContextFlags::update(Context* context)
 
       updateFlagsFromSite(site);
 
-      if (document->canWriteLockFromRead())
+      if (document->canWriteLockFromRead() && !document->isReadOnly())
         m_flags |= ActiveDocumentIsWritable;
 
       document->unlock();

src/app/doc.cpp

@@ -332,6 +332,25 @@ bool Doc::isFullyBackedUp() const
   return (m_flags & kFullyBackedUp ? true: false);
 }
 
+void Doc::markAsReadOnly()
+{
+  DOC_TRACE("DOC: Mark as read-only", this);
+
+  m_flags |= kReadOnly;
+}
+
+bool Doc::isReadOnly() const
+{
+  return (m_flags & kReadOnly ? true: false);
+}
+
+void Doc::removeReadOnlyMark()
+{
+  DOC_TRACE("DOC: Read-only mark removed", this);
+
+  m_flags &= ~kReadOnly;
+}
+
 //////////////////////////////////////////////////////////////////////
 // Loaded options from file
 

src/app/doc.h

@@ -12,8 +12,10 @@
 #include "app/doc_observer.h"
 #include "app/extra_cel.h"
 #include "app/file/format_options.h"
+#include "app/i18n/strings.h"
 #include "app/transformation.h"
 #include "base/disable_copying.h"
+#include "base/exception.h"
 #include "base/rw_lock.h"
 #include "doc/blend_mode.h"
 #include "doc/color.h"
@@ -64,6 +66,7 @@ namespace app {
       kMaskVisible      = 2, // The mask wasn't hidden by the user
       kInhibitBackup    = 4, // Inhibit the backup process
       kFullyBackedUp    = 8, // Full backup was done
+      kReadOnly         = 16,// This document is read-only
     };
   public:
     Doc(Sprite* sprite);
@@ -146,6 +149,10 @@ namespace app {
     void markAsBackedUp();
     bool isFullyBackedUp() const;
 
+    void markAsReadOnly();
+    bool isReadOnly() const;
+    void removeReadOnlyMark();
+
     //////////////////////////////////////////////////////////////////////
     // Loaded options from file
 
@@ -268,6 +275,14 @@ namespace app {
     DISABLE_COPYING(Doc);
   };
 
+  // Exception thrown when we want to modify a sprite (add new
+  // app::Cmd objects) marked as read-only.
+  class CannotModifyWhenReadOnlyException : public base::Exception {
+  public:
+    CannotModifyWhenReadOnlyException() throw()
+    : base::Exception(Strings::alerts_cannot_modify_readonly()) { }
+  };
+
 } // namespace app
 
 #endif

src/app/file/ase_format.cpp

@@ -54,6 +54,10 @@ public:
     m_fop->setError(msg.c_str());
   }
 
+  void incompatibilityError(const std::string& msg) override {
+    m_fop->setIncompatibilityError(msg.c_str());
+  }
+
   void progress(double fromZeroToOne) override {
     m_fop->setProgress(fromZeroToOne);
   }
@@ -289,7 +293,7 @@ bool AseFormat::onLoad(FileOp* fop)
     return false;
 
   Sprite* sprite = delegate.sprite();
-  fop->createDocument(sprite);
+  fop->createDocument(sprite, fop->hasIncompatibilityError());
 
   if (sprite->colorSpace() != nullptr &&
       sprite->colorSpace()->type() != gfx::ColorSpace::None) {

src/app/file/file.cpp

@@ -920,6 +920,11 @@ void FileOp::operate(IFileOpProgress* progress)
         setError("Error loading data file: %s\n", ex.what());
       }
     }
+
+    if (hasIncompatibilityError()) {
+      setError(fmt::format(Strings::alerts_load_file_with_incompatibilities(),
+               m_incompatibilityError).c_str());
+    }
   }
   // Save //////////////////////////////////////////////////////////////////////
   else if (m_type == FileOpSave &&
@@ -936,6 +941,11 @@ void FileOp::operate(IFileOpProgress* progress)
     }
 #endif
 
+    if (m_document && m_document->isReadOnly()) {
+      setError(fmt::format(Strings::alerts_cannot_overwrite_readonly()).c_str());
+      return;
+    }
+
     // Save a sequence
     if (isSequence()) {
       ASSERT(m_format->support(FILE_SUPPORT_SEQUENCES));
@@ -1081,12 +1091,14 @@ FileOp::~FileOp()
   delete m_seq.palette;
 }
 
-void FileOp::createDocument(Sprite* spr)
+void FileOp::createDocument(Sprite* spr, bool readOnly)
 {
   // spr can be NULL if the sprite is set in onPostLoad() then
 
   ASSERT(m_document == NULL);
   m_document = new Doc(spr);
+  if (readOnly)
+    m_document->markAsReadOnly();
 }
 
 void FileOp::postLoad()
@@ -1362,6 +1374,18 @@ void FileOp::setError(const char *format, ...)
   }
 }
 
+void FileOp::setIncompatibilityError(const std::string& msg)
+{
+  // Concatenate the new error
+  {
+    std::lock_guard lock(m_mutex);
+    // Add a newline char automatically if it's needed
+    if (!m_incompatibilityError.empty() && m_incompatibilityError.back() != '\n')
+      m_incompatibilityError.push_back('\n');
+    m_incompatibilityError += msg;
+  }
+}
+
 void FileOp::setProgress(double progress)
 {
   std::lock_guard lock(m_mutex);

src/app/file/file.h

@@ -168,7 +168,7 @@ namespace app {
 
     const FileOpROI& roi() const { return m_roi; }
 
-    void createDocument(Sprite* spr);
+    void createDocument(Sprite* spr, bool readOnly = false);
     void operate(IFileOpProgress* progress = nullptr);
 
     void done();
@@ -247,6 +247,8 @@ namespace app {
     const std::string& error() const { return m_error; }
     void setError(const char *error, ...);
     bool hasError() const { return !m_error.empty(); }
+    void setIncompatibilityError(const std::string& msg);
+    bool hasIncompatibilityError() const { return !m_incompatibilityError.empty(); }
 
     double progress() const;
     void setProgress(double progress);
@@ -283,6 +285,7 @@ namespace app {
     double m_progress;          // Progress (1.0 is ready).
     IFileOpProgress* m_progressInterface;
     std::string m_error;        // Error string.
+    std::string m_incompatibilityError; // Incompatibility error string.
     bool m_done;                // True if the operation finished.
     bool m_stop;                // Force the break of the operation.
     bool m_oneframe;            // Load just one frame (in formats

src/app/transaction.cpp

@@ -140,6 +140,12 @@ void Transaction::rollback(CmdTransaction* newCmds)
 
 void Transaction::execute(Cmd* cmd)
 {
+  // Read-only sprites cannot be modified.
+  if (m_doc->isReadOnly()) {
+    delete cmd;
+    throw CannotModifyWhenReadOnlyException();
+  }
+
   // If we are undoing/redoing, just throw an exception, we cannot
   // modify the sprite while we are moving throw the undo history.
   // To undo/redo we have just to call the onUndo/onRedo of each

src/app/ui/editor/editor.cpp

@@ -2418,7 +2418,8 @@ bool Editor::canDraw()
           m_layer->isImage() &&
           m_layer->isVisibleHierarchy() &&
           m_layer->isEditableHierarchy() &&
-          !m_layer->isReference());
+          !m_layer->isReference() &&
+          !m_document->isReadOnly());
 }
 
 bool Editor::isInsideSelection()

src/app/ui/editor/tool_loop_impl.cpp

@@ -783,6 +783,14 @@ tools::ToolLoop* create_tool_loop(
   Site site = editor->getSite();
   doc::Grid grid = site.grid();
 
+  // If the document is read-only.
+  if (site.document()->isReadOnly()) {
+    StatusBar::instance()->showTip(
+      1000,
+      fmt::format(Strings::statusbar_tips_unmodifiable_sprite()));
+    return nullptr;
+  }
+
   ToolLoopParams params;
   params.tool = editor->getCurrentEditorTool();
   params.ink = editor->getCurrentEditorInk();

src/dio/aseprite_decoder.cpp

@@ -1295,20 +1295,26 @@ void AsepriteDecoder::readPropertiesMaps(doc::UserData::PropertiesMaps& properti
   auto startPos = f()->tell();
   auto size = read32();
   auto numMaps = read32();
-  for (int i=0; i<numMaps; ++i) {
+  try {
-    auto id = read32();
+    for (int i=0; i<numMaps; ++i) {
-    std::string extensionId; // extensionId = empty by default (when id == 0)
+      auto id = read32();
-    if (id &&
+      std::string extensionId; // extensionId = empty by default (when id == 0)
-        !extFiles.getFilenameByID(id, extensionId)) {
+      if (id &&
-      // This shouldn't happen, but if it does, we put the properties
+          !extFiles.getFilenameByID(id, extensionId)) {
-      // in an artificial extensionId.
+        // This shouldn't happen, but if it does, we put the properties
-      extensionId = fmt::format("__missed__{}", id);
+        // in an artificial extensionId.
-      delegate()->error(
+        extensionId = fmt::format("__missed__{}", id);
-        fmt::format("Error: Invalid extension ID (id={0} not found)", id));
+        delegate()->error(
+          fmt::format("Error: Invalid extension ID (id={0} not found)", id));
+      }
+      auto properties = readPropertyValue(USER_DATA_PROPERTY_TYPE_PROPERTIES);
+      propertiesMaps[extensionId] = doc::get_value<doc::UserData::Properties>(properties);
     }
-    auto properties = readPropertyValue(USER_DATA_PROPERTY_TYPE_PROPERTIES);
-    propertiesMaps[extensionId] = doc::get_value<doc::UserData::Properties>(properties);
   }
+  catch(const base::Exception& e) {
+    delegate()->incompatibilityError(fmt::format("Error reading custom properties: {0}", e.what()));
+  }
+
   f()->seek(startPos+size);
 }
 
@@ -1420,6 +1426,9 @@ const doc::UserData::Variant AsepriteDecoder::readPropertyValue(uint16_t type)
       }
       return value;
     }
+    default: {
+      throw base::Exception(fmt::format("Unexpected property type '{0}' at file position {1}", type, f()->tell()));
+    }
   }
 
   return doc::UserData::Variant{};

src/dio/decode_delegate.h

@@ -24,6 +24,10 @@ public:
   // Used to log errors
   virtual void error(const std::string& msg) { }
 
+  // Sets an error when an incompatibility issue is
+  // detected.
+  virtual void incompatibilityError(const std::string &msg) { }
+
   // Used to report progress of the whole operation
   virtual void progress(double fromZeroToOne) { }