Fix curve saving, refined undo/redo

editor/plugins/curve_editor_plugin.cpp

@@ -133,7 +133,16 @@ void CurveEditor::on_gui_input(const Ref<InputEvent> &p_event) {
 		if (!mb.is_pressed() && _dragging && mb.get_button_index() == BUTTON_LEFT) {
 			_dragging = false;
 			if (_has_undo_data) {
-				push_undo(_undo_data);
+
+				UndoRedo &ur = *EditorNode::get_singleton()->get_undo_redo();
+
+				ur.create_action(_selected_tangent == TANGENT_NONE ? TTR("Modify Curve Point") : TTR("Modify Curve Tangent"));
+				ur.add_do_method(*_curve_ref, "_set_data", _curve_ref->get_data());
+				ur.add_undo_method(*_curve_ref, "_set_data", _undo_data);
+				// Note: this will trigger one more "changed" signal even if nothing changes,
+				// but it's ok since it would have fired every frame during the drag anyways
+				ur.commit_action();
+
 				_has_undo_data = false;
 			}
 		}
@@ -152,7 +161,8 @@ void CurveEditor::on_gui_input(const Ref<InputEvent> &p_event) {
 			if (_selected_point != -1) {
 
 				if (!_has_undo_data) {
-					// Save curve state before dragging points
+					// Save full curve state before dragging points,
+					// because this operation can modify their order
 					_undo_data = curve.get_data();
 					_has_undo_data = true;
 				}
@@ -175,8 +185,6 @@ void CurveEditor::on_gui_input(const Ref<InputEvent> &p_event) {
 
 					curve.set_point_value(_selected_point, point_pos.y);
 
-					//auto_calculate_tangents(i);
-
 				} else {
 					// Drag tangent
 
@@ -275,7 +283,13 @@ void CurveEditor::on_preset_item_selected(int preset_id) {
 			break;
 	}
 
-	push_undo(previous_data);
+	UndoRedo &ur = *EditorNode::get_singleton()->get_undo_redo();
+	ur.create_action(TTR("Load Curve Preset"));
+
+	ur.add_do_method(&curve, "_set_data", curve.get_data());
+	ur.add_undo_method(&curve, "_set_data", previous_data);
+
+	ur.commit_action();
 }
 
 void CurveEditor::_curve_changed() {
@@ -397,7 +411,8 @@ CurveEditor::TangentIndex CurveEditor::get_tangent_at(Vector2 pos) const {
 void CurveEditor::add_point(Vector2 pos) {
 	ERR_FAIL_COND(_curve_ref.is_null());
 
-	Array prev_data = _curve_ref->get_data();
+	UndoRedo &ur = *EditorNode::get_singleton()->get_undo_redo();
+	ur.create_action(TTR("Remove Curve Point"));
 
 	Vector2 point_pos = get_world_pos(pos);
 	if (point_pos.y < 0.0)
@@ -405,41 +420,64 @@ void CurveEditor::add_point(Vector2 pos) {
 	else if (point_pos.y > 1.0)
 		point_pos.y = 1.0;
 
-	_curve_ref->add_point(point_pos);
+	// Small trick to get the point index to feed the undo method
+	int i = _curve_ref->add_point(point_pos);
+	_curve_ref->remove_point(i);
+
+	ur.add_do_method(*_curve_ref, "add_point", point_pos);
+	ur.add_undo_method(*_curve_ref, "remove_point", i);
 
-	push_undo(prev_data);
+	ur.commit_action();
 }
 
 void CurveEditor::remove_point(int index) {
 	ERR_FAIL_COND(_curve_ref.is_null());
 
-	Array prev_data = _curve_ref->get_data();
+	UndoRedo &ur = *EditorNode::get_singleton()->get_undo_redo();
+	ur.create_action(TTR("Remove Curve Point"));
+
+	Curve::Point p = _curve_ref->get_point(index);
 
-	_curve_ref->remove_point(index);
+	ur.add_do_method(*_curve_ref, "remove_point", index);
+	ur.add_undo_method(*_curve_ref, "add_point", p.pos, p.left_tangent, p.right_tangent, p.left_mode, p.right_mode);
 
 	if (index == _selected_point)
 		set_selected_point(-1);
 
-	push_undo(prev_data);
+	ur.commit_action();
 }
 
 void CurveEditor::toggle_linear(TangentIndex tangent) {
 	ERR_FAIL_COND(_curve_ref.is_null());
 
-	Array prev_data = _curve_ref->get_data();
+	UndoRedo &ur = *EditorNode::get_singleton()->get_undo_redo();
+	ur.create_action(TTR("Toggle Curve Linear Tangent"));
 
 	if (tangent == TANGENT_NONE)
 		tangent = _selected_tangent;
 
 	if (tangent == TANGENT_LEFT) {
+
 		bool is_linear = _curve_ref->get_point_left_mode(_selected_point) == Curve::TANGENT_LINEAR;
-		_curve_ref->set_point_left_mode(_selected_point, is_linear ? Curve::TANGENT_FREE : Curve::TANGENT_LINEAR);
+
+		Curve::TangentMode prev_mode = _curve_ref->get_point_left_mode(_selected_point);
+		Curve::TangentMode mode = is_linear ? Curve::TANGENT_FREE : Curve::TANGENT_LINEAR;
+
+		ur.add_do_method(*_curve_ref, "set_point_left_mode", _selected_point, mode);
+		ur.add_undo_method(*_curve_ref, "set_point_left_mode", _selected_point, prev_mode);
+
 	} else {
+
 		bool is_linear = _curve_ref->get_point_right_mode(_selected_point) == Curve::TANGENT_LINEAR;
-		_curve_ref->set_point_right_mode(_selected_point, is_linear ? Curve::TANGENT_FREE : Curve::TANGENT_LINEAR);
+
+		Curve::TangentMode prev_mode = _curve_ref->get_point_right_mode(_selected_point);
+		Curve::TangentMode mode = is_linear ? Curve::TANGENT_FREE : Curve::TANGENT_LINEAR;
+
+		ur.add_do_method(*_curve_ref, "set_point_right_mode", _selected_point, mode);
+		ur.add_undo_method(*_curve_ref, "set_point_right_mode", _selected_point, prev_mode);
 	}
 
-	push_undo(prev_data);
+	ur.commit_action();
 }
 
 void CurveEditor::set_selected_point(int index) {
@@ -456,20 +494,6 @@ void CurveEditor::set_hover_point_index(int index) {
 	}
 }
 
-void CurveEditor::push_undo(Array previous_curve_data) {
-	UndoRedo *ur = EditorNode::get_singleton()->get_undo_redo();
-
-	ur->create_action(TTR("Modify Curve"));
-	ur->add_do_method(*_curve_ref, "_set_data", _curve_ref->get_data());
-	ur->add_undo_method(*_curve_ref, "_set_data", previous_curve_data);
-
-	// This boolean is to prevent commit_action from executing the do method,
-	// because at this point it's already done, there is no point in doing it twice
-	_curve_ref->_disable_set_data = true;
-	ur->commit_action();
-	_curve_ref->_disable_set_data = false;
-}
-
 void CurveEditor::update_view_transform() {
 	Vector2 control_size = get_size();
 	const real_t margin = 24;

editor/plugins/curve_editor_plugin.h

@@ -88,7 +88,6 @@ private:
 	void toggle_linear(TangentIndex tangent = TANGENT_NONE);
 	void set_selected_point(int index);
 	void set_hover_point_index(int index);
-	void push_undo(Array previous_curve_data);
 	void update_view_transform();
 
 	Vector2 get_tangent_view_pos(int i, TangentIndex tangent) const;
@@ -108,7 +107,6 @@ private:
 
 	Array _undo_data;
 	bool _has_undo_data;
-	bool _undo_no_commit;
 
 	Vector2 _context_click_pos;
 	int _selected_point;

scene/resources/curve.cpp

@@ -387,12 +387,9 @@ Curve::Curve() {
 	_baked_cache_dirty = false;
 	_min_value = 0;
 	_max_value = 1;
-#ifdef TOOLS_ENABLED
-	_disable_set_data = false;
-#endif
 }
 
-int Curve::add_point(Vector2 p_pos, real_t left_tangent, real_t right_tangent) {
+int Curve::add_point(Vector2 p_pos, real_t left_tangent, real_t right_tangent, TangentMode left_mode, TangentMode right_mode) {
 	// Add a point and preserve order
 
 	// Curve bounds is in 0..1
@@ -404,7 +401,7 @@ int Curve::add_point(Vector2 p_pos, real_t left_tangent, real_t right_tangent) {
 	int ret = -1;
 
 	if (_points.size() == 0) {
-		_points.push_back(Point(p_pos, left_tangent, right_tangent));
+		_points.push_back(Point(p_pos, left_tangent, right_tangent, left_mode, right_mode));
 		ret = 0;
 
 	} else if (_points.size() == 1) {
@@ -413,10 +410,10 @@ int Curve::add_point(Vector2 p_pos, real_t left_tangent, real_t right_tangent) {
 		real_t diff = p_pos.x - _points[0].pos.x;
 
 		if (diff > 0) {
-			_points.push_back(Point(p_pos, left_tangent, right_tangent));
+			_points.push_back(Point(p_pos, left_tangent, right_tangent, left_mode, right_mode));
 			ret = 1;
 		} else {
-			_points.insert(0, Point(p_pos, left_tangent, right_tangent));
+			_points.insert(0, Point(p_pos, left_tangent, right_tangent, left_mode, right_mode));
 			ret = 0;
 		}
 
@@ -435,16 +432,18 @@ int Curve::add_point(Vector2 p_pos, real_t left_tangent, real_t right_tangent) {
 
 		if (i == 0 && p_pos.x < _points[0].pos.x) {
 			// Insert before anything else
-			_points.insert(0, Point(p_pos, left_tangent, right_tangent));
+			_points.insert(0, Point(p_pos, left_tangent, right_tangent, left_mode, right_mode));
 			ret = 0;
 		} else {
 			// Insert between i and i+1
 			++i;
-			_points.insert(i, Point(p_pos, left_tangent, right_tangent));
+			_points.insert(i, Point(p_pos, left_tangent, right_tangent, left_mode, right_mode));
 			ret = i;
 		}
 	}
 
+	update_auto_tangents(i);
+
 	mark_dirty();
 
 	return ret;
@@ -593,6 +592,11 @@ Vector2 Curve::get_point_pos(int p_index) const {
 	return _points[p_index].pos;
 }
 
+Curve::Point Curve::get_point(int p_index) const {
+	ERR_FAIL_INDEX_V(p_index, _points.size(), Point());
+	return _points[p_index];
+}
+
 void Curve::update_auto_tangents(int i) {
 
 	Point &p = _points[i];
@@ -699,33 +703,32 @@ void Curve::mark_dirty() {
 Array Curve::get_data() const {
 
 	Array output;
-	output.resize(_points.size() * 3);
+	const unsigned int ELEMS = 5;
+	output.resize(_points.size() * ELEMS);
 
 	for (int j = 0; j < _points.size(); ++j) {
 
 		const Point p = _points[j];
-		int i = j * 3;
+		int i = j * ELEMS;
 
 		output[i] = p.pos;
 		output[i + 1] = p.left_tangent;
 		output[i + 2] = p.right_tangent;
+		output[i + 3] = p.left_mode;
+		output[i + 4] = p.right_mode;
 	}
 
 	return output;
 }
 
 void Curve::set_data(Array input) {
-	ERR_FAIL_COND(input.size() % 3 != 0);
-
-#ifdef TOOLS_ENABLED
-	if (_disable_set_data)
-		return;
-#endif
+	const unsigned int ELEMS = 5;
+	ERR_FAIL_COND(input.size() % ELEMS != 0);
 
 	_points.clear();
 
 	// Validate input
-	for (int i = 0; i < input.size(); i += 3) {
+	for (int i = 0; i < input.size(); i += ELEMS) {
 		ERR_FAIL_COND(input[i].get_type() != Variant::VECTOR2);
 		ERR_FAIL_COND(input[i + 1].get_type() != Variant::REAL);
 		ERR_FAIL_COND(input[i + 2].get_type() != Variant::REAL);
@@ -739,12 +742,12 @@ void Curve::set_data(Array input) {
 		ERR_FAIL_COND(right_mode < 0 || right_mode >= TANGENT_MODE_COUNT);
 	}
 
-	_points.resize(input.size() / 3);
+	_points.resize(input.size() / ELEMS);
 
 	for (int j = 0; j < _points.size(); ++j) {
 
 		Point &p = _points[j];
-		int i = j * 3;
+		int i = j * ELEMS;
 
 		p.pos = input[i];
 		p.left_tangent = input[i + 1];
@@ -822,7 +825,8 @@ real_t Curve::interpolate_baked(real_t offset) {
 
 void Curve::_bind_methods() {
 
-	ClassDB::bind_method(D_METHOD("add_point", "pos", "left_tangent", "right_tangent"), &Curve::add_point, DEFVAL(0), DEFVAL(0));
+	ClassDB::bind_method(D_METHOD("add_point", "pos", "left_tangent", "right_tangent", "left_mode", "right_mode"),
+			&Curve::add_point, DEFVAL(0), DEFVAL(0), DEFVAL(TANGENT_FREE), DEFVAL(TANGENT_FREE));
 	ClassDB::bind_method(D_METHOD("remove_point", "index"), &Curve::remove_point);
 	ClassDB::bind_method(D_METHOD("clear_points"), &Curve::clear_points);
 	ClassDB::bind_method(D_METHOD("get_point_pos", "index"), &Curve::get_point_pos);

scene/resources/curve.h

@@ -91,10 +91,6 @@ public:
 
 	static const char *SIGNAL_RANGE_CHANGED;
 
-#ifdef TOOLS_ENABLED
-	bool _disable_set_data;
-#endif
-
 	enum TangentMode {
 		TANGENT_FREE = 0,
 		TANGENT_LINEAR,
@@ -115,12 +111,17 @@ public:
 			right_mode = TANGENT_FREE;
 		}
 
-		Point(Vector2 p, real_t left = 0, real_t right = 0) {
-			pos = p;
-			left_tangent = left;
-			right_tangent = right;
-			left_mode = TANGENT_FREE;
-			right_mode = TANGENT_FREE;
+		Point(Vector2 p_pos,
+				real_t p_left = 0,
+				real_t p_right = 0,
+				TangentMode p_left_mode = TANGENT_FREE,
+				TangentMode p_right_mode = TANGENT_FREE) {
+
+			pos = p_pos;
+			left_tangent = p_left;
+			right_tangent = p_right;
+			left_mode = p_left_mode;
+			right_mode = p_right_mode;
 		}
 	};
 
@@ -128,7 +129,12 @@ public:
 
 	int get_point_count() const { return _points.size(); }
 
-	int add_point(Vector2 p_pos, real_t left_tangent = 0, real_t right_tangent = 0);
+	int add_point(Vector2 p_pos,
+			real_t left_tangent = 0,
+			real_t right_tangent = 0,
+			TangentMode left_mode = TANGENT_FREE,
+			TangentMode right_mode = TANGENT_FREE);
+
 	void remove_point(int p_index);
 	void clear_points();
 
@@ -138,6 +144,8 @@ public:
 	int set_point_offset(int p_index, float offset);
 	Vector2 get_point_pos(int p_index) const;
 
+	Point get_point(int p_index) const;
+
 	float get_min_value() const { return _min_value; }
 	void set_min_value(float p_min);