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.

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

Add ScheduleAfterChildren()-API to Containers #802

Merged
merged 3 commits into from Jun 8, 2017

Conversation

3 participants
@default0
Contributor

default0 commented Jun 7, 2017

Does what it says: Schedules a call to run the next time
UpdateAfterChildren() gets called.

default0 added some commits Jun 7, 2017

Add ScheduleAfterChildren()-API to Containers
Does what it says: Schedules a call to run the next time
UpdateAfterChildren() gets called.
@peppy

This comment has been minimized.

Show comment
Hide comment
@peppy

peppy Jun 8, 2017

Member

what's the use-case for this one?

Member

peppy commented Jun 8, 2017

what's the use-case for this one?

@Tom94

This comment has been minimized.

Show comment
Hide comment
@Tom94

Tom94 Jun 8, 2017

Collaborator
Collaborator

Tom94 commented Jun 8, 2017

@peppy

This comment has been minimized.

Show comment
Hide comment
@peppy

peppy Jun 8, 2017

Member

Is there an osu-level PR to move such things over? I would've thought there was at least one potential usage in the framework for this...

Member

peppy commented Jun 8, 2017

Is there an osu-level PR to move such things over? I would've thought there was at least one potential usage in the framework for this...

@Tom94

This comment has been minimized.

Show comment
Hide comment
@Tom94

Tom94 Jun 8, 2017

Collaborator
Collaborator

Tom94 commented Jun 8, 2017

@peppy

peppy approved these changes Jun 8, 2017

@peppy peppy merged commit 5b2bfe8 into ppy:master Jun 8, 2017

1 check passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
@peppy

This comment has been minimized.

Show comment
Hide comment
@peppy

peppy Jun 8, 2017

Member

TO be honest, every place I can find UpdateAfterChildren usages, a Cache-Invalidate-Refresh pattern looks to work better...

Member

peppy commented Jun 8, 2017

TO be honest, every place I can find UpdateAfterChildren usages, a Cache-Invalidate-Refresh pattern looks to work better...

@peppy

This comment has been minimized.

Show comment
Hide comment
@peppy

peppy Jun 8, 2017

Member

Adding a scheduler specifically for scrollToEnd feels very wrong. Just add a single internal state for that instead?

I mean there's so many other scenarios where scrollToEnd won't work too (if new content appears when a ScrollContainer is off-screen it won't work etc.) to the point we probably want a permanent ShouldBeAnimatedScrollingToTop bool or something.

management of keeping a scroll position should be the ScrollContainers responsibility IMO.�

Member

peppy commented Jun 8, 2017

Adding a scheduler specifically for scrollToEnd feels very wrong. Just add a single internal state for that instead?

I mean there's so many other scenarios where scrollToEnd won't work too (if new content appears when a ScrollContainer is off-screen it won't work etc.) to the point we probably want a permanent ShouldBeAnimatedScrollingToTop bool or something.

management of keeping a scroll position should be the ScrollContainers responsibility IMO.�

@default0

This comment has been minimized.

Show comment
Hide comment
@default0

default0 Jun 8, 2017

Contributor

Regarding UpdateAfterChildren(): While using the framework I've encountered situations where the literal action, ie "let my children update, then update me" is necessary to get the right order of operations, where this wasn't purely related to just sizes not getting invalidated.
The concrete situation is that I needed a parent container to update based on a childs position, and the childs positioning was managed by the child itself, so the parent has to wait for the childs update to complete before making decisions based on it.

I can imagine similar situations where I will want to use ScheduleAfterChildren() because I need to do an action of a similar kind - but schedule it to the next frame - even though the only use-case I currently have in my own code is making sure that a ScrollContainer properly scrolls to its end.

Contributor

default0 commented Jun 8, 2017

Regarding UpdateAfterChildren(): While using the framework I've encountered situations where the literal action, ie "let my children update, then update me" is necessary to get the right order of operations, where this wasn't purely related to just sizes not getting invalidated.
The concrete situation is that I needed a parent container to update based on a childs position, and the childs positioning was managed by the child itself, so the parent has to wait for the childs update to complete before making decisions based on it.

I can imagine similar situations where I will want to use ScheduleAfterChildren() because I need to do an action of a similar kind - but schedule it to the next frame - even though the only use-case I currently have in my own code is making sure that a ScrollContainer properly scrolls to its end.

@peppy

This comment has been minimized.

Show comment
Hide comment
@peppy

peppy Jun 8, 2017

Member

override UpdateAfterChildren in the parent?

Member

peppy commented Jun 8, 2017

override UpdateAfterChildren in the parent?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment