Please add a global function that lets us check if an instance has already been freed.
I know, it's possible to avoid this situation by meticulously enforcing the order in which objects are removed from scene and object references are freed, but sometimes it would take tremendous effort to refactor the code to free the instance correctly / only once.
Currently, if an object was part of a display tree and its parent / grandparent has been freed, the object would be freed along with the parent, and a variable pointing to it would become invalid.
Attempting to call any function on this instance results in:
Attempt to call function 'FUNCTION_NAME' in base 'previously freed instance' on a null instance.
Since the Godot runtime is able to detect this and break with an error, I assume there's a way to check if an instance pointed at by a variable has been freed or not.
It would be a very very helpful addition and could save many hours of debugging. Something like:
bool is_instance_valid( variable )
Thankee sai.
Just wanna make sure you know Godot's 'weak references', because by design you should use them if object can be freed:
_While you know the object is stll alive:_
var wr = weakref(object);
_Then you can check like this:_
if (!wr.get_ref()):
#object is erased
else:
#object is fine so you can do something with it:
wr.get_ref().doSomeOperation();
While I agree it's not the most straightforward mechanism and I would like to see something like 'automatic null pointer resetting (or whatever), I'm recommending to use weakref in this case -> it's still not so much hassle, and it's always working.
It would be nice to add some information about WeakRef in the official documentation, apart from the API reference I can't find any explanations on when and how to use WeakRefs.
@akien-mga Thanks. I ended up patching my problem in a different way.
But it would still be nice to have a simpler mechanism to check if object is valid in a pinch. When there are multiple ways an object can be freed, you're bound to run into problems.
Thanks for weakref tip - I did not know what that was for.
I am currently using this.
It is probably slower than the weakref solution but more straightforward.
str(object)=="[Deleted Object]"
Can anyone see downsides?
I would really appreciate something like bool is_instance_valid( variable )
Just to make sure, is freed information available in release builds?
I am currently using this.
It is probably slower than the weakref solution but more straightforward.
str(object)=="[Deleted Object]"
This will not work in release builds, only while running in the debugger because it seriously affects performance to check if an object is valid every time you use it.
Currently there is no way to check if the instance is valid, but the best way is to use weakref. That said, it should be possible to add a function to check if the instance is valid..
weakref() isn't really a solution. You would either have to always know which instances you might want to delete (which is impossible as the project grows and changes) or code every reference to an instance with weakref() which begs the question why isn't weakref used by default by the instance() method.
We really need is_valid() method. Seems like a really basic functionality and I'm really surprised its not there.
Why don't we store the ObjectID
as well in Variant::ObjData
? This way, when comparing it to null we also check if the instance can be found in ObjectDB
.
@akien-mga Created a doc issue for this, this one can be closed.
Can we re-open as an enhancement? This is a useful feature to have, and the following code:
var wr = weakref(object);
if (!wr.get_ref()):
#object is erased
else:
#object is fine so you can do something with it:
wr.get_ref().doSomeOperation();
is much more difficult to use than something like is_freed(node)
. It's so much worse, that I would think people just avoid this situation instead of using the code. It is also not immediately clear what is going on, while is_freed
would be a self documenting API call.
For example, assume you have an array of enemies that are in an AoE attack. Each _process you want to perform an operation on those nodes.
var collided_nodes = []
func collide(body):
collided_nodes.append(body)
func _physics_process(delta):
for node in collided_nodes:
if not is_freed(node):
do_op(node)
vs
var collided_nodes = []
func collide(body):
collided_nodes.append(weakref(body))
func _physics_process(delta):
for node_ref in collided_nodes:
var node= node_ref.get_ref()
if (node != null)
do_op(node)
The current method will also break erase
. If you want to erase a specific body you need to loop thru and compare body == arr[i].get_ref()
, as the references will be different.
Even better and easier, add is_freed
var to Object, set to false in free()
.
How are you supposed to check a variable in an object that doesn't exists?
This will need some changes, as the instance -> id reverse map is currently
only availabe on debug builds.. will probably not hurt to make it available
in release I guess..
On Thu, Mar 29, 2018, 10:28 teedoubleuu notifications@github.com wrote:
How are you supposed to check a variable in an object that doesn't exists?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/godotengine/godot/issues/4306#issuecomment-377311153,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AF-Z20eYNLE1SDwt6S3Hn-tNjYA6n6l7ks5tjRm6gaJpZM4IF-io
.
@teedoubleuu, good point
The is_instance_valid()
method was just added now by reduz! ff1e7cf
Closing since this is now implemented.
Most helpful comment
Can we re-open as an enhancement? This is a useful feature to have, and the following code:
is much more difficult to use than something like
is_freed(node)
. It's so much worse, that I would think people just avoid this situation instead of using the code. It is also not immediately clear what is going on, whileis_freed
would be a self documenting API call.For example, assume you have an array of enemies that are in an AoE attack. Each _process you want to perform an operation on those nodes.
vs
The current method will also break
erase
. If you want to erase a specific body you need to loop thru and comparebody == arr[i].get_ref()
, as the references will be different.