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

Fix crash when loading project with missing peak controller effect #4391

Merged
merged 2 commits into from Jun 13, 2018

Conversation

Projects
None yet
2 participants
@PhysSong
Member

PhysSong commented Jun 3, 2018

Fixes #4060.

@PhysSong

This comment has been minimized.

Show comment
Hide comment
@PhysSong

PhysSong Jun 3, 2018

Member

Loading such project will create dummy connections where the broken connection was. Would it be better to remove them or leave them? Leaving will let user easily notice there were connections, but it leads to weird connections when re-saved.
I will add a check for dummy connection in saving code, and removal of them if needed.

Member

PhysSong commented Jun 3, 2018

Loading such project will create dummy connections where the broken connection was. Would it be better to remove them or leave them? Leaving will let user easily notice there were connections, but it leads to weird connections when re-saved.
I will add a check for dummy connection in saving code, and removal of them if needed.

@zonkmachine

This comment has been minimized.

Show comment
Hide comment
@zonkmachine

zonkmachine Jun 3, 2018

Member

I'm only able to reproduce #4060 with the AppImages but the test build of this PR worked at a more casual test. I have problems building right now but may test this more later. I don't know why I see #4060 only with the AppImages.

Member

zonkmachine commented Jun 3, 2018

I'm only able to reproduce #4060 with the AppImages but the test build of this PR worked at a more casual test. I have problems building right now but may test this more later. I don't know why I see #4060 only with the AppImages.

@PhysSong

This comment has been minimized.

Show comment
Hide comment
@PhysSong

PhysSong Jun 4, 2018

Member

Issues with dummy connections:

  • Lock model value
  • Cause wrong connections to be added when saving/reopening

So I think broken connections should be removed when loading project.

Member

PhysSong commented Jun 4, 2018

Issues with dummy connections:

  • Lock model value
  • Cause wrong connections to be added when saving/reopening

So I think broken connections should be removed when loading project.

@PhysSong PhysSong added the in progress label Jun 6, 2018

@PhysSong PhysSong removed the in progress label Jun 6, 2018

@zonkmachine

This comment has been minimized.

Show comment
Hide comment
@zonkmachine

zonkmachine Jun 10, 2018

Member

I've tested this some more and the demo files from #4060 opens up with no problems. I'm also fine with removing broken connections. It's the manly way to go about the issue. Just nuke it. I think you should go ahead and merge this if/when you're happy with it. I bumped #devtalk for a review and there's been no objections.

Member

zonkmachine commented Jun 10, 2018

I've tested this some more and the demo files from #4060 opens up with no problems. I'm also fine with removing broken connections. It's the manly way to go about the issue. Just nuke it. I think you should go ahead and merge this if/when you're happy with it. I bumped #devtalk for a review and there's been no objections.

@PhysSong PhysSong merged commit 407973a into LMMS:stable-1.2 Jun 13, 2018

2 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@PhysSong PhysSong deleted the PhysSong:ctrlcrash branch Jun 13, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment