From 92814bafb7dfb4d211581e1b4e08f1d205e40352 Mon Sep 17 00:00:00 2001
From: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com>
Date: Mon, 26 Feb 2024 10:29:37 +0100
Subject: [PATCH] [Animation] Improvements to `Tween` memory management

Using `ObjectID` instead of manually breaking reference cycles.
---
 scene/animation/tween.cpp | 29 ++++++++++++++---------------
 scene/animation/tween.h   |  7 ++++---
 2 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/scene/animation/tween.cpp b/scene/animation/tween.cpp
index 2e5fd6180c8..56c582e2d77 100644
--- a/scene/animation/tween.cpp
+++ b/scene/animation/tween.cpp
@@ -54,11 +54,11 @@ Tween::interpolater Tween::interpolaters[Tween::TRANS_MAX][Tween::EASE_MAX] = {
 };
 
 void Tweener::set_tween(const Ref<Tween> &p_tween) {
-	tween = p_tween;
+	tween_id = p_tween->get_instance_id();
 }
 
-void Tweener::clear_tween() {
-	tween.unref();
+Ref<Tween> Tweener::_get_tween() {
+	return Ref<Tween>(ObjectDB::get_instance(tween_id));
 }
 
 void Tweener::_bind_methods() {
@@ -192,12 +192,6 @@ bool Tween::is_valid() {
 
 void Tween::clear() {
 	valid = false;
-
-	for (List<Ref<Tweener>> &step : tweeners) {
-		for (Ref<Tweener> &tweener : step) {
-			tweener->clear_tween();
-		}
-	}
 	tweeners.clear();
 }
 
@@ -504,6 +498,7 @@ Tween::Tween(bool p_valid) {
 }
 
 Ref<PropertyTweener> PropertyTweener::from(const Variant &p_value) {
+	Ref<Tween> tween = _get_tween();
 	ERR_FAIL_COND_V(tween.is_null(), nullptr);
 
 	Variant from_value = p_value;
@@ -592,6 +587,8 @@ bool PropertyTweener::step(double &r_delta) {
 		do_continue_delayed = false;
 	}
 
+	Ref<Tween> tween = _get_tween();
+
 	double time = MIN(elapsed_time - delay, duration);
 	if (time < duration) {
 		if (custom_method.is_valid()) {
@@ -623,12 +620,12 @@ bool PropertyTweener::step(double &r_delta) {
 }
 
 void PropertyTweener::set_tween(const Ref<Tween> &p_tween) {
-	tween = p_tween;
+	Tweener::set_tween(p_tween);
 	if (trans_type == Tween::TRANS_MAX) {
-		trans_type = tween->get_trans();
+		trans_type = p_tween->get_trans();
 	}
 	if (ease_type == Tween::EASE_MAX) {
-		ease_type = tween->get_ease();
+		ease_type = p_tween->get_ease();
 	}
 }
 
@@ -781,6 +778,8 @@ bool MethodTweener::step(double &r_delta) {
 		return true;
 	}
 
+	Ref<Tween> tween = _get_tween();
+
 	Variant current_val;
 	double time = MIN(elapsed_time - delay, duration);
 	if (time < duration) {
@@ -810,12 +809,12 @@ bool MethodTweener::step(double &r_delta) {
 }
 
 void MethodTweener::set_tween(const Ref<Tween> &p_tween) {
-	tween = p_tween;
+	Tweener::set_tween(p_tween);
 	if (trans_type == Tween::TRANS_MAX) {
-		trans_type = tween->get_trans();
+		trans_type = p_tween->get_trans();
 	}
 	if (ease_type == Tween::EASE_MAX) {
-		ease_type = tween->get_ease();
+		ease_type = p_tween->get_ease();
 	}
 }
 
diff --git a/scene/animation/tween.h b/scene/animation/tween.h
index 8dcc3ad7b64..f5ae5e97764 100644
--- a/scene/animation/tween.h
+++ b/scene/animation/tween.h
@@ -39,16 +39,18 @@ class Node;
 class Tweener : public RefCounted {
 	GDCLASS(Tweener, RefCounted);
 
+	ObjectID tween_id;
+
 public:
 	virtual void set_tween(const Ref<Tween> &p_tween);
 	virtual void start() = 0;
 	virtual bool step(double &r_delta) = 0;
-	void clear_tween();
 
 protected:
 	static void _bind_methods();
 
-	Ref<Tween> tween;
+	Ref<Tween> _get_tween();
+
 	double elapsed_time = 0;
 	bool finished = false;
 };
@@ -291,7 +293,6 @@ private:
 	Tween::TransitionType trans_type = Tween::TRANS_MAX;
 	Tween::EaseType ease_type = Tween::EASE_MAX;
 
-	Ref<Tween> tween;
 	Variant initial_val;
 	Variant delta_val;
 	Variant final_val;