-
Notifications
You must be signed in to change notification settings - Fork 293
Pegbar refactoring and fixes #1580
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
Conversation
Additional changes: - Fix cache issue caused by not calling frameModified. - Make PegbarResult -> PegStatus, inherits from Status now. - Implement ability to update status while keeping data.
- "Select" tool will be set upon opening the dialog - A selection will be made unless one already exists - First layer will be pre-selected Other: - Make sure dialogs are being closed when starting a new project context. Cleanup code...
647fa34 to
8263989
Compare
J5lx
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good so far, I like that the aligner has its own class now. However, a few issues are still left. First of all, here’s a PR with minor changes, as usual: CandyFace#13. As for the other issues:
- When the current position on the timeline is before the first frame, opening the peg bar dialog leads to a segfault
- Simply closing the peg bar dialog leads to a segfault
- When a selection exists before opening the dialog, it is necessary to deselect, then reselect the layer in order for the align button to become enabled
- The selection is lost when the dialog is closed, even when the user didn’t actually confirm the dialog. Either way I’m not really sure what the intention behind that is.
Peg Bar Alignment Minor Tweaks
|
All but one change has been addressed, not what to do with the last one as we can't make use of the existing functions, the way we've hooked actions directly to the manager. |
…lification Simplify toolbox slots
J5lx
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Merging.
The PR is focused on moving pegbar related code into its own core class but it contains a bunch of improvements and fixes too.
Among the list of changes:
Quality of life improvements:
Other:
Cleanup code...