Save packed project feature by glebmish · Pull Request #1345 · appleseedhq/appleseed

glebmish · 2017-03-22T13:41:54Z

Added minizip library sources needed to create zip files and wrapped it with zipper.cpp which is renamed from unzipper.cpp and became wrapper over all minizip library.
Implemented ability to save packed projects to projectfilewriter.cpp. Refactored it to consider file extension to choose between saving plain and packed project
Added ability to save appleseedz in UI

Source files needed for creating zip files

Wrapper allows to zip all files provided in filenames_to_zip to zip_filename archive. Used for saving packed projects.

dictoon

Top quality work as usual!

There is just a number of mostly cometics details to address before we can merge this.

Thanks!

dictoon · 2017-03-22T14:31:59Z

src/appleseed/foundation/utility/zipper.cpp

+
+}   // namespace foundation
+
+


Remove one redundant blank line.

dictoon · 2017-03-22T14:32:10Z

src/appleseed/foundation/utility/zipper.cpp

+
+namespace foundation
+{
+


Remove blank line.

dictoon · 2017-03-22T14:33:51Z

src/appleseed/foundation/utility/zipper.h

+    class ZipException
+            : public foundation::Exception
+    {
+    public:


Indent with two spaces.

dictoon · 2017-03-22T14:44:07Z

src/appleseed.studio/mainwindow/mainwindow.cpp

            this,
            "Save As...",
-            "Project Files (*.appleseed)",
+            "Project Files (*.appleseed);;Packed project files (*.appleseedz)",


Use title capitalization: Packed Project Files

Also rename Project Files to Plain Project Files.

dictoon · 2017-03-22T14:45:59Z

src/appleseed.studio/utility/miscellaneous.cpp

    }
 }

+QString get_extension(ParamArray& settings, const QString& target_dialog) {


Braces always go on their own line.

dictoon · 2017-03-22T15:11:33Z

src/appleseed/renderer/modeling/project/projectfilewriter.cpp

+
+            bf::create_directory(temp_project_filepath.parent_path());
+
+            bool success = write_project_file(project,


Make success const.

Fix indentation, I suggest this:

const bool success = write_project_file( project, temp_project_filepath.c_str(), options | ProjectFileWriter::CopyAllAssets);

dictoon · 2017-03-22T15:11:55Z

src/appleseed/renderer/modeling/project/projectfilewriter.cpp

+                                      options | ProjectFileWriter::CopyAllAssets);
+            if (!success)
+            {
+                RENDERER_LOG_ERROR("Failed to save project %s.", filepath);


Use lower case for log messages (fix everywhere).

dictoon · 2017-03-22T15:28:20Z

src/appleseed/renderer/modeling/project/projectfilewriter.h


 class APPLESEED_DLLSYMBOL ProjectFileWriter
 {
+  private:


Private declarations go after public ones.

dictoon · 2017-03-22T15:28:29Z

src/appleseed/renderer/modeling/project/projectfilewriter.h

 class APPLESEED_DLLSYMBOL ProjectFileWriter
 {
+  private:
+    // Write a project file to disk. Return true on success, false otherwise


End full line comments with a period (fix everywhere).

dictoon · 2017-03-22T15:28:34Z

src/appleseed/renderer/modeling/project/projectfilewriter.h

+  private:
+    // Write a project file to disk. Return true on success, false otherwise
+    static bool write_project_file(
+            const Project&  project,


Fix indentation.

dictoon · 2017-03-22T15:33:11Z

Ah, you'll also need to fix the build, Travis complains with gcc 6. The error message is interesting :)

dictoon · 2017-03-23T09:35:11Z

src/appleseed/foundation/utility/minizip/zip.c


-      if (uPosFound!=0)
-        break;
+        if (uPosFound != 0)


I believe you introduced a bug here. I think the initial logic was correct, there was just an indentation problem that was tripping the compiler (one reason why we're so strict on indentation!)

dictoon · 2017-03-23T09:35:51Z

src/appleseed/foundation/utility/zipper.h

+  const std::string& extension);
+
+//
+// Retrieves


Re-unite the two pieces of the sentence into one?

dictoon · 2017-03-23T09:36:30Z

src/appleseed/renderer/modeling/project/projectfilewriter.cpp

-        const Project&  project,
-        const char*     filepath,
-        const int       options)
+  const Project&  project,


Please follow the indentation style of the other functions: arguments are indented normally (4 spaces).

dictoon · 2017-03-23T09:36:53Z

src/appleseed/renderer/modeling/project/projectfilewriter.h

+  private:
+    // Write a project file to disk. Return true on success, false otherwise.
+    static bool write_project_file(
+      const Project&  project,


Follow the indentation style of the other functions.

Sorry for this indentation issues, I'm setting up indentation settings in IDE so it corresponds to your codestyle.
What should be indented with 4 spaces and what with 2?
As I see now:

In constructor before ':' it should be 2 spaces

In multi-line function invocation it should be 4

In multi-line function declaration it should be 4 as well

So, the general rule for indentation is 4 spaces, no tabs (ever).

The only things that are indented with 2 spaces are:

initializer and inheritance lists

public, protected, private, case and default keywords

Everything else is indented with 4 spaces.

Example:

class Derived : public Base { public: Derived() : Base(11) { } void foo( const int y, const int z); private: int bar(const int x) { switch (x) { case 1: return 2; case 2: return 4; default: return 0; } } };

glebmish · 2017-03-23T13:40:24Z

Thanks for detailed comments!
Hope it's okay now

dictoon · 2017-03-23T13:42:58Z

It is, thanks! Will merge once Travis passed.

glebmish · 2017-03-23T13:43:53Z

Great!
Later I'll make tests and save scenario we've discussed in slack and submit it with other PRs

glebmish added 10 commits March 21, 2017 20:37

Add source files of minizip library

89ecbbd

Source files needed for creating zip files

Minor changes to unzipper

d667b5a

Implement wrapper over minizip zip functions

67653df

Wrapper allows to zip all files provided in filenames_to_zip to zip_filename archive. Used for saving packed projects.

Merged unzipper files to zipper

3014bec

Change slot_save_project_as() to consider file extension

2e86507

Intoduced appleseedz extension to save as... dialog

c685a89

Change zip() function to accept directory as argument

c6095c2

Introduce saving packed projects

23e8434

Rename test_unzipper.cpp to test_zipper.cpp

dfa50d9

Fix codestyle issues before PR

80918f2

dictoon requested changes Mar 22, 2017

View reviewed changes

Fix codestyle issues

af60f54

dictoon reviewed Mar 23, 2017

View reviewed changes

dictoon requested changes Mar 23, 2017

View reviewed changes

glebmish added 2 commits March 23, 2017 12:40

Removed wrong curly braces

92d966a

Fix codestyle issues

d733b24

dictoon approved these changes Mar 23, 2017

View reviewed changes

dictoon merged commit a7210ca into appleseedhq:master Mar 23, 2017

glebmish deleted the save-packed-project branch May 31, 2017 19:25


		bf::create_directory(temp_project_filepath.parent_path());

		bool success = write_project_file(project,

Conversation

glebmish commented Mar 22, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dictoon left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dictoon commented Mar 22, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

glebmish Mar 23, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dictoon Mar 23, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

glebmish commented Mar 23, 2017

Uh oh!

dictoon commented Mar 23, 2017

Uh oh!

glebmish commented Mar 23, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

glebmish commented Mar 22, 2017 •

edited

Loading

glebmish Mar 23, 2017 •

edited

Loading

dictoon Mar 23, 2017 •

edited

Loading