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

Pattern Editor - Copy and paste of patterns is broken #6756

Closed
zonkmachine opened this issue Jul 1, 2023 · 5 comments
Closed

Pattern Editor - Copy and paste of patterns is broken #6756

zonkmachine opened this issue Jul 1, 2023 · 5 comments

Comments

@zonkmachine
Copy link
Member

zonkmachine commented Jul 1, 2023

Bug Summary

Copy and paste of patterns in the Pattern Editor is broken.

Steps to reproduce

In the Pattern Editor. With the context menu, copy a pattern from one pattern to the other. Can't do it!

Affected LMMS versions

The issue was introduced in b64fe8e Update: On the original PR #5627, it's commit ebc808e that is responsible.
@IanCaio

b64fe8e7c0afc33ead8c88761efbd71358656890 is the first bad commit

commit b64fe8e7c0afc33ead8c88761efbd71358656890
Author: IanCaio <iancaio_dev@hotmail.com>
Date:   Sat Oct 10 03:17:25 2020 -0300

    Refactor Clipboard methods (#5627)
    
    * Moves mimeType from StringPairDrag to Clipboard
    
    * Simplifies decodeKey and decodeValue methods
    
    * Adds method to copy a string to clipboard
    
    * Adds method to copy a string pair to the Clipboard
    
    * Adds convenience methods to Clipboard.h to retrieve the QMimeData from the clipboard and checking whether a particular mime type format is present on it
    
    * Uses only the TCOV copy/paste methods on the song editor
    To keep consistency on the behavior of the TCOV copy and paste operations, we now only use the TCOV::copy() and TCOV::paste() methods for copying both individual and multiple TCOs.
    
    * Removes obsolete TrackContentObject::cut() and merge copy() and paste() methods to single function TrackContentObject::copyStateTo()
    
    * Adds Clipboard::getString method to get data for particular mime type
    
    * Makes AutomatableModelView.cpp use the Clipboard class instead of calling Qt clipboard directly


@zonkmachine zonkmachine added the bug label Jul 1, 2023
@zonkmachine zonkmachine added this to the 1.3 milestone Jul 1, 2023
@zonkmachine
Copy link
Member Author

zonkmachine commented Aug 1, 2023

I probed this further. On the original PR #5627, it's commit ebc808e that is responsible.
1 changed file with 16 additions and 43 deletions


 Uses only the TCOV copy/paste methods on the SE

	To keep consistency on the behavior of the TCOV copy and paste operations, we now only use the
        TCOV::copy() and TCOV::paste() methods for copying both individual and multiple TCOs. The TCO::copy()
        and TCO::paste() methods are still kept since they are used on the src/track/BBTrack.cpp file.

TODO:
	Since the TCO::copy() and TCO::paste() are now only used for internal operations that have
        nothing to do with Userland clipboard operations, I believe there's no need to change them to
        use the system's clipboard (if they do, cloning a BB Track would clear the clipboard which is a
        somewhat undesired behavior). However, if they don't use the clipboard they shouldn't be relying
        on the Clipboard.h class at all, so a small refactor would have to be done to move away from it.
        I believe renaming would also be interesting since now their behavior resembles more a
        copyState and pasteState operation.

@Rossmaxx
Copy link
Contributor

Rossmaxx commented Aug 9, 2023

Been looking at some suspected code for trying to track down the missing stuff but couldn't understand what was missed (mainly because how everything changed after reorg and comparing the diff of that pr and master makes little sense). If you got anything more, please post it here.

@zonkmachine
Copy link
Member Author

@Rossmaxx
Copy link
Contributor

Rossmaxx commented Aug 9, 2023

Update: i probed a bit more in the issue tracker and found #6002, apparently, this issue is a duplicate of that one. Closing or are you leaving this here.

The changes from ebc808e are now in https://github.com/LMMS/lmms/blob/master/src/gui/clips/ClipView.cpp

I looked that file and found very little related code. Might have been moved somewhere in between the refactor.
Edit : might have found the culprit but am not sure.

@zonkmachine
Copy link
Member Author

zonkmachine commented Aug 9, 2023

Update: i probed a bit more in the issue tracker and found #6002, apparently, this issue is a duplicate of that one. Closing or are you leaving this here.

Thanks, closing this one!

I looked that file and found very little related code.

Copy a section of the commit I mentioned that looks general enough. Then search on the page I linked to. The code is there.

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

No branches or pull requests

2 participants