Discussing Architectural Patterns in Godot after 2 years of development

[dragonforge-dev]

@normalized

@dragonforge-dev

I’ve just always avoided it. I inferred that using get_children() and doing my own filtering is faster.

Don’t tell me you’re trying to optimize for performance here

I knew you were going to give me sh*t about this. LOL

You asking the question has honestly gotten me to rethink my stance on it - because I made this decision back when I was learning Godot and I was still in that over-optimization mindset so many of us experienced programmers come into Godot with. I fought against the idea of Autoloads for so long and insisted on making my own static singletons until I learned the benefits of Autoloads.

@normalized

I’d always opt for initializing via scene unique names but if it must be done via searching for nodes then all code snippets shown so far are quite pedestrian. You need to manually add initialization code whenever you add a component and you need to implement this initialization function in every class that uses components. Not very S O L I D

True. I think this is a result of me not really using components at scale yet, so I haven’t refactored that functionality.

@normalized

If you must do it that way, then implement a universal initializer in component base class:

I’m actually starting to lean towards what @surferix said:

@surferix

animation_player wiring

Yes, @export on the component:

class_name Collectible extends Node

@export var animation_player: AnimationPlayer
@export var area: Area2D
@export var pickup_animation: StringName = &"pickup"

func _ready() -> void:
    assert(animation_player != null, "Collectible: animation_player slot is empty")
    assert(area != null, "Collectible: area slot is empty")
    area.body_entered.connect(_on_body_entered)

You set the slots in the editor by drag-dropping the right node onto each @export field. Same model for every cross-node reference inside a scene — no $Path/To/Node, no get_node, no find_child, no group lookup. If the component is genuinely required, the assert in _ready makes a missing slot fail loudly at scene load; if it’s optional, it stays null and the caller guards on it.

The principle matters more than the syntax. This way the scene is the wiring diagram: open the Inspector, you see exactly which AnimationPlayer drives the pickup. Move the AnimationPlayer node anywhere in the scene, the reference stays valid. Delete it, the scene loads with a visible warning. Try the same with find_child("AnimationPlayer") and the link is invisible until it breaks at runtime.

The @export variables become the interface. And so instead of an annoyance, they become the way you wire something up. In something like my Camera2DComponent, the only thing it needs to know about is its parent, which is always a Camera2D. So much so, that it throws a configuration warning if it’s not added as a child of one.

@tool
@icon("res://addons/dragonforge_camera_2d/assets/textures/icons/component.svg")
@abstract class_name Camera2DComponent extends Node
## Abstract base class for components added to a [Camera2D]. Objects inheriting
## from this class can only be attached to a Camera2D node and will issue
## an editor warning if not.

## The Camera2D object to which this component is attached and operates on.
var camera: Camera2D


# Runs whenver the node is parented or reparented.
func _enter_tree() -> void:
	camera = null
	var parent = get_parent()
	if parent is Camera2D:
		camera = parent
	update_configuration_warnings()


# Overridden built-in function of the [Node] class.
func _get_configuration_warnings() -> PackedStringArray:
	var warnings: PackedStringArray = []
	if not camera is Camera2D:
		warnings.append("Camera2DComponent only serves to provide modifications to Camera2D derived nodes. Please only use it as a child of a Camera2D to modify it.")
	return warnings

@normalized

Component code

class_name Component extends Node

static var component_classes: Array

static func _static_init():
	component_classes = ProjectSettings.get_global_class_list()
	component_classes = component_classes.filter(func(global_class): return global_class.base == &"Component" )
	component_classes = component_classes.map(func(global_class): return global_class.class)

static func find_and_assign_components(componentized_node: Node):
	var properties = componentized_node.get_property_list().filter(func(prop): return prop.usage & PROPERTY_USAGE_SCRIPT_VARIABLE and prop.class_name in component_classes)
	for prop in properties:
		var nodes = componentized_node.find_children("", prop.class_name, false, false)
		assert(nodes.size() == 1, "Number of %s components for %s must be exactly 1" % [prop.class_name, componentized_node])
		componentized_node.set(prop.name, nodes[0])

And just call it for each class that needs to assign its component variables:

Component.find_and_assign_components(self)

No additional code needed ever, just declare component vars and enjoy.

I don’t know that there’s a need for a static helper for that kind of thing. In fact, I think it might be an anti-pattern. Which perhaps should have been my answer to @amarc. Because it might be good to try and make sure that Components don’t ever need to know about each other whenever possible. Things like a Poison status effect are an outlier, and might actually be better implemented not as a Node, but as a Resource that is applied to a Health component node.