Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extract pclx to a random generated folder name to avoid name conflicts #1043

Merged
merged 6 commits into from Aug 16, 2018

Conversation

Projects
None yet
3 participants
@chchwy
Copy link
Member

commented Aug 7, 2018

Related issues #932 #752

The issue described by @scribblemaniac :
If a user opens the same project twice in a row, Penci2D will:

  1. Create data folder for file to load
  2. Load everything into the data folder
  3. Stop if there is an error
  4. Switch to using the new data folder
  5. Delete the old data folder

However, if the old data folder and new data folder are the same, then step 5 ends up deleting all the data from our project!

In order to resolve the name conflicts, the data folder will include an 8 character long random string.

For example:

  • YourProject_Y2xD_6xseyd51
  • Default_Y2xD_5zy4hsui

@chchwy chchwy requested a review from scribblemaniac Aug 7, 2018

Extract pclx to the data folder with random generated folder name to …
…avoid folder name conflicts.

If a user opens the same project twice in a row, Penci2D will:

1. Create data folder for file to load
2. Load everything into the data folder
3. Stop if there is an error
4. Switch to using the new data folder
5. Delete the old data folder

However, if the old data folder and new data folder are the same, then step 5 ends up deleting all the data from our project! (by scribblemaniac)

@chchwy chchwy force-pushed the chchwy:fix/load-twice-wipe branch from a5deac9 to aec2652 Aug 7, 2018

+ strFolderName
+ PFF_TMP_DECOMPRESS_EXT
+ "/";
const QString strWorkingDir =

This comment has been minimized.

Copy link
@scribblemaniac

scribblemaniac Aug 8, 2018

Member

For my peace of mind, could we change the following few lines to this:

QString strWorkingDir;
QDir dir(QDir::tempPath());
do {
    strWorkingDir =
        QString("%1/Pencil2D/%2_%3_%4/").arg(QDir::tempPath()).arg(strFolderName).arg(PFF_TMP_DECOMPRESS_EXT).arg(uniqueString(8));
} while(dir.exists(strWorkingDir));

The chance of generating the same string is almost impossible, but I would feel better if we checked for it anyway.

This comment has been minimized.

Copy link
@chchwy

chchwy Aug 9, 2018

Author Member

Done. thanks for reminding.

+ strFolderName
+ PFF_TMP_DECOMPRESS_EXT
+ "/";
const QString strWorkingDir =

This comment has been minimized.

Copy link
@scribblemaniac

scribblemaniac Aug 8, 2018

Member

The default folders are not getting deleted. This is because the Object destructor only calls deleteWorkingDir if filePath() is empty. Instead I think we should always call deleteWorkingDir, and just add a check in that function to make sure that mWorkingDirPath is not empty.

On a slightly unrelated note, is there any reason why we don't just call init in the Object constructor? That way we can be sure that it is called at least once.

This comment has been minimized.

Copy link
@chchwy

chchwy Aug 9, 2018

Author Member

The behaviours were introduced in a PR #591, I don't really remember why it's working in this way. So the working directory will be deleted anyway now.

For my understanding, Object::init() is for a new Object. In Pencil2D it's the starting default project and the project when you click from the menu File->New. It's no harm to call init() on an Object from a pclx file but all data will be overwritten immediately.

@J5lx

This comment has been minimized.

Copy link
Member

commented Aug 8, 2018

Seems good to me except for what scribblemaniac noted. Also, not directly related to the PR, but what does Y2xC and Y2xD stand for? It seems the last letter distinguishes between (de)compression, but what about the rest? Just curious.

@chchwy

This comment has been minimized.

Copy link
Member Author

commented Aug 9, 2018

Y2xC is legacy and not used anymore. I deleted it. Preserved Y2xD for the sake of debugging. I can quickly check all pencil2d working directories on my computer by a keyword.

@chchwy chchwy merged commit 504280a into pencil2d:master Aug 16, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@chchwy chchwy added this to the 0.6.2 milestone Sep 25, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.