Skip to content

Commit

Permalink
Persist undo/redo history correctly across OpenShot sessions (OpenSho…
Browse files Browse the repository at this point in the history
…t#2474)

* Updating default history limit to 15 items (for new users)
* Remove "discard history" logic, since we now want to persist it between sessions
* Correctly remove "history" attribute from undo/redo data (since history is a huge data set of undo/redo actions). Also, do not save or load update actions of type "load". Also clear actionHistory when loading a new project. These changes should prevent the OSP project file from becoming HUGE. At most, 15 changes in the undo and redo history should only increase file size by 100k or less (which I think is a fair trade off).
* Do not clear redo history if "ignore_history" is true
* Only clear redo history when a non-history update is performed
  • Loading branch information
jonoomph authored Dec 23, 2018
1 parent beec2f8 commit 33c0c21
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 21 deletions.
8 changes: 0 additions & 8 deletions src/classes/project_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -339,10 +339,6 @@ def load(self, file_path):
# Check if paths are all valid
self.check_if_paths_are_valid()

# Discard history
log.info("Discarding history")
self._data["history"] = { "undo": [], "redo": [] }

# Copy any project thumbnails to main THUMBNAILS folder
loaded_project_folder = os.path.dirname(self.current_filepath)
project_thumbnails_folder = os.path.join(loaded_project_folder, "thumbnail")
Expand Down Expand Up @@ -703,10 +699,6 @@ def save(self, file_path, move_temp_files=True, make_paths_relative=True):
if make_paths_relative:
self.convert_paths_to_relative(file_path)

# Discard history
log.info("Discarding history")
self._data["history"] = { "undo": [], "redo": [] }

# Append version info
v = openshot.GetVersion()
self._data["version"] = { "openshot-qt" : info.VERSION,
Expand Down
45 changes: 33 additions & 12 deletions src/classes/updates.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,15 @@ def json(self, is_array=False, only_value=False):
"partial": self.partial_update,
"old_values": copy.deepcopy(self.old_values)}

# Always remove 'history' key (if found)
if "history" in data_dict:
data_dict.pop("history")
# Always remove 'history' key (if found). This prevents nested "history"
# attributes when a project dict is loaded.
try:
if data_dict.get("value") and "history" in data_dict.get("value"):
data_dict.get("value").pop("history", None)
if data_dict.get("old_values") and "history" in data_dict.get("old_values"):
data_dict.get("old_values").pop("history", None)
except Exception:
log.info('Warning: failed to clear history attribute from undo/redo data.')

if not is_array:
# Use a JSON Object as the root object
Expand All @@ -106,6 +112,16 @@ def load_json(self, value):
self.old_values = update_action_dict.get("old_values")
self.partial_update = update_action_dict.get("partial")

# Always remove 'history' key (if found). This prevents nested "history"
# attributes when a project dict is loaded.
try:
if self.values and "history" in self.values:
self.values.pop("history", None)
if self.old_values and "history" in self.old_values:
self.old_values.pop("history", None)
except Exception:
log.info('Warning: failed to clear history attribute from undo/redo data.')


class UpdateManager:
""" This class is used to track and distribute changes to listeners. Typically, only 1 instance of this class is needed,
Expand All @@ -130,14 +146,14 @@ def load_history(self, project):

# Loop through each, and load serialized data into updateAction objects
for actionDict in history.get("redo", []):
if "history" not in actionDict.keys():
action = UpdateAction()
action.load_json(json.dumps(actionDict))
action = UpdateAction()
action.load_json(json.dumps(actionDict))
if action.type != "load":
self.redoHistory.append(action)
for actionDict in history.get("undo", []):
if "history" not in actionDict.keys():
action = UpdateAction()
action.load_json(json.dumps(actionDict))
action = UpdateAction()
action.load_json(json.dumps(actionDict))
if action.type != "load":
self.actionHistory.append(action)

# Notify watchers of new status
Expand All @@ -151,9 +167,11 @@ def save_history(self, project, history_length):
# Loop through each, and serialize
history_length_int = int(history_length)
for action in self.redoHistory[-history_length_int:]:
redo_list.append(json.loads(action.json()))
if action.type != "load":
redo_list.append(json.loads(action.json()))
for action in self.actionHistory[-history_length_int:]:
undo_list.append(json.loads(action.json()))
if action.type != "load":
undo_list.append(json.loads(action.json()))

# Set history data in project
self.ignore_history = True
Expand Down Expand Up @@ -274,6 +292,7 @@ def load(self, values):

self.last_action = UpdateAction('load', '', values)
self.redoHistory.clear()
self.actionHistory.clear()
self.dispatch_action(self.last_action)

# Perform new actions, clearing redo history for taking a new path
Expand All @@ -290,7 +309,9 @@ def update(self, key, values, partial_update=False):
""" Update the UpdateManager with an UpdateAction (this action will then be distributed to all listeners) """

self.last_action = UpdateAction('update', key, values, partial_update)
self.redoHistory.clear()
if self.last_action.key and self.last_action.key[0] != "history":
# Clear redo history for any update except a "history" update
self.redoHistory.clear()
if not self.ignore_history:
self.actionHistory.append(self.last_action)
self.dispatch_action(self.last_action)
Expand Down
2 changes: 1 addition & 1 deletion src/settings/_default.settings
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@
"category": "Autosave",
"min": 0,
"setting": "history-limit",
"value": 20,
"value": 15,
"type": "spinner-int"
},
{
Expand Down

0 comments on commit 33c0c21

Please sign in to comment.