godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
88.89k stars 20.16k forks source link

struck at splash screen after adding child inside _draw() function #95810

Open syntaxerror247 opened 3 weeks ago

syntaxerror247 commented 3 weeks ago

Tested versions

System information

Godot v4.3.stable

Issue description

When adding a child inside _draw(), it gets called infinitely. I think adding new child is calling _draw() again and a new child is added and it goes on and on.... This issue was not present in godot 4.2.2 and new child was added without any problem

Steps to reproduce

Minimal reproduction project (MRP)

null

syntaxerror247 commented 3 weeks ago

As a workaround, I created a can_draw variable and made it true after a process_frame, now adding child is not triggering _draw() func. So maybe, it only occurs if _draw() is called(or child is added ) in same process_frame as _ready()

This is my workaround code

extends Control
var can_draw = false
func _draw() -> void:
    if not can_draw: return
    print("draw func called")
    var n = Label.new()
    add_child(n)

func _ready() -> void:
    await get_tree().process_frame
    can_draw = true
    queue_redraw()
AThousandShips commented 3 weeks ago

To me this isn't very surprising, but since it changed since 4.2 it's something to investigate, but I don't expect there to be any reasonable use case for adding nodes in _draw without doing some tracking, so this code isn't really valid generally IMO

This is by design, as per: https://github.com/godotengine/godot/blob/da5f39889f155658cef7f7ec3cc1abb94e17d815/scene/gui/control.cpp#L3299-L3303

Unsure why it doesn't happen in 4.2, could be that it incorrectly queues drawing immediately due to some flushing

Related:

AThousandShips commented 3 weeks ago

The immediate redraw is a bug, though the provided code is still invalid, or generally not appropriate as it'll add an infinite number of children as it'll redraw itself each time

So the error with immediate redraw is an issue, but is already tracked above, so this should probably focus on the possibility of documenting that adding children redraws (though I'd say that should be expected)

syntaxerror247 commented 3 weeks ago

though the provided code is still invalid, or generally not appropriate as it'll add an infinite number of children as it'll redraw itself each time

I am drawing a pie chart inside _draw() and adding new labels ( which are Label nodes) everytime it redraws(or changes) and delete previous one and it was working fine in 4.2.2

AThousandShips commented 3 weeks ago

Well that would be decent code, but your example doesn't include any removal so that's what I based my statement on 😆

The redrawing should block any queueing happening, so unsure what is broken here, if done during the draw, so unsure why this happens, will try testing it tomorrow