Add manual_fit_child_rect In BoxContainer

[mnikn]

trafficstars

Closes https://github.com/godotengine/godot-proposals/issues/10437, related https://github.com/godotengine/godot-proposals/issues/9616

In BoxContainer,add a flag manual_fit_child_in_rect and signal request_fit_child_in_rect.

when manual_fit_child_in_rect=true, it will not call method fit_child_in_rect and emit signal request_fit_child_in_rect, so that user can implement custom positioning.

usage for custom animation positioning in gdscript:

extends Control

# Called when the node enters the scene tree for the first time.
func _ready() -> void:
	$HBoxContainer.set_manual_fit_child_in_rect(true)
	$HBoxContainer.connect("request_fit_child_in_rect", self._handle_request_fit_child_in_rect)
	await self.get_tree().create_timer(1.0).timeout
	
	for i in range(5):
		var node = ColorRect.new()
		node.custom_minimum_size = Vector2(64, 64)
		node.size = node.custom_minimum_size
		node.color = Color("#fff")
		$HBoxContainer.add_child(node)
		await self.get_tree().create_timer(1.0).timeout
	pass


func _handle_request_fit_child_in_rect(c: Control, rect: Rect2) -> void:
	if c.has_meta("origin_pos"):
		var origin_pos = c.get_meta("origin_pos")
		c.global_position = origin_pos
	else:
		c.global_position.x = -240
	var target_pos = $HBoxContainer.global_position + rect.position
	c.set_meta("origin_pos", target_pos)
	var tween = self.create_tween()
	tween.tween_property(c, "global_position", target_pos, 0.2)

Feel free to give any feedback!

[KoBeWi]

Also related: #87081 and likely #51547 I don't think this property makes sense specifically in BoxContainer. While it's probably the most commonly used one, you can as well run into this issue in other containers too.

[groud]

I do agree with @KoBeWi here, and I believe #87081 is a better solution to the problem as it supports rotation/skew too.

However, if we want to keep a similar solution to be implemented for all containers, I would probably reuse the system we already have with custom containers, but allowing them to override/build upon the base class's behavior, something like that.

[mnikn]

@groud Yes, I agree we should reuse the custom container system, but I think the current solution can also handle rotation/skew because we just emit the signal with the calculated positions. Users can do whatever they want, including rotation in this method. https://github.com/godotengine/godot/pull/87081 seems less flexible to me because we can only set the offset parameter to control position/rotation, but we can't get the calculated position. This limitation makes the animation less flexible if it needs the calculated position.

@KoBeWi I agree that this property should not be limited to BoxContainer. What do you think about moving the property and signal to Container, when Container call fit_child_in_rect, check the flag and emit singal? This change would benefit all child containers.

void Container::fit_child_in_rect(Control *p_child, const Rect2 &p_rect) {
        if (manual_fit_child_in_rect) {
            emit_signal(SceneStringName(request_fit_child_in_rect), p_child, p_rect);
            return
        }
        // ...origin logic
}

[KoBeWi]

What do you think about moving the property and signal to Container, when Container call fit_child_in_rect, check the flag and emit singal?

That makes more sense.

[kitbdev]

when Container call fit_child_in_rect, check the flag and emit singal?

In this case, shouldn't we just make fit_child_in_rect virtual? I don't think it needs to be a signal.

[mnikn]

@kitbdev Use virtual method can only let child container override this method, but I want to let user script override the fit_child_in_rect behaviour, seems that only signal can communicate with user script. Any idea can we do this without signal?

[KoBeWi]

Check GDVIRTUAL. It allows making virtual methods to be overriden in script.

[mnikn]

Changed to use GDVIRTUAL override fit_child_in_rect in Container, feel free to give any feedback!

[KoBeWi]

Overriding _fit_child_in_rect makes the container no longer work. Maybe the method should return bool to enable/disable override. Another thing is that container's minimum size will use the default layout, not sure if that's expected 🤔 Also the virtual method is missing argument names.

tbh this solution is rather hacky; it allows manually fitting the children using code, but it can't be used with e.g. AnimationPlayer. Also to animate it with code, you need to continuously sort the children.

[mnikn]

@KoBeWi I agree it's kinda hack solution, but I think override _fit_child_in_rect can do any animation not only sort, and in my case when a child position has changed, it need to smoothly move to target position from origin position, so continuously sort the children is actually what I want. I think this solution maybe hacky, but it's the least effort to archive my goal, or any advice can we do this with different way?

[KoBeWi]

I'm not sure about the best solution. #87081 is the only one that allows using AnimationPlayer, but it duplicates transform properties.

I know some users use containers to set layout and then change their type to Control to allow animating. This aligns with an old proposal that suggested disabling container behavior, and IIRC it was rejected. It's likely the simplest solution, but it's only reliable with relative movement, so only Tweens.

Your solution is similar to the latter, except the animation has to be done in a certain way.

[mnikn]

@KoBeWi I get your point, rethink it in my case what I really want is a pure logic layouter, which will not process any ui, only calculate positions. I think it wolud be better if we can extract the layout calculate logic from container to a independent node or method, something like GraphEditArranger.