-
-
Notifications
You must be signed in to change notification settings - Fork 934
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
fix: Remove deprecations for 1.10.0 #2809
Conversation
doc/flame/camera_component.md
Outdated
@@ -262,8 +262,7 @@ if (!camera.canSee(component)) { | |||
|
|||
## Comparison to the deprecated camera |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we remove the entire section now?
doc/flame/inputs/tap_events.md
Outdated
- If your component needs to know the coordinates of the point of touch, use | ||
`event.localPosition` instead of computing it manually. Properties `event.canvasPosition` and | ||
`event.devicePosition` are also available. | ||
- If the component is a `PositionComponent`, then make sure its size is set correctly (for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand what you mean with for example by turning on the debug mode
, but I think it is a bit misleading, seem to be implying turning that on will cause the size to be set.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't write this, but I can fix it up!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I realized this point is not even related to the migration so I removed it.
@@ -83,7 +78,7 @@ class Viewfinder extends Component | |||
} | |||
|
|||
/// Reference to the parent camera. | |||
CameraComponent get camera => parent; | |||
CameraComponent get camera => parent! as CameraComponent; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this better?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not, but we have to be able to check if parent
is null at some places and parentIsA
doesn't allow that.
@internal | ||
Transform2D get transform => _transform; | ||
/// Transform matrix used by the viewfinder. | ||
final Transform2D transform = Transform2D(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we can't prevent people from doing viewport.transform.transformMatrix.storage[0] = 0
or something...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, if they do that they'll have to suffer the consequences 😂
/// Remember to set [devicePixelRatio] before using the viewport; a good place | ||
/// to update it is in handleResize of your game, directly from | ||
/// WidgetBindings.instance.window.devicePixelRatio. | ||
class FixedIntegerResolutionViewport extends Viewport { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why remove?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because it's a viewport for the old camera
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Created #2810 to track this for the new camera, since it was in the experimental section I don't think it matters much that we remove it momentarily (it was used for a specific rendering bug in Flutter).
@@ -220,12 +201,6 @@ class FlameGame<W extends World> extends ComponentTreeRoot | |||
Duration.microsecondsPerSecond; | |||
} | |||
|
|||
@override | |||
Projector get viewportProjector => oldCamera.viewport; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do still have anything that uses this interface Projector?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not after this PR
@@ -41,4 +41,4 @@ void main() { | |||
} | |||
|
|||
class _ComponentWithViewportMargin extends PositionComponent | |||
with HasGameRef, ComponentViewportMargin {} | |||
with HasGameReference, ComponentViewportMargin {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
did we reach a consensus about this? should this be a separate PR/discussion?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think a long time ago we decided to phase out HasGameRef
and replace it with HasGameReference
, there should be some old issue for that. We just haven't phased it out everywhere yet, so I do that when I stumble upon them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you so much for finally deleting the old code path! That makes me so much happy...
Description
Removed deprecations for 1.10.0 and fixes some small unreleased regressions found when going through the examples.
Checklist
docs
and added dartdoc comments with///
.examples
ordocs
.Breaking Change?
Related Issues