Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using ResourceSaver to save Resource with cross/circle reference crashed the game #79197

Open
Tracked by #80877
peterhoglund opened this issue Jul 8, 2023 · 3 comments

Comments

@peterhoglund
Copy link

Godot version

4.1

System information

MacOS 11.7.4, v4.1.stable.official [9704596], Vulkan forward+

Issue description

I have a Resource of type Deck. Every Deck has an array of cards of type Card, also a Resource. I can create new cards in the Deck Resource from an add_card function which creates a new Card and appends it to the array. I then save the Deck with ResourceSaver. This works as you'd expect.

Now, I also want all cards to hold a reference to the Deck it's in, so in Card, I have a variable called deck, of type Deck, and in the add_card function I assign it with self, since it is in the Deck Resource. When I do this I can no longer save the deck resource with ResourceSaver, the game simply shuts down with a vague message about the Debugger not running anymore. I assume Resource cant handle this type of circle reference, so not sure if this is a bug or if this working as intended.

If this is how it is supposed to work, then I suggest expanding the error message and not simply crashing the app.

Steps to reproduce

Deck Resource

extends Resource
class_name Deck

@export var id : String
@export var cards : Array[Card]

func _init():
    id = str(Time.get_unix_time_from_system()).left(-6)

func add_card():
    var new_card : Card = Card.new()
    new_card.deck = self
    cards.append(new_card)

Card Resource

extends Resource
class_name Card

@export var deck : Deck

Main script

extends Control

const SAVE_PATH = "res://projects/"
var deck = Deck.new()

func _ready():
    for i in range(2, 6):
        deck.add_card()
    
    ResourceSaver.save(deck, SAVE_PATH + deck.id + ".tres")

Minimal reproduction project

@AThousandShips
Copy link
Member

AThousandShips commented Jul 8, 2023

While the crash is absolutely undesirable, and should be fixed, it's not correct to have circular references like this, you need to use weak references to break the cycle (Card should have a weak reference to Deck, using weakref)

My suspicion is that it's caused by infinite recursion, which I'm not sure how to combat here, in this specific case I think you'll be better off handling this by setting the parent reference on adding the card, not serializing it

Edit: Using weakref prevents crash, but I don't think you need to or should store the data in that way, instead I think you should make it so it handles that on addition

@AThousandShips
Copy link
Member

Might be solved by #68281

@jsjtxietian
Copy link
Contributor

I can comfirm that pr #68281 prevents the crash.

But the saved data seems lost the new_card's deck property.

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants