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.

Sign up for GitHub

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

Jump to bottom

feat: Add HasWorldReference mixin #2746

Merged
merged 31 commits into from
Sep 20, 2023
Merged

feat: Add HasWorldReference mixin #2746

merged 31 commits into from
Sep 20, 2023

Conversation

spydon
Copy link
Member

@spydon spydon commented Sep 18, 2023 •
edited
Loading

Description

Adds a mixin for components similar to HasAncestor, HasParent and HasGameReference but which provides access to the World which the component has as an ancestor.

Checklist

  • 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.

Breaking Change?

  • No, this PR is not a breaking change.

Related Issues

@spydon
feat!: Add CameraComponent to FlameGame
a126060
@spydon
Automatically add world
b0f8f58
@spydon
Merge remote-tracking branch 'origin/main' into spydon/camera-component
cde9892
@spydon
Fix Forge2DGame
8f29843
@spydon
Fix some tests
ea9c170
@spydon
Make viewport and viewfinder changeable
e0f5421
@spydon
Update tests
8f4a54f
@spydon
Add example of world as level
f9be615
@spydon
Update tests
5359425
@spydon
Merge remote-tracking branch 'origin/main' into spydon/camera-component
1187e7d
@spydon
T -> W for world generics
c20b5d1
@spydon
Fix tiled_test
7626ca7
@spydon
Rename cameraComponent field to camera
ef4e8f0
@spydon
Update dartdoc
469b5d2
@spydon
Fix tiled example
d2e1d80
@spydon
Fix flame_lottie example
1f6e196
@spydon
Fix particles example camera
11ef505
@spydon
Fix zoom for particles example
d25006d
@spydon
Add assertion to explain world generics error better
9e5b25c
@spydon
HasWorldReference
d1614eb
@spydon
Merge remote-tracking branch 'origin/main' into spydon/has-world-refe…
3d68aa1 
…rence
@spydon
feat: Add HasWorldReference mixin
0eeba8d
@spydon
Remove accidentally added files
e4dccbe
@spydon
Fix test
c1a34a6
@spydon
Add docs
ed15c46
@spydon
HasWorldReference -> HasWorld
37ba71a
@spydon
Update test
2568805
@spydon spydon changed the title feat: Add HasWorldReference mixin feat: Add HasWorld mixin Sep 18, 2023
erickzanardo
erickzanardo reviewed Sep 19, 2023
View reviewed changes
doc/flame/components.md Outdated
@@ -189,6 +189,28 @@ that they will appear in the children list in the same order as they were
scheduled for addition.


### Access to the World from a Component

When a component requires access to the `World` that it is attached to one can
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unsure how to improve but the "that is attached to one" seems a bit confusing to me, maybe you meant when the user need to attach and have access to a world instance?

ufrshubham
ufrshubham reviewed Sep 20, 2023
View reviewed changes
Copy link
Member

@ufrshubham ufrshubham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why a separate mixin is needed for this. Wouldn't HasAncestor<MyWorld> achieve the same result?

@spydon
Copy link
Member Author

spydon commented Sep 20, 2023

Not sure why a separate mixin is needed for this. Wouldn't HasAncestor<MyWorld> achieve the same result?

It would, but the world will be very used in the upcoming release and when you use HasAncestor you probably want to use it for another ancestor than the root world a lot of the time and you can't use the mixin twice.

@ufrshubham
Copy link
Member

Okay, in that case, should we call it HasAncestorWorld or HasWorldAncestor? Or maybe BelongsToWorld, DescendantOfWorld. HasWorld makes me think that the component is saying it has a world within it 😅

@spydon
Copy link
Member Author

spydon commented Sep 20, 2023

Okay, in that case, should we call it HasAncestorWorld or HasWorldAncestor? Or maybe BelongsToWorld, DescendantOfWorld. HasWorld makes me think that the component is saying it has a world within it 😅

Haha we actually discussed exactly that in the chat yesterday! It was previously called HasWorldReference to be in line with HasGameReference, Luan also had the same argument as you, so time to change back I think (or to HasWorldAncestor maybe).

@spydon spydon changed the title feat: Add HasWorld mixin feat: Add HasWorldReference mixin Sep 20, 2023
@spydon spydon merged commit 9105411 into main Sep 20, 2023
7 checks passed
@spydon spydon deleted the spydon/has-world-reference branch September 20, 2023 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@erickzanardo erickzanardo erickzanardo left review comments

@wolfenrain wolfenrain wolfenrain approved these changes

@ufrshubham ufrshubham ufrshubham approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@spydon @ufrshubham @erickzanardo @wolfenrain