flame-engine / flame

A Flutter based game engine.
https://flame-engine.org
MIT License
9.19k stars 899 forks source link

SpriteWidget.srcPosition and srcSize are not really used #3194

Closed erickzanardo closed 3 months ago

erickzanardo commented 3 months ago

What happened?

SpriteWidget class imitates Sprite and has two fields srcPosition and srcSize.

Those are not used though, what matters are the fields inside the Sprite instance that it holds.

What do you expect?

We should remove those fields from SpriteWidget to avoid confusion.

How can we reproduce this?

No response

What steps should take to fix this?

No response

Do have an example of where the bug occurs?

No response

Relevant log output

No response

Execute in a terminal and put output into the code block below

Output of: flutter doctor -v

Affected platforms

All

Other information

No response

Are you interested in working on a PR for this?

spydon commented 3 months ago

They only seem to be present on the asset constructor and there they are used?

erickzanardo commented 3 months ago

Yeah, they kinda are, but then on the asset constructor we can just transform them into constructors argument, they don't need to be fields

erickzanardo commented 3 months ago

Those two fields in the default constructor just made me stare at my game for around 5 minutes trying to understand why my sprite was being rendered messed up hahah

spydon commented 3 months ago

But they aren't fields, and they aren't on the default constructor, are we really looking at the same class? :sweat_smile:

  /// renders the [sprite] as a Widget.
  ///
  /// To change the source size and position, see [Sprite.new]
  const SpriteWidget({
    required Sprite sprite,
    this.anchor = Anchor.topLeft,
    this.angle = 0,
    this.errorBuilder,
    this.loadingBuilder,
    super.key,
  }) : _spriteFuture = sprite;
erickzanardo commented 3 months ago

Oh, this was removed in the last version 😅

https://github.com/flame-engine/flame/commit/f49d24c02dd0d9b781926908bad1fb6dfcbda5f2