Fix crashes/bugs related to adding components with undo/redo

src/core/changelog.c

@@ -18,7 +18,7 @@
 
 #include <stdalign.h>
 
-#define LOG_LEVEL LL_DEBUG
+#define LOG_LEVEL LL_INFO
 #include "log.h"
 
 #define cl_entry_at(log, offset) ((LogEntry *)((uint8_t *)log->log + (offset)))
@@ -270,4 +270,25 @@ void cl_redo(ChangeLog *log) {
 
   log->nextEntry = poppedCommitStart;
   log->lastEntry = log->nextEntry - cl_entry_at(log, log->nextEntry)->psize;
+}
+
+void cl_discard(ChangeLog *log) {
+  LogIndex prevCommitStart = log->commits[arrlen(log->commits) - 1];
+  if (log->nextEntry == prevCommitStart) {
+    return;
+  }
+
+  // restore the snapshot
+  log->cl_revert_snapshot(log->user);
+
+  // replay all actions up to the previous commit point
+  LogEntry *entry = (LogEntry *)log->log;
+  LogEntry *last = cl_entry_at(log, prevCommitStart);
+  while (entry < last) {
+    cl_replay_entry(log, entry);
+    entry = (LogEntry *)((uint8_t *)entry + entry->size);
+  }
+
+  log->nextEntry = prevCommitStart;
+  log->lastEntry = log->nextEntry - cl_entry_at(log, log->nextEntry)->psize;
 }

src/core/circuit.c

@@ -411,6 +411,7 @@ void circ_snapshot(Circuit *circ) {
 
 void circ_undo(Circuit *circ) { cl_undo(&circ->log); }
 void circ_redo(Circuit *circ) { cl_redo(&circ->log); }
+void circ_discard_since_last_commit(Circuit *circ) { cl_discard(&circ->log); }
 
 static void circ_add_impl(Circuit *circ, EntityType type, ID id) {
   Table *header = circ->table[type];

src/core/core.h

@@ -261,6 +261,7 @@ void cl_untag(ChangeLog *log, ID id, uint8_t table, uint16_t tag);
 
 void cl_undo(ChangeLog *log);
 void cl_redo(ChangeLog *log);
+void cl_discard(ChangeLog *log);
 
 ////////////////////////////////////////////////////////////////////////////////
 // Circuit
@@ -898,6 +899,7 @@ static inline ID circ_lliter_get(LinkedListIter *iter) { return iter->current; }
 void circ_snapshot(Circuit *circ);
 void circ_undo(Circuit *circ);
 void circ_redo(Circuit *circ);
+void circ_discard_since_last_commit(Circuit *circ);
 
 ////////////////////////////////////////////////////////////////////////////////
 // Higher level functions

src/core/core_test.c

@@ -61,61 +61,6 @@ UTEST(bv, clear_bit) {
 // ChangeLog tests
 ////////////////////////
 
-// UTEST(ChangeLog, new) {
-//   ChangeLog log;
-//   cl_init(&log);
-//   ASSERT_NE(log.log, NULL);
-//   ASSERT_EQ(log.nextEntry, (LogEntry *)log.log);
-//   ASSERT_EQ(log.lastEntry, NULL);
-//   ASSERT_EQ(log.lastCommitStart, log.nextEntry);
-//   cl_free(&log);
-// }
-
-// UTEST(ChangeLog, create) {
-//   ChangeLog log;
-//   cl_init(&log);
-//   cl_create(&log, id_make(0, 1, 1), 2);
-//   ASSERT_EQ(log.lastEntry->verb, LOG_CREATE);
-//   ASSERT_EQ(log.lastEntry->id, id_make(0, 1, 1));
-//   ASSERT_EQ(log.lastEntry->table, 2);
-//   cl_free(&log);
-// }
-
-// UTEST(ChangeLog, delete) {
-//   ChangeLog log;
-//   cl_init(&log);
-//   cl_delete(&log, id_make(0, 1, 1), 2);
-//   ASSERT_EQ(log.lastEntry->verb, LOG_DELETE);
-//   ASSERT_EQ(log.lastEntry->id, id_make(0, 1, 1));
-//   ASSERT_EQ(log.lastEntry->table, 2);
-//   cl_free(&log);
-// }
-
-// UTEST(ChangeLog, commit) {
-//   ChangeLog log;
-//   cl_init(&log);
-//   cl_create(&log, id_make(0, 1, 1), 2);
-//   cl_commit(&log);
-//   ASSERT_EQ(log.lastEntry->verb, LOG_CREATE | LOG_COMMIT);
-//   ASSERT_EQ(log.lastCommitStart, log.nextEntry);
-//   cl_free(&log);
-// }
-
-// UTEST(ChangeLog, update) {
-//   ChangeLog log;
-//   cl_init(&log);
-//   cl_update(&log, id_make(0, 1, 1), 1, 2, &(uint8_t){4}, sizeof(uint8_t));
-//   ASSERT_EQ(log.lastEntry->verb, LOG_UPDATE);
-//   ASSERT_EQ(log.lastEntry->id, id_make(0, 1, 1));
-//   ASSERT_EQ(log.lastEntry->table, 1);
-//   // +3 is to correct for alignment
-//   ASSERT_EQ(log.lastEntry->size, sizeof(LogUpdate) + sizeof(uint8_t) + 3);
-//   LogUpdate *update = (LogUpdate *)log.lastEntry;
-//   ASSERT_EQ(update->column, 2);
-//   ASSERT_EQ(update->newValue[0], 4);
-//   cl_free(&log);
-// }
-
 UTEST(ChangeLog, undo_3_redo_3) {
   Circuit circuit;
   circ_init(&circuit);
@@ -147,14 +92,17 @@ UTEST(ChangeLog, undo_3_redo_3) {
   circ_undo(&circuit);
 
   ASSERT_EQ(circ_len(&circuit, Symbol), 2);
+  ASSERT_FALSE(circ_has(&circuit, symID3));
 
   circ_undo(&circuit);
 
   ASSERT_EQ(circ_len(&circuit, Symbol), 1);
+  ASSERT_FALSE(circ_has(&circuit, symID2));
 
   circ_undo(&circuit);
 
   ASSERT_EQ(circ_len(&circuit, Symbol), 0);
+  ASSERT_FALSE(circ_has(&circuit, symID1));
 
   // should do nothing
   circ_undo(&circuit);
@@ -176,6 +124,8 @@ UTEST(ChangeLog, undo_3_redo_3) {
 
   // should do nothing
   circ_redo(&circuit);
+
+  circ_free(&circuit);
 }
 
 UTEST(ChangeLog, undo_redo_undo_redo) {
@@ -225,6 +175,42 @@ UTEST(ChangeLog, undo_redo_undo_redo) {
   ASSERT_TRUE(circ_has(&circuit, symID1));
   ASSERT_TRUE(circ_has(&circuit, symID2));
   ASSERT_TRUE(circ_has(&circuit, symID3));
+
+  circ_free(&circuit);
+}
+
+UTEST(ChangeLog, discard) {
+  Circuit circuit;
+  circ_init(&circuit);
+  SymbolLayout layout = (SymbolLayout){
+    .portSpacing = 20.0f,
+    .symbolWidth = 55.0f,
+    .borderWidth = 1.0f,
+    .labelPadding = 2.0f,
+    .user = NULL,
+    .textSize = testTextSize,
+  };
+  circ_load_symbol_descs(&circuit, &layout, circuit_symbol_descs(), COMP_COUNT);
+
+  circuit.top = circ_add_module(&circuit);
+  circ_snapshot(&circuit);
+
+  circ_commit(&circuit);
+
+  ID symID1 = circ_add_symbol(
+    &circuit, circuit.top, circ_get_symbol_kind_by_name(&circuit, "AND"));
+  circ_commit(&circuit);
+  ID symID2 = circ_add_symbol(
+    &circuit, circuit.top, circ_get_symbol_kind_by_name(&circuit, "OR"));
+
+  ASSERT_TRUE(circ_has(&circuit, symID1));
+  ASSERT_TRUE(circ_has(&circuit, symID2));
+
+  circ_discard_since_last_commit(&circuit);
+
+  ASSERT_FALSE(circ_has(&circuit, symID2));
+
+  circ_free(&circuit);
 }
 
 ////////////////////////

src/ux/input.c

@@ -610,8 +610,10 @@ void ux_start_adding_symbol(CircuitUX *ux, ID symbolKindID) {
 
 void ux_stop_adding_symbol(CircuitUX *ux) {
   ux->mouseDownState = STATE_UP;
-  circ_remove_symbol(&ux->view.circuit, ux->addingSymbol);
+  circ_discard_since_last_commit(&ux->view.circuit);
   ux->addingSymbol = NO_ID;
+  ux_route(ux);
+  ux_build_bvh(ux);
 }
 
 void ux_change_adding_symbol(CircuitUX *ux, ID symbolKindID) {

src/ux/undo.c

@@ -258,6 +258,12 @@ UndoCommand ux_undo(CircuitUX *ux) {
   // arrput(ux->redoStack, redoCmd);
 
   // ux_perform_command(ux, redoCmd);
+
+  if (ux->tool == TOOL_SYMBOL) {
+    // will crash if we allow this
+    return (UndoCommand){0};
+  }
+
   circ_undo(&ux->view.circuit);
   ux_route(ux);
   ux_build_bvh(ux);
@@ -275,6 +281,11 @@ UndoCommand ux_redo(CircuitUX *ux) {
   // UndoCommand undoCmd = ux_flip_command(cmd);
   // arrput(ux->undoStack, undoCmd);
 
+  if (ux->tool == TOOL_SYMBOL) {
+    // will crash if we allow this
+    return (UndoCommand){0};
+  }
+
   // ux_perform_command(ux, undoCmd);
   circ_redo(&ux->view.circuit);
   ux_route(ux);

src/ux/ux_test.c

@@ -27,7 +27,7 @@ UTEST(CircuitUX, adding_components) {
 
   ux_change_adding_symbol(&ux, orSymbolKindID);
 
-  ASSERT_FALSE(circ_has(&ux.view.circuit, oldID));
+  ASSERT_TRUE(!circ_has(&ux.view.circuit, oldID) || oldID == ux.addingSymbol);
   ASSERT_TRUE(circ_has(&ux.view.circuit, ux.addingSymbol));
   ASSERT_EQ(
     circ_get(&ux.view.circuit, ux.addingSymbol, SymbolKindID), orSymbolKindID);