feat: Draft for new Layout components Row and Column #1971

rivella50 · 2022-09-30T13:20:28Z

Description

This is a draft for the planned row and column components for Flame.
At the moment it only covers the horizontal direction class RowComponent.
As soon as we are clear what features these 2 components will have, the class ColumnComponent will be added accordingly.
How to use RowComponent can be seen in issue #1944 .

To be discussed:

how shall the layouting routine behave if the children don't have the same anchor type?
additional parameters involved?

Checklist

The title of my PR starts with a [Conventional Commit] prefix (fix:, feat:, docs: etc).
I have followed the [Contributor Guide] when preparing my PR.
I have updated/added tests for ALL new/updated/fixed functionality.
I have updated/added relevant documentation in docs and added dartdoc comments with ///.
[-] I have updated/added relevant examples in examples or docs.
No, this PR is not a breaking change.

Related Issues

Closes #1944

st-pasha · 2022-09-30T18:32:48Z

I'm worried about overriding the add() and remove() methods, since we don't really provide a guarantee that these are the only ways to add or remove children components. For example, there are also addAll(), removeAll(), addToParent(), removeFromParent(), changeParent(), adding via the constructor; more methods like these can be added in the future.

I believe a reasonable alternative would be to:

not subscribe to size changes at all: it is a very rare operation after all, especially for the kinds of components that get put in rows/columns;
make the layout() method public, so that in a rare circumstance that a child does need to change its size, it would simply instruct the parent to re-do the layout manually;
create a new lifecycle callback Component.onChildrenChanged() which would be invoked whenever children components are added/removed (<-- this should probably go in a separate PR);
do not require that children of LayoutComponent must be PositionComponents -- what if someone wants to add an Effect for example? The approach taken in layoutChildren() is better: simply perform a filter. And even that can be improved: instead of looking for PositionComponents, use PositionProvider / SizeProvider (the PositionComponent satisfies both of these interfaces).

rivella50 · 2022-09-30T19:28:09Z

@st-pasha Thanks for the feedback. Makes all sense imo.

rivella50 · 2022-10-01T05:34:31Z

@st-pasha Regarding the filter in layoutChildren: You mean filtering like

final list = children.where((element) => element is PositionProvider && element is SizeProvider);

and then casting to one of the interfaces where needed, e.g.

for (final child in list) {
  (child as PositionProvider).position = Vector2(currentPosition.x, currentPosition.y);
  currentPosition.x += (child as SizeProvider).size.x + gap;  // width cannot be used anymore
}

rivella50 · 2022-10-05T19:01:12Z

With the merge of PR #1976 the class LayoutComponent could get rid of the overridden methods add and remove and now only has to override the new method onChildrenChanged in order to get notified about any change in a parent's children and therefore know when to relayout them.

rivella50 · 2022-10-06T07:28:46Z

@spydon Can we discuss how to proceed with this draft?
I.e. what's missing in functionality in order to be released?
Writing tests won't be a problem.

erickzanardo

Two small nits but I think this good for an initial approach.

I have two suggestions though

The code between Column and Row components are very similar, I wonder if we could share some methods/logic between them to avoid repetition.
I would suggest to move these components to the experimental namespace, that we way we have the freedom to explore this new functionality without fear of breaking changes.

packages/flame/lib/src/components/column_component.dart

packages/flame/lib/src/components/row_component.dart

rivella50 · 2022-10-06T13:53:20Z

@erickzanardo Thanks for the feedback. I've refactored the 3 classes as follows:

method layoutChildren has been moved up to class LayoutComponent
a switch statement is used now
all 3 classes have been moved to the experimental namespace

erickzanardo · 2022-10-06T16:55:35Z

Awesome. From my part. I would say this is ready to receive tests and examples.

Once we have RowComponent and ColumnComponent on main, we can start expanding this, like an ExpandedComponent, a Flex one and so on.

rivella50 · 2022-10-06T17:03:47Z

Great, thank you, Erick. Then i will write some tests for it.
I also hope that these components will receive many more extensions.

spydon

Just the sizes for the components that are missing I think (and tests for the sizes), otherwise it looks good!

spydon · 2022-10-07T10:24:27Z

packages/flame/lib/src/experimental/layout_component.dart

+      return;
+    }
+    final currentPosition = Vector2.zero();
+    final componentsDimension = list.fold<double>(


This should be set to the PositionComponent's width or height right? And then the one that isn't set here should be the max width/height of the child components.

You mean componentsDimension? This only sums up the width (or height) for all components which will be used to calculate the free space in all spaceXY and the center alignment type.

The size of the parent component is checked and handled here:

flame/packages/flame/lib/src/experimental/layout_component.dart

Lines 45 to 48 in 7486750

final dimensionAvailable = size[vectorIndex] != 0.0

? size[vectorIndex] - absoluteTopLeftPosition[vectorIndex]

: gameRef.canvasSize[vectorIndex] -

absoluteTopLeftPosition[vectorIndex];

dimensionAvailable should possibly be the parent's size, or it's own size, it shouldn't check anything regarding the canvas size or such.
So either we let the size of the LayoutComponent grow dynamically with the children added to it, or it has a set size from the beginning, but to make it more flutter like I think that it should grow the size itself and then just be constrained by the parent's size.
Because right now the layout component's size will be (0, 0) now since it isn't updated when children are added. The setter of size should probably also be overridden so that it can't be set too if we choose to go with a dynamic size.

But can the parent not also have a zero size?
I had it in mind like this: if the developer defines a size for LayoutComponent then this size will be taken for the available dimension calculation, else it just takes the canvas' width or height for the end edge.

As you like. It's already implemented though.

Perhaps let's not overcomplicate this?

In the first version of the LayoutComponent the size will have to be provided explicitly. In the next version (in a separate PR), we will relax that requirement and allow size to be omitted, in which case it will be derived from a parent.

When re-reviewing this I agree, maybe it's a bit overkill to listen to the parent's size.
Then we can scrap isManuallySized too, so that the component only grows to its size, can we restrict the user from changing the size variable from the outside though? Then we only need to do the layout when children are added.

Later we could add a WrapComponent so that components can be organized within a set size.

What do you think about this @rivella50? :)

@spydon Sure, why not. I've already removed setting the size and isManuallySized last weekend.
Just to be clear: Without being able to set the size (parent or LayoutComponent) the alignment types spaceXY don't make sense anymore.

Without being able to set the size (parent or LayoutComponent) the alignment types spaceXY don't make sense anymore.

Hmm, that's true, a shame to drop that one...
But maybe it is worth dropping it to just have a simple first version?

spydon · 2022-10-16T13:09:14Z

doc/flame/components.md

@@ -1104,6 +1104,21 @@ Check the example app
 for details on how to use it.


+## LayoutComponent


If we won't expose this we should change the docs to be about RowComponent and ColumnComponent instead.

spydon · 2022-10-16T13:22:48Z

packages/flame/lib/src/experimental/layout_component.dart

+      return;
+    }
+    final currentPosition = Vector2.zero();
+    final componentsDimension = list.fold<double>(


Perhaps let's not overcomplicate this?

In the first version of the LayoutComponent the size will have to be provided explicitly. In the next version (in a separate PR), we will relax that requirement and allow size to be omitted, in which case it will be derived from a parent.

When re-reviewing this I agree, maybe it's a bit overkill to listen to the parent's size.
Then we can scrap isManuallySized too, so that the component only grows to its size, can we restrict the user from changing the size variable from the outside though? Then we only need to do the layout when children are added.

Later we could add a WrapComponent so that components can be organized within a set size.

packages/flame/lib/src/experimental/layout_component.dart

spydon · 2022-10-23T15:36:01Z

packages/flame/lib/src/experimental/layout_component.dart

+  final MainAxisAlignment alignment;
+  bool _allowSetSize = false;
+
+  /// gap between components


Suggested change

/// gap between components

packages/flame/lib/src/experimental/layout_component.dart

luanpotter · 2022-10-29T16:24:57Z

packages/flame/test/experimental/layout_component_test.dart

+  for (var k = 0; k <= 1; k++) {
+    group(k == 0 ? 'RowComponent' : 'ColumnComponent', () {


would this be better as:

for (final name in ['RowComponent', 'ColumnComponent']) {

or, maybe even better, extract the block to a function and call it twice?

group('RowComponent', testLayoutComponent(RowComponent)) group('ColumnComponent', testLayoutComponent(ColumnComponent))

luanpotter · 2022-10-29T16:29:35Z

packages/flame/lib/src/experimental/layout_component.dart

+    _allowSetSize = true;
+    size[vectorIndex] = totalSizeOfComponents;
+    _allowSetSize = false;


I don't think you need this variable -- can you just user super.size?

example:

class A { double size = 1.0; } class B extends A { @override set size(double size) { assert(false, 'Setting the size is currently unsupported.'); } void foo() { // this.size = 2.0; -> this breaks super.size = 2.0; } } void main() { final b = B()..foo(); print(b.size); }

luanpotter · 2022-10-29T16:31:03Z

packages/flame/lib/src/experimental/row_component.dart

+/// type.
+class RowComponent extends LayoutComponent {
+  RowComponent({
+    LayoutComponentAlignment alignment = LayoutComponentAlignment.start,


can't you use super.alignment here?

packages/flame/test/experimental/layout_component_test.dart

luanpotter · 2022-10-29T16:44:03Z

doc/flame/components.md

@@ -1104,6 +1104,18 @@ Check the example app
 for details on how to use it.


+## RowComponent & ColumnComponent


if it is experimental, should we flag it in the doc, similar to how it is done elsewhere?

spydon · 2022-11-08T21:48:03Z

Do you want to continue on this PR @rivella50 or should we take it over? :)
(Great work so far btw!)

rivella50 · 2022-11-09T06:27:47Z

@spydon Feel free to take it over. My current project doesn't let me much free time at the moment.

spydon · 2023-04-16T14:15:51Z

I'll close this for now since it's been open for so long, but we should definitely look at this when creating these components based on the AlignComponent.

Valentin Treu added 2 commits September 30, 2022 15:11

Adding draft for ComponentRow

a151fd1

Formatting the code

6490ec1

rivella50 marked this pull request as draft September 30, 2022 13:49

Valentin Treu added 2 commits September 30, 2022 19:41

correct format errors

1b499d1

correct analyze errors

0a58df6

Valentin Treu added 3 commits October 1, 2022 16:33

Rename ComponentRow to RowComponent

462dee6

Merge branch 'main' into layout-components

a7dbc2a

Switching to onChildrenChanged notifier

0f6c853

Valentin Treu added 2 commits October 5, 2022 21:43

changing class documentations

881faf3

adding ColumnComponent

dc46f17

spydon requested review from a team and st-pasha October 6, 2022 08:39

erickzanardo reviewed Oct 6, 2022

View reviewed changes

packages/flame/lib/src/components/column_component.dart Outdated Show resolved Hide resolved

packages/flame/lib/src/components/row_component.dart Outdated Show resolved Hide resolved

Refactor children of LayoutComponent

734a4bd

erickzanardo requested a review from a team October 6, 2022 14:02

Valentin Treu added 3 commits October 6, 2022 21:50

first tests and bugfix for spaceXY alignment types

47d7491

format code

cd83a85

bugfix for MainAxisAlignment.end and tests for ColumnComponent

7486750

spydon reviewed Oct 7, 2022

View reviewed changes

Valentin Treu added 3 commits October 8, 2022 15:26

LayoutComponent now handles size or parent size

01acbd3

fixing analyze problems

08d731e

fix format issue

00044a5

Valentin Treu added 4 commits October 10, 2022 20:29

apply feedback from pasha

7fbde92

Refactor tests for LayoutComponent

2614d77

Adding documentation

4c08854

Correct Lint errors

467e893

spydon reviewed Oct 16, 2022

View reviewed changes

Remove size and disallow resizing a LayoutComponent

6c32ea3

rivella50 marked this pull request as ready for review October 16, 2022 17:26

Fix format error

5cf1131

st-pasha reviewed Oct 17, 2022

View reviewed changes

packages/flame/lib/src/experimental/layout_component.dart Outdated Show resolved Hide resolved

Add assert statement in set size

2124cda

spydon reviewed Oct 17, 2022

View reviewed changes

packages/flame/lib/src/experimental/layout_component.dart Outdated Show resolved Hide resolved

Adding an assert message

9b51c96

spydon reviewed Oct 23, 2022

View reviewed changes

Reduce the alignment modes to start, end and center

59ba7c4

rivella50 requested a review from spydon October 29, 2022 13:12

wolfenrain approved these changes Oct 29, 2022

View reviewed changes

luanpotter reviewed Oct 29, 2022

View reviewed changes

packages/flame/test/experimental/layout_component_test.dart Outdated Show resolved Hide resolved

luanpotter reviewed Oct 29, 2022

View reviewed changes

Use Vector2.all in tests where suitable

45fde7e

spydon added the hacktoberfest-accepted label Oct 30, 2022

st-pasha mentioned this pull request Jan 28, 2023

Create LayoutComponent classes #2302

Open

spydon marked this pull request as draft February 22, 2023 09:30

spydon closed this Apr 16, 2023

rivella50 mentioned this pull request Jul 15, 2023

feat: Added AlignComponent layout component #2350

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Draft for new Layout components Row and Column #1971

feat: Draft for new Layout components Row and Column #1971

rivella50 commented Sep 30, 2022 •

edited

Loading

st-pasha commented Sep 30, 2022

rivella50 commented Sep 30, 2022

rivella50 commented Oct 1, 2022

rivella50 commented Oct 5, 2022

rivella50 commented Oct 6, 2022

erickzanardo left a comment

rivella50 commented Oct 6, 2022

erickzanardo commented Oct 6, 2022

rivella50 commented Oct 6, 2022

spydon left a comment

spydon Oct 7, 2022

rivella50 Oct 7, 2022

rivella50 Oct 7, 2022

spydon Oct 7, 2022 •

edited

Loading

rivella50 Oct 7, 2022

rivella50 Oct 9, 2022

spydon Oct 16, 2022

spydon Oct 21, 2022

rivella50 Oct 21, 2022

spydon Oct 23, 2022

spydon Oct 16, 2022

spydon Oct 16, 2022

spydon Oct 23, 2022

luanpotter Oct 29, 2022

luanpotter Oct 29, 2022

luanpotter Oct 29, 2022

luanpotter Oct 29, 2022

spydon commented Nov 8, 2022

rivella50 commented Nov 9, 2022

spydon commented Apr 16, 2023

	final dimensionAvailable = size[vectorIndex] != 0.0
	? size[vectorIndex] - absoluteTopLeftPosition[vectorIndex]
	: gameRef.canvasSize[vectorIndex] -
	absoluteTopLeftPosition[vectorIndex];

		@@ -1104,6 +1104,21 @@ Check the example app
		for details on how to use it.


		## LayoutComponent

		for (var k = 0; k <= 1; k++) {
		group(k == 0 ? 'RowComponent' : 'ColumnComponent', () {

		@@ -1104,6 +1104,18 @@ Check the example app
		for details on how to use it.


		## RowComponent & ColumnComponent

feat: Draft for new Layout components Row and Column #1971

feat: Draft for new Layout components Row and Column #1971

Conversation

rivella50 commented Sep 30, 2022 • edited Loading

Description

Checklist

Related Issues

st-pasha commented Sep 30, 2022

rivella50 commented Sep 30, 2022

rivella50 commented Oct 1, 2022

rivella50 commented Oct 5, 2022

rivella50 commented Oct 6, 2022

erickzanardo left a comment

Choose a reason for hiding this comment

rivella50 commented Oct 6, 2022

erickzanardo commented Oct 6, 2022

rivella50 commented Oct 6, 2022

spydon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

spydon Oct 7, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

spydon commented Nov 8, 2022

rivella50 commented Nov 9, 2022

spydon commented Apr 16, 2023

rivella50 commented Sep 30, 2022 •

edited

Loading

spydon Oct 7, 2022 •

edited

Loading