godotengine / godot-cpp

C++ bindings for the Godot script API
MIT License
1.69k stars 509 forks source link

Crash when setting script on the wrong node #323

Open BastiaanOlij opened 5 years ago

BastiaanOlij commented 5 years ago

This off course is stupid but it is something that requires better protection. GDScript has this when you assign a script that extends a node that is not related to the node you apply the script on.

I had this with Procmesh before and just spend hours looking for the issue in a new module only to realize it was my age old problem. I'm extending an array mesh but seeing this is used as a property of a mesh instance the new embedded interface in Godot makes it very easy to assign the script onto the mesh instance instead of the array mesh. The result is that as soon as you call a method, Godot will crash.

I think we need to change two things: 1) find a way to prevent the script from loading if its assigned to the wrong node type (not sure if this is even possible) 2) maybe check in our _icall functions whether the method pointer isn't NULL

Zylann commented 4 years ago

The fact Godot even allows this in the first place is a big flaw IMO. It should not let the assignment succeed. We should not be littering every single function in our bindings with null checks just because Godot doesn't do the job properly, allowing an invalid state to persist :/