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

Improve error message when calling method with "previously freed instance" argument #35192

Closed
akien-mga opened this issue Jan 16, 2020 · 4 comments
Assignees
Milestone

Comments

@akien-mga
Copy link
Member

Godot version:
3.2 master (3af0400)

OS/device including version:
Any

Issue description:
When calling a GDScript method with a "previously freed instance" parameter, the error which is raised is not as clear as it could be. Maybe also for the "null instance" case, though I don't know how to reproduce it.

var node = Node.new()
node.free()
add_child(node)

raises:

Invalid type in function 'add_child' in base 'Node2D (Node2D.gd)'. The Object-derived class of argument 1 (previously freed instance) is not a subclass of the expected argument class.

Relevant code to adjust:

if (p_err.error == Variant::CallError::CALL_ERROR_INVALID_ARGUMENT) {
int errorarg = p_err.argument;
// Handle the Object to Object case separately as we don't have further class details.
#ifdef DEBUG_ENABLED
if (p_err.expected == Variant::OBJECT && argptrs[errorarg]->get_type() == p_err.expected) {
err_text = "Invalid type in " + p_where + ". The Object-derived class of argument " + itos(errorarg + 1) + " (" + _get_var_type(argptrs[errorarg]) + ") is not a subclass of the expected argument class.";
} else
#endif // DEBUG_ENABLED
{
err_text = "Invalid type in " + p_where + ". Cannot convert argument " + itos(errorarg + 1) + " from " + Variant::get_type_name(argptrs[errorarg]->get_type()) + " to " + Variant::get_type_name(p_err.expected) + ".";
}

Steps to reproduce:

  • Use above snippet on any Node-derived script.

Minimal reproduction project:
See above.

@timothyqiu
Copy link
Member

I'm a bit confused when reading the documentation for Object.free()

Deletes the object from memory. Any pre-existing reference to the freed object will now return null.

Isn't node a reference to the object? If yes, then it should become null after node.free() I think.

Or, maybe the documentation is referring to the Reference class?

@bojidar-bg
Copy link
Contributor

bojidar-bg commented Jan 16, 2020

Well, there is the thing.. pre-existing references become "previously freed instance" instead of "null".
Note that things can get much worse from there on as of now:

var node = Node.new()
node.free()
var node2 = Node.new()
add_child(node) # No error!
print(node, node2) # e.g. [Node:1370][Node:1370]
print(is_instance_valid(node)) # True ?!

But:

var node = Node.new()
node.free()
var node2 = Sprite.new()
add_child(node) # Error: Same as before (if you are lucky with malloc)

@remram44
Copy link
Contributor

@bojidar-bg I think this is what was fixed by #38119

@akien-mga
Copy link
Member Author

This is fixed indeed.

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

4 participants