search

LMMS / lmms

Cross-platform music production software
https://lmms.io
GNU General Public License v2.0
8.11k stars 1.01k forks source link

Undo doesn't work for deleting and adding tracks #2235

Open ElectricVersion opened 9 years ago

ElectricVersion commented 9 years ago

Pretty self explanatory, the undo and redo buttons don't work for deleting or adding tracks.

tresf commented 9 years ago

There are a few areas that aren't yet handled by undo. I'm leaving this one open and closing the non-descriptive bug, #1463.

1699 describes the details a little better so that's some nice reading material for those interested in tackling this. Thanks for the bug report.

tresf commented 9 years ago

A copy of the relevant text from #1699

@curlymorphic commented on Jan 27

I have done some reading on undo systems, as i have seen the idea mentioned :)

In an ideal world using journals is not the best way forward. A Command pattern is normally used, and Qt does have a framework for this.

from the Qt manual

The Command pattern is based on the idea that all editing in an application is done by creating instances of command objects. Command objects apply changes to the document and are stored on a command stack. Furthermore, each command knows how to undo its changes to bring the document back to its previous state. As long as the application only uses command objects to change the state of the document, it is possible to undo a sequence of commands by traversing the stack downwards and calling undo on each command in turn. It is also possible to redo a sequence of commands by traversing the stack upwards and calling redo on each command.

http://qt-project.org/doc/qt-4.8/qundo.html

Lmms is currently not written using commands that know how to undo themselves. This would involve a lot of work and alot of testing, but would probably result in a better system. If we had the resources I would recommend this.

The Journal method while still needs testing, does not change the actual code base. appart for 1 liners to take a 'snapshot', so the amount of code that gets broken and needs fixing should be far less. There is also no need to code,test,debug an undo function for every command. So the dev time should be shorter. The downside , im sure we are going to hit limitations.

So i guess it's a decision as to what route to take?

jackokring commented 8 years ago

The undo of track deletes does not work in the 1.1.3-g3e03e71 build I made last night.

Is this the right thread for discussion of undo improvement too?

I agree that out of order undoing would not be possible for all undo 'anti commands', but for some it would be and would therefore be useful.

I think it is worth considering a sub classing override on some undo class methods so to allow an out of order undo for some done things, and (possibly?) an extra saved file in a global context as a multi clip from various undo buffers.

Lyrcaxis commented 5 years ago

bump....

tresf commented 5 years ago

Please do not bump bug reports. You've been warned on Discord and now you've been warned on GitHub. Let's not escalate this.

tresf commented 5 years ago

Quoting discord:

@Lyrcaxis said: [...] it caused me enough frustration to wanna take a look at it lol [...] i'm trying to come up with a more general solution, like having each void that would trigger undo registered as an UndoAction somehow

@Lyrcaxis we welcome contributions, but we have far too many bugs to allow "bumping", so please make sure your messages have context.