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.

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

SpriteWidget has unused parameters #3071

Closed
1 task done
YukiAttano opened this issue Mar 7, 2024 · 1 comment · Fixed by #3074
Closed
1 task done

SpriteWidget has unused parameters #3071

YukiAttano opened this issue Mar 7, 2024 · 1 comment · Fixed by #3074
Assignees
Labels

Comments

@YukiAttano
Copy link
Contributor

YukiAttano commented Mar 7, 2024

What happened?

The SpriteWidget Class has two parameters, "srcPosition" and "srcSize", that are not used.

What do you expect?

I expect that this is either noted in the documentation or that the parameters are not requested in the constructor.

How can we reproduce this?

  // The fields of the class

  final Anchor anchor;

  final double angle;

  /// Holds the position of the sprite on the image
  final Vector2? srcPosition;

  /// Holds the size of the sprite on the image
  final Vector2? srcSize;

  final WidgetBuilder? errorBuilder;

  final WidgetBuilder? loadingBuilder;

  final FutureOr<Sprite> _spriteFuture;
 // The build method.
 // Here you can see the fields that are used.
 // srcPosition and srcSize are directed into the _spriteFuture, which we see at in the last code block.

 @override
  Widget build(BuildContext context) {
    return BaseFutureBuilder<Sprite>(
      future: _spriteFuture,
      builder: (_, sprite) {
        return InternalSpriteWidget(
          sprite: sprite,
          anchor: anchor,
          angle: angle,
        );
      },
      errorBuilder: errorBuilder,
      loadingBuilder: loadingBuilder,
    );
  }
  const SpriteWidget({
    required Sprite sprite,
    this.anchor = Anchor.topLeft,
    this.angle = 0,
    this.srcPosition, // mentioned in the constructor, but never used inside the class
    this.srcSize,       // check the constructor below.
    this.errorBuilder,
    this.loadingBuilder,
    super.key,
  }) : _spriteFuture = sprite;

  /// Load the image from the asset [path] and renders it as a widget.
  ///
  /// It will use the [loadingBuilder] while the image from [path] is loading.
  /// To render without loading, or when you want to have a gapless playback
  /// when the [path] value changes, consider loading the image beforehand
  /// and direct pass it to the default constructor.
  SpriteWidget.asset({
    required String path,
    Images? images,
    this.anchor = Anchor.topLeft,
    this.angle = 0,
    this.srcPosition,
    this.srcSize,
    this.errorBuilder,
    this.loadingBuilder,
    super.key,
  }) : _spriteFuture = Sprite.load(
          path,
          srcSize: srcSize,
          srcPosition: srcPosition, // here, srcPosition and srcSize are directed into the _spriteFuture but never used in the class itself
          images: images,
        );

What steps should take to fix this?

Because the parameters are only used to be redirected into the Sprite.load() method in the SpriteWidget.asset() constructor, i would suggest that these fields should be removed from the SpriteWidget class.

The documentation of the constructor can therefore mention that these parameters should be used in the Sprite.load() method.

Affected platforms

All

Other information

Link to the file: packages/flame/lib/src/widgets/sprite_widget.dart

Are you interested in working on a PR for this?

  • I want to work on this
@YukiAttano YukiAttano added the bug label Mar 7, 2024
@spydon
Copy link
Member

spydon commented Mar 7, 2024

Agree, they can be removed from that constructor.
I'll assign you to the issue since you've noted that you want to work on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants