Issue #447: apply review comments

src/js/service/HistoryService.js

@@ -7,7 +7,7 @@
     this.deserializer = deserializer || pskl.utils.serialization.Deserializer;
 
     this.stateQueue = [];
-    this.currentUUID = false;
+    this.currentIndex = -1;
     this.lastLoadState = -1;
   };
 
@@ -40,89 +40,52 @@
   };
 
   ns.HistoryService.prototype.saveState = function (action) {
+    this.stateQueue = this.stateQueue.slice(0, this.currentIndex + 1);
+    this.currentIndex = this.currentIndex + 1;
+
     var state = {
       action : action,
       frameIndex : action.state ? action.state.frameIndex : this.piskelController.currentFrameIndex,
       layerIndex : action.state ? action.state.layerIndex : this.piskelController.currentLayerIndex,
-      previousIndex: false,
+      uuid: pskl.utils.Uuid.generate()
-      nextIndex: false
     };
 
     var isSnapshot = action.type === ns.HistoryService.SNAPSHOT;
-    var isAtAutoSnapshotInterval = (this.stateQueue.length - 1) % ns.HistoryService.SNAPSHOT_PERIOD === 0;
+    var isAtAutoSnapshotInterval = this.currentIndex % ns.HistoryService.SNAPSHOT_PERIOD === 0;
     if (isSnapshot || isAtAutoSnapshotInterval) {
       state.piskel = this.piskelController.serialize(true);
     }
 
-    this.pushNewState_(state);
+    this.stateQueue.push(state);
     $.publish(Events.HISTORY_STATE_SAVED);
   };
 
-  ns.HistoryService.prototype.getCurrentStateIndex = function() {
+  ns.HistoryService.prototype.getCurrentStateId = function () {
-    return this.currentUUID;
+    var state = this.stateQueue[this.currentIndex];
-  };
+    if (!state) {
-
-  ns.HistoryService.prototype.getCurrentState = function() {
-    if (this.currentUUID) {
-      return this.stateQueue[this.currentUUID];
-    } else {
       return false;
     }
-  };
 
-  ns.HistoryService.prototype.pushNewState_ = function(state) {
+    return state.uuid;
-    // Generate a random UUID (~1e28 combinations)
-    var uuid = 'xxxxxx'.replace(/x/g, function() {
-      return (Math.random() * 36 << 0).toString(36);
-    });
-
-    var currentState = this.getCurrentState();
-    if (currentState) {
-      // Clear unlinked states
-      if (currentState.nextIndex) {
-        this.clearBranchingQueue_(currentState.nextIndex);
-      }
-
-      currentState.nextIndex = uuid;
-    }
-
-    state.previousIndex = this.currentUUID;
-    this.stateQueue[uuid] = state;
-
-    this.currentUUID = uuid;
-  };
-
-  ns.HistoryService.prototype.clearBranchingQueue_ = function(index) {
-    while (this.stateQueue[index]) {
-      var next = this.stateQueue[index].nextIndex;
-      delete(this.stateQueue[index]);
-      index = next;
-    }
   };
 
   ns.HistoryService.prototype.undo = function () {
-    var currentState = this.getCurrentState();
+    this.loadState(this.currentIndex - 1);
-    if (currentState.previousIndex) {
-      this.loadState(currentState.previousIndex);
-    }
   };
 
   ns.HistoryService.prototype.redo = function () {
-    var currentState = this.getCurrentState();
+    this.loadState(this.currentIndex + 1);
-    if (currentState.nextIndex) {
-      this.loadState(currentState.nextIndex);
-    }
   };
 
   ns.HistoryService.prototype.isLoadStateAllowed_ = function (index) {
     var timeOk = (Date.now() - this.lastLoadState) > ns.HistoryService.LOAD_STATE_INTERVAL;
-    var indexInRange = index && this.stateQueue[index];
+    var indexInRange = index >= 0 && index < this.stateQueue.length;
     return timeOk && indexInRange;
   };
 
   ns.HistoryService.prototype.getPreviousSnapshotIndex_ = function (index) {
     while (this.stateQueue[index] && !this.stateQueue[index].piskel) {
-      index = this.stateQueue[index].previousIndex;
+      index = index - 1;
     }
     return index;
   };
@@ -133,7 +96,7 @@
         this.lastLoadState = Date.now();
 
         var snapshotIndex = this.getPreviousSnapshotIndex_(index);
-        if (!snapshotIndex) {
+        if (snapshotIndex < 0) {
           throw 'Could not find previous SNAPSHOT saved in history stateQueue';
         }
         var serializedPiskel = this.getSnapshotFromState_(snapshotIndex);
@@ -144,7 +107,7 @@
       console.error('[CRITICAL ERROR] : Unable to load a history state.');
       this.logError_(error);
       this.stateQueue = [];
-      this.currentUUID = false;
+      this.currentIndex = -1;
     }
   };
 
@@ -179,23 +142,19 @@
     piskel.savePath = this.piskelController.piskel.savePath;
     this.piskelController.setPiskel(piskel);
 
-    var walkingIndex = snapshotIndex;
+    for (var i = snapshotIndex + 1 ; i <= index ; i++) {
-    while (walkingIndex && walkingIndex != index) {
+      var state = this.stateQueue[i];
-      walkingIndex = this.stateQueue[walkingIndex].nextIndex;
-      if (walkingIndex) {
-        var state = this.stateQueue[walkingIndex];
       this.setupState(state);
       this.replayState(state);
     }
-    }
 
     // Should only do this when going backwards
-    var next = this.stateQueue[index].nextIndex;
+    var lastState = this.stateQueue[index + 1];
-    if (next) {
+    if (lastState) {
-      this.setupState(this.stateQueue[next]);
+      this.setupState(lastState);
     }
 
-    this.currentUUID = index;
+    this.currentIndex = index;
     $.publish(Events.PISKEL_RESET);
     $.publish(Events.HISTORY_STATE_LOADED);
     if (originalSize !== this.getPiskelSize_()) {

src/js/service/SavedStatusService.js

@@ -5,33 +5,27 @@
     this.piskelController = piskelController;
     this.historyService = historyService;
     this.lastSavedStateIndex = '';
+
+    this.publishStatusUpdateEvent_ = this.publishStatusUpdateEvent_.bind(this);
   };
 
   ns.SavedStatusService.prototype.init = function () {
-    $.subscribe(Events.TOOL_RELEASED, this.onToolReleased.bind(this));
+    $.subscribe(Events.TOOL_RELEASED, this.publishStatusUpdateEvent_);
-    $.subscribe(Events.PISKEL_RESET, this.onPiskelReset.bind(this));
+    $.subscribe(Events.PISKEL_RESET, this.publishStatusUpdateEvent_);
     $.subscribe(Events.PISKEL_SAVED, this.onPiskelSaved.bind(this));
-    this.lastSavedStateIndex = this.historyService.getCurrentStateIndex();
+    this.lastSavedStateIndex = this.historyService.getCurrentStateId();
-  };
-
-  ns.SavedStatusService.prototype.onToolReleased = function () {
-    this.updateDirtyStatus();
-  };
-
-  ns.SavedStatusService.prototype.onPiskelReset = function () {
-    this.updateDirtyStatus();
   };
 
   ns.SavedStatusService.prototype.onPiskelSaved = function () {
-    this.lastSavedStateIndex = this.historyService.getCurrentStateIndex();
+    this.lastSavedStateIndex = this.historyService.getCurrentStateId();
-    $.publish(Events.PISKEL_SAVED_STATUS_UPDATE);
+    this.publishStatusUpdateEvent_();
   };
 
-  ns.SavedStatusService.prototype.updateDirtyStatus = function () {
+  ns.SavedStatusService.prototype.publishStatusUpdateEvent_ = function () {
     $.publish(Events.PISKEL_SAVED_STATUS_UPDATE);
   };
 
   ns.SavedStatusService.prototype.isDirty = function () {
-    return (this.lastSavedStateIndex != this.historyService.getCurrentStateIndex());
+    return (this.lastSavedStateIndex != this.historyService.getCurrentStateId());
   };
 })();

test/js/service/HistoryServiceTest.js

@@ -18,7 +18,7 @@ describe("History Service suite", function() {
   var historyService = null;
 
   var getLastState = function () {
-    return historyService.getCurrentState();
+    return historyService.stateQueue[historyService.currentIndex];
   };
 
   var createMockHistoryService = function () {
@@ -34,15 +34,15 @@ describe("History Service suite", function() {
     return new pskl.service.HistoryService(mockPiskelController, mockShortcutService);
   };
 
-  it("starts empty", function() {
+  it("starts at -1", function() {
     historyService = createMockHistoryService();
-    expect(historyService.stateQueue.length).toBe(0);
+    expect(historyService.currentIndex).toBe(-1);
   });
 
-  it("has 1 item after init", function() {
+  it("is at 0 after init", function() {
     historyService = createMockHistoryService();
     historyService.init();
-    expect(historyService.stateQueue.length).toBe(1);
+    expect(historyService.currentIndex).toBe(0);
   });
 
   var sendSaveEvents = function (type) {
@@ -65,7 +65,7 @@ describe("History Service suite", function() {
 
     sendSaveEvents(pskl.service.HistoryService.REPLAY).times(5);
 
-    expect(historyService.stateQueue.length).toBe(5);
+    expect(historyService.currentIndex).toBe(5);
 
     expect(getLastState().piskel).toBe(SERIALIZED_PISKEL);