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

Non linear undo redo #11

Closed
wants to merge 15 commits into from

Conversation

JordanMartinez
Copy link
Contributor

Rather than trying to store all changes in the graph and using the queues as proxy (my approach in #10), I decided to store the changes in the queues and use the graph to store the edges and the queues themselves. This way, the queues feel more queue-like and all three types of queues can be used in the same non-linear undo graph: an unlimited, a fixed-size, and a zero-size queue.

@JordanMartinez
Copy link
Contributor Author

JordanMartinez commented Jun 2, 2016

@TomasMikula I've updated the PR to be much simpler and to read more like a story. I'm not sure if you'd support my decision, but I decided to relocate the UndoManager's boolean bindings inside of the change queue. This leads to more consistency across both implementations of UndoManager. Moreover, a non-linear change queue requires those bindings to be encapsulated inside of it rather than the manager. So, for consistency's sake (and since it doesn't affect much of the other queues), I decided to take that path.

Things still to do:

  • There are the tests that still need to be written to insure this code works properly. Due to the large dependency on functional programming, what would be the best way to do this?

Since this has already taken up so much time (too much to be honest...), I'm limiting the scope of this PR to implement a non-linear system that only uses unlimited non-linear change queues, not the two other variants (fixed-size and zero-size).
The biggest issue here is determining how to do the fixed-size change queue. Really, the question comes down to determining how a bubbled change should be inserted into a full array: if full and inserting the bubbled portion of an undo/redo, does one push out the first undo ever done or the last redo?
Since this deals with an actual array and not some list-version of it (e.g. ArrayList), I didn't want to deal with all the potential problems that might arise if I created a wrapper around the array for easier, list-like management (e.g. FixedSizeArrayList), especially since this code doesn't directly help me in any way yet.

@JordanMartinez
Copy link
Contributor Author

Ok. I'm pretty sure this will actually work correctly now.

…als()' in case 2+ queues have added a different-but-equivalent change at some point in their history (whether an undo/redo)
…oes a change

- no need to call `graph.updateChangesWithRedo` AFTER change has been applied since changes only affect which undo/redo is the next valid one. So, call is made within `next/prev` methods
@JordanMartinez
Copy link
Contributor Author

See #12 since commits were added to the master branch since this PR

@JordanMartinez JordanMartinez deleted the NonLinearUndoRedo branch January 21, 2017 04:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant