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

Recover file fixes #3722

Merged
merged 1 commit into from Jul 30, 2017

Conversation

Projects
None yet
2 participants
@zonkmachine
Member

zonkmachine commented Jul 23, 2017

I'll collect a bunch of, hopefully, last tweaks to the recover file system here.

  • Remove extra calls to autoSave() and a separate function runAutoSave() which was only related to the Limited Session that was dropped from the code in 290556e.

  • Don't auto-save while playing by default. ( From criticism after RC3 where it created issues with glitches for users with weaker machines )

@zonkmachine zonkmachine added this to the 1.2.0 milestone Jul 23, 2017

Fixes to recover file system
Don't auto-save while playing by default. On weaker machines (xp?) we
see glitches so better turn this on after need.

Remove the last of Limited Sessin which was removed in 290556e.

@zonkmachine zonkmachine merged commit 2e841e4 into LMMS:stable-1.2 Jul 30, 2017

1 check passed

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

zonkmachine added a commit that referenced this pull request Jul 30, 2017

Fixes to recover file system (#3722)
Don't auto-save while playing by default. On weaker machines (xp?) we
see glitches so better turn this on after need.

Remove the last of Limited Sessin which was removed in 290556e.

[cherry-picked from stable-1.2]

@zonkmachine zonkmachine deleted the zonkmachine:recoverfilefixup branch Jul 30, 2017

@tresf

This comment has been minimized.

Show comment
Hide comment
@tresf

tresf Jul 30, 2017

Member

This PR had an unanswered question when it was merged.

Member

tresf commented Jul 30, 2017

This PR had an unanswered question when it was merged.

@zonkmachine

This comment has been minimized.

Show comment
Hide comment
@zonkmachine

zonkmachine Jul 30, 2017

Member

I missed that. Where? What was the question?

Member

zonkmachine commented Jul 30, 2017

I missed that. Where? What was the question?

@tresf

This comment has been minimized.

Show comment
Hide comment
@tresf

tresf Jul 30, 2017

Member

I missed that. Where? What was the question?

It's a commit review, so I can't hard-link it. Please scroll through the commit code, you can't miss it.

Member

tresf commented Jul 30, 2017

I missed that. Where? What was the question?

It's a commit review, so I can't hard-link it. Please scroll through the commit code, you can't miss it.

@zonkmachine

This comment has been minimized.

Show comment
Hide comment
@zonkmachine

zonkmachine Jul 30, 2017

Member

Sorry, I force pushed and the comments seem gone. I can't find them in my mail either.

Member

zonkmachine commented Jul 30, 2017

Sorry, I force pushed and the comments seem gone. I can't find them in my mail either.

@tresf

This comment has been minimized.

Show comment
Hide comment
@tresf
Member

tresf commented Jul 30, 2017

screenshot_20170730-130535

@zonkmachine

This comment has been minimized.

Show comment
Hide comment
@zonkmachine

zonkmachine Jul 31, 2017

Member

When a review says Pending, it is only visible to you.
https://help.github.com/articles/reviewing-proposed-changes-in-a-pull-request/

Let's see, 7 days ago (review date) that really was a one line fix and the rest is removing old logic from the defunct Limited Session. I introduced void MainWindow::runAutoSave() to enforce saving whenever a new project was started to ensure there was a recovery.mmp. This would just be confusing to leave in as I've seen questions about why the recovery.mmp is spamming...

If you mean the last update from today before I rebased the PR, no. If you just change the bool "1" the update function still is hard coded "true" so I did it this way. Still hard coded but now false.

Member

zonkmachine commented Jul 31, 2017

When a review says Pending, it is only visible to you.
https://help.github.com/articles/reviewing-proposed-changes-in-a-pull-request/

Let's see, 7 days ago (review date) that really was a one line fix and the rest is removing old logic from the defunct Limited Session. I introduced void MainWindow::runAutoSave() to enforce saving whenever a new project was started to ensure there was a recovery.mmp. This would just be confusing to leave in as I've seen questions about why the recovery.mmp is spamming...

If you mean the last update from today before I rebased the PR, no. If you just change the bool "1" the update function still is hard coded "true" so I did it this way. Still hard coded but now false.

@tresf

This comment has been minimized.

Show comment
Hide comment
@tresf

tresf Jul 31, 2017

Member

When a review says Pending, it is only visible to you.
https://help.github.com/articles/reviewing-proposed-changes-in-a-pull-request/

Thanks that explains it.

So when does auto-save kick in? Is it running in the background? What triggers it? Yes, the spamming of auto-save files is annoying and I tend to agree with that observation, but in my opinion, that's a failure to clean-up after successful save (or when explicitly click "Do not save").

If this feature causes performance issues, it's likely a threading prioritization issue.

Member

tresf commented Jul 31, 2017

When a review says Pending, it is only visible to you.
https://help.github.com/articles/reviewing-proposed-changes-in-a-pull-request/

Thanks that explains it.

So when does auto-save kick in? Is it running in the background? What triggers it? Yes, the spamming of auto-save files is annoying and I tend to agree with that observation, but in my opinion, that's a failure to clean-up after successful save (or when explicitly click "Do not save").

If this feature causes performance issues, it's likely a threading prioritization issue.

@zonkmachine

This comment has been minimized.

Show comment
Hide comment
@zonkmachine

zonkmachine Jul 31, 2017

Member

So when does auto-save kick in? Is it running in the background? What triggers it?

The timer is started here and the first save now is done after the first time-out of m_autoSaveTimer.

Two of the most common complaints around the auto-save function are both user related:

  1. Error message: 'Could not open /recover.mmp for writing...'
    Most likely from the user setting the working directory to '/'.
  2. User doesn't save at all but uses auto-save as backup and therefore doesn't have any real safety net so when the crash finally comes they hit the forum with a post like: 'I had the auto-save on and worked for 2 days without turning the computer off but then it crashed and all was lost... How do I get my project back?'
Member

zonkmachine commented Jul 31, 2017

So when does auto-save kick in? Is it running in the background? What triggers it?

The timer is started here and the first save now is done after the first time-out of m_autoSaveTimer.

Two of the most common complaints around the auto-save function are both user related:

  1. Error message: 'Could not open /recover.mmp for writing...'
    Most likely from the user setting the working directory to '/'.
  2. User doesn't save at all but uses auto-save as backup and therefore doesn't have any real safety net so when the crash finally comes they hit the forum with a post like: 'I had the auto-save on and worked for 2 days without turning the computer off but then it crashed and all was lost... How do I get my project back?'
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment