Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

refactor: Component rebalancing is now performed via a global queue #2352

Merged
merged 10 commits into from
Feb 26, 2023

Conversation

st-pasha
Copy link
Contributor

Description

This PR ensures that all component rebalancing operations are resolved from a single location, after the update stage but before the render stage (thus, components may get reordered during the update, and these changes will go into effect during the rendering step on the same game tick).

This also fixes the problem where the child changing the priorities of its parent would cause a ConcurrentModificationError.

A number of methods that were used to handle rebalancing are now marked as deprecated. From the user's perspective, the only API they should be using is the .priority setter.

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?

  • [-] Yes, this PR is a breaking change.
  • No, this PR is not a breaking change.

Related Issues

Closes #2263

Copy link
Member

@spydon spydon left a comment

Choose a reason for hiding this comment

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

Good stuff! This should make it quite easy to introduce global priorities too right?

}
}

/// Usually this is not something that the user would want to call since the
/// component list isn't re-ordered when it is called.
/// See FlameGame.changePriority instead.
@Deprecated('Will be removed in 1.8.0. Use priority setter instead.')
Copy link
Member

Choose a reason for hiding this comment

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

🥳
Nice, so much cleaner without this!

@spydon spydon requested a review from a team February 20, 2023 22:01
}
}
import '../custom_component.dart';
// ignore_for_file: deprecated_member_use_from_same_package
Copy link
Member

Choose a reason for hiding this comment

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

what does it use that is deprecated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe it's mostly calls like game.children.changePriority(firstComponent, 11);. Some time later we will refactor the test file to use more straightforward firstComponent.priority = 11;, but for now I wanted to leave everything mostly as-is to ensure that the change is non-breaking.

Copy link
Member

@luanpotter luanpotter left a comment

Choose a reason for hiding this comment

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

LGTM with one comment

@spydon spydon merged commit 1ef5187 into main Feb 26, 2023
@spydon spydon deleted the ps.priorities branch February 26, 2023 23:45
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RouterComponent.pushOverlay() causes concurrent modification exception.
3 participants