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

Undefined behavior when set_script on itself #16943

Open
ghost opened this issue Feb 23, 2018 · 5 comments
Open

Undefined behavior when set_script on itself #16943

ghost opened this issue Feb 23, 2018 · 5 comments

Comments

@ghost
Copy link

ghost commented Feb 23, 2018

Godot/OS version:

92ece2e | Antergos x86_64

Issue description:

Adding these scripts after set_script

  • print("some string")
  • set_process(true)

Expected: old script stops execution when set_script on itself / Not crash / some docs about it
Happened: print prints Null, set_process crashes the game

Steps to reproduce:

  1. Create script
    A.gd
extends Node
func _ready():
    set_script(preload("res://B.gd"))
    print(0)
    set_process(true)

B.gd

extends Node
func _process(dt):
    print(1)
  1. Create node N attach A
  2. Run

Minimal reproduction project:

SetScript.zip

@ghost ghost added this to the 3.1 milestone Feb 23, 2018
@vnen
Copy link
Member

vnen commented Jul 22, 2018

It's natural that the current function will finish execution, or at least try to. I'm not sure why the reference is lost so soon to make it crash.

@reduz
Copy link
Member

reduz commented Sep 6, 2018

I am not sure how to fix this or whether it's worth fixing, up to you @vnen

@ghost
Copy link
Author

ghost commented Sep 6, 2018

If it couldn't be fixed (expected because it's probably not intended to be used this way) then I'd appreciate some documentation/warning messages discouraging users to do so.

@vnen
Copy link
Member

vnen commented Sep 7, 2018

We can add a warning (or even an error) if you try this, similar to the error you get when you use get_node() without onready. You can probably do this properly by using call_deferred().

@ghost ghost closed this as completed Sep 15, 2018
@akien-mga akien-mga reopened this Sep 15, 2018
@akien-mga akien-mga modified the milestones: 3.1, 3.2 Jan 19, 2019
@akien-mga akien-mga modified the milestones: 3.2, 4.0 Dec 13, 2019
@tom-jk
Copy link

tom-jk commented Aug 2, 2020

Testing MRP on linux mint 18.3:

3.0: Error immediately, but no crash on nearly every run.
Invalid call. Nonexistent function '' in base 'Node (B.gd)'. error.
Output shows Null and No loader found for resource: res://.
Debugger->Stack Frame shows 0 - :6.
Debugger->Errors has Condition ' script.is_null() ' is true.

3.1: same as 3.0, but game crashes on most runs.

3.2, 3.2.1: crashes on every run. No output in editor but terminal outputs null, handle crash with signal 11, backtrace.

3.2.2, 3.2.3.rc3: same as 3.0 without script.is_null() error.

Backtrace from a crash in 3.2.2 (Calinou godot-3.2.2-debug-linux-gcc5.4) (crashed less than every 1/100 runs)
Running: /home/thomas/Downloads/Godot_v3.2.2-debug/godot.x11.tools.64 --path /home/thomas/Downloads/GodotIssues/16943 --remote-debug 127.0.0.1:6007 --allow_focus_steal_pid 28469 --position 208,1050
Loading resource: res://
ERROR: _load: Resource file not found: res://.
   At: core/io/resource_loader.cpp:282.
Godot Engine v3.2.2.stable.custom_build.6a9fbafcb - https://godotengine.org
OpenGL ES 3.0 Renderer: GeForce GTX 750 Ti/PCIe/SSE2
 
Null
handle_crash: Program crashed with signal 11
Dumping the backtrace. Please include this when reporting the bug on https://github.com/godotengine/godot/issues
[1] /lib/x86_64-linux-gnu/libc.so.6(+0x354c0) [0x7faa5a20a4c0] (??:0)
[2] StringName::operator String() const (/godot/./core/string_name.h:122)
[3] GDScriptFunction::call(GDScriptInstance*, Variant const**, int, Variant::CallError&, GDScriptFunction::CallState*) (/godot/modules/gdscript/gdscript_function.cpp:1594)
[4] GDScriptInstance::_ml_call_reversed(GDScript*, StringName const&, Variant const**, int) (/godot/modules/gdscript/gdscript.cpp:1268)
[5] GDScriptInstance::call_multilevel_reversed(StringName const&, Variant const**, int) (/godot/modules/gdscript/gdscript.cpp:1277)
[6] Node::_notification(int) (/godot/scene/main/node.cpp:152)
[7] Node::_notificationv(int, bool) (/godot/./scene/main/node.h:46 (discriminator 14))
[8] Object::notification(int, bool) (/godot/core/object.cpp:934)
[9] Node::_propagate_ready() (/godot/scene/main/node.cpp:197)
[10] Node::_propagate_ready() (/godot/scene/main/node.cpp:186 (discriminator 2))
[11] Node::_set_tree(SceneTree*) (/godot/scene/main/node.cpp:2574)
[12] SceneTree::init() (/godot/scene/main/scene_tree.cpp:464)
[13] OS_X11::run() (/godot/platform/x11/os_x11.cpp:3259)
[14] /home/thomas/Downloads/Godot_v3.2.2-debug/godot.x11.tools.64(main+0x116) [0x1424b7c] (/godot/platform/x11/godot_x11.cpp:57)
[15] /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf0) [0x7faa5a1f5840] (??:0)
[16] /home/thomas/Downloads/Godot_v3.2.2-debug/godot.x11.tools.64(_start+0x29) [0x1424999] (??:?)
-- END OF BACKTRACE --
Socket error: 104
Loading resource: res://
ERROR: _load: Resource file not found: res://.
   At: core/io/resource_loader.cpp:282.
Loading resource: res://
ERROR: _load: Resource file not found: res://.
   At: core/io/resource_loader.cpp:282.

tl;dr: MRP does still crash Godot, sometimes.


Using call_deferred appears to work, at least in this particular test case:

A.gd

extends Node

func _ready():
	call_deferred("set_script", preload("res://B.gd"))
	call_deferred("set_process", true)
	print("A.gd: My final message.")

B.gd

extends Node

func _process(dt):
	print("B.gd: process")

produces good output (sample from 3.2.2):

--- Debugging process started ---
Godot Engine v3.2.2.stable.custom_build.6a9fbafcb - https://godotengine.org
OpenGL ES 3.0 Renderer: GeForce GTX 750 Ti/PCIe/SSE2
 
A.gd: My final message.
B.gd: process
B.gd: process
(...)
B.gd: process
B.gd: process
--- Debugging process stopped ---

Tested with 3.0, 3.1, 3.2, 3.2.1, 3.2.2 (50 runs), 3.2.3.rc3, all successful.

Small quirk: in recent versions where you don't need to set_process(true), you can omit it but A.gd must have a dummy _process function of its own instead, or B.gd's won't be called.

@YuriSizov YuriSizov removed this from the 4.0 milestone Feb 23, 2023
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

6 participants