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

Add hitbox to PositionComponent #618

Merged
merged 28 commits into from
Jan 20, 2021
Merged

Add hitbox to PositionComponent #618

merged 28 commits into from
Jan 20, 2021

Conversation

spydon
Copy link
Member

@spydon spydon commented Jan 10, 2021

Description

Adds the possibility to define a hitbox for a PositionComponent, which makes gestures more accurate.

In a follow up pull request I will add the possibility for collision detection between PositionComponents and other PositionComponents, and walls (the screen etc).

Accurate tap detection

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

Checklist:

If something is unclear, please submit the PR anyways and ask about what you thought was unclear.

  • This branch is based on the latest master
  • I have added an entry under [next] in CHANGELOG.md
  • I have formatted my code with flutter format
  • I have made corresponding changes to the documentation
  • I have added examples for new features in doc/examples
  • The continuous integration (CI) is passing

Copy link
Contributor

@wolfenrain wolfenrain left a comment

Choose a reason for hiding this comment

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

LGTM, just few nits :-)

doc/examples/gestures/lib/main_tapables_hitbox.dart Outdated Show resolved Hide resolved
lib/components/mixins/has_hitbox.dart Outdated Show resolved Hide resolved
lib/components/mixins/has_hitbox.dart Outdated Show resolved Hide resolved
lib/components/mixins/has_hitbox.dart Outdated Show resolved Hide resolved
lib/components/mixins/has_hitbox.dart Outdated Show resolved Hide resolved
lib/components/mixins/has_hitbox.dart Outdated Show resolved Hide resolved
lib/components/mixins/has_hitbox.dart Outdated Show resolved Hide resolved
lib/components/position_component.dart Outdated Show resolved Hide resolved
@spydon spydon changed the title Add hull to PositionComponent Add hitbox to PositionComponent Jan 12, 2021
Copy link
Member

@erickzanardo erickzanardo left a comment

Choose a reason for hiding this comment

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

Two nits, but LGTM already

lib/collision_detection.dart Outdated Show resolved Hide resolved
Copy link
Member

@renancaraujo renancaraujo left a comment

Choose a reason for hiding this comment

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

Awesome job!


/// Checks whether the [polygon] represented by the list of [Vector2] contains
/// the [point].
bool containsPoint(Vector2 point, List<Vector2> polygon) {
Copy link
Member

Choose a reason for hiding this comment

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

@spydon I think I have some of these methods on flame_geom, have you taken a look at that?

Copy link
Member Author

Choose a reason for hiding this comment

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

I only think flame_geom can handle basic shapes and it isn't migrated to Vector2 yet, I can have a look at migrating it once I'm done with the follow up collision detection PR that is coming after this one.
And if we do the migration, we should really rename flame_geom, I really dislike abbreviations in package names.
😅

}

// Rotates the [point] with [angle] around [position]
Vector2 rotatePoint(Vector2 point, double angle, Vector2 position) {
Copy link
Member

Choose a reason for hiding this comment

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

The old Position class had this method, don't we have an extension function on Vector2 (or something native) to do this? if so we should add instead of an utils

Copy link
Member Author

Choose a reason for hiding this comment

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

Makes sense, migrated this to the Vector2 extension.

Comment on lines 112 to 120
final hitboxPath = Path()
..addPolygon(
(this as Hitbox)
.scaledShape
.map((point) => (point + size / 2).toOffset())
.toList(),
true,
);
canvas.drawPath(hitboxPath, debugPaint);
Copy link
Member

Choose a reason for hiding this comment

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

this code should be within hitbox:

if (this is Hitbox) {
  this.renderContour(canvas);
}

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

import '../../extensions/vector2.dart';

mixin Hitbox on PositionComponent {
List<Vector2> _shape;
Copy link
Member

Choose a reason for hiding this comment

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

I think this should be an Overlapable.
Then you can create a Polygon class that extends Overlapable and has a List
We will need to implement some more methods but that allows you to have more complex shapes (circles, unions) and allows you to check overlap between two shapes. containsPoint is useful but it would also be useful to inherit the overlaps method.
We can also rename Overlapable to Shape if that makes more sense

Copy link
Member Author

Choose a reason for hiding this comment

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

Makes sense, I'll do the migration and fixes of flame_geom in the follow up branch that already has a lot of code so that the merge conflict isn't that massive. And then I'll point that PR towards this branch.

@spydon spydon force-pushed the spydon.add-hull-capabilities branch 2 times, most recently from ba01fd2 to 3bb3c2b Compare January 18, 2021 21:15
@spydon spydon force-pushed the spydon.add-hull-capabilities branch from 3bb3c2b to d193c93 Compare January 20, 2021 22:11
@spydon spydon merged commit 2b873ee into master Jan 20, 2021
@spydon spydon deleted the spydon.add-hull-capabilities branch January 20, 2021 22:39
@callagga
Copy link

callagga commented Feb 10, 2021

Is the Hitbox mixin in RC6? Just trying to see an example of how to add a hitbox area to a PositionalComponent?

@spydon
Copy link
Member Author

spydon commented Feb 10, 2021

@callagga yes, this is in rc6.
Just be aware that there will be some changes to it in rc7, and collision detection added.
Here is an example of how to use it in rc6:
https://github.com/flame-engine/flame/blob/master/doc/examples/gestures/lib/main_tapables_hitbox.dart

@spydon
Copy link
Member Author

spydon commented Feb 10, 2021

Sorry, for some reason it is in the changelog, but not in the code.
It will be in rc7 though.

@erickzanardo
Copy link
Member

Sorry, for some reason it is in the changelog, but not in the code.

It will be in rc7 though.

We should fix that in the chancelog for the next version, can you fix it on your PR about the collision detection @spydon ?

@spydon
Copy link
Member Author

spydon commented Feb 10, 2021

Sorry, for some reason it is in the changelog, but not in the code.
It will be in rc7 though.

We should fix that in the chancelog for the next version, can you fix it on your PR about the collision detection @spydon ?

Will do!

spydon added a commit that referenced this pull request Feb 24, 2021
* Update README to point at rc5 (#630)

* Update README to point at rc5

* Update README.md

* 👌 Throw exception from `parseAnchor` in `examples/widgets` instead of returning null (#632)

* Updated the widgets docs (#628)

Co-authored-by: Erick <erickzanardoo@gmail.com>

* Removing initialDimensions and removeGesture methods (#627)

* Removing initialDimensions and removeGesture methods

* Removing wrongly commited folder

* PR suggestion

* Making SpriteComponent and SpriteAnimationComponent follow the same standard for empty constructors (#620)

* Added fallback support for the web on the `SpriteBatch` API (#612)

* Added fallback support for the web on the `SpriteBatch` API

* Refactored the SpriteBatch class

* 👌 Use `Offset` type directly in `JoystickAction.update` calculations (#631)

* Move files to src and comply with the dart package layout convention (#621)

* 👌 Use `Offset` type directly in `JoystickAction.update` calculations (#631)

* Move files to src and comply with the dart package layout convention

* Fixing widgets example

Co-authored-by: Serge Matveenko <lig@countzero.co>
Co-authored-by: Erick Zanardo <erickzanardoo@gmail.com>

* Fix gesture detection on children (#636)

* Fix gesture detection on children

* Remove unused imports

* positionParent should be private

* Fixed comment

* Moved parent to BaseComponent

* Fix formatting

* Initial implementation of the Composition class. (#634)

* Initial implementation of the Composition class

* Added decodeImageFromPixels web support

* Added a `required` to GameWidget and updated the docs (#638)

* Added a required to GameWidget and updated the docs

* Removed entry from CHANGELOG

* Release v1.0.0-rc6 (#639)

* Add hitbox to PositionComponent (#618)

* Move out collision detection methods

* Add possibility to define a hull for PositionComponents

* Add example of how to use hull with tapable

* Update contains point comment

* Fix contains point

* Hull should be based on center position

* Remove collision detection parts

* Added tests

* Use percentage of size instead of absolute size

* Separate hull from PositionComponent

* Clarify hull example

* Fix formatting

* Override correct method

* Use mixin for hitbox

* Update changelog

* Rename HasHitbox to Hitbox

* Clarified names

* Center to edge is considered as 1.0

* Fix test

* Add spaces within braces

* Removed extra spaces in the braces

* Add hitbox docs

* Fix link

* Moved point rotation to Vector2 extension

* Render hitbox within extension

* Fix rebase

* Fix rebase

* Fix formatting

* Removing Util.dart and moving its remaining parts to better places (#640)

* Removing Util.dart and moving its remaining parts to better places

* Fixing lint

* Doc fixes

* Doc fixes

* PR suggestions

* Adapting SpriteBatchComponent constructors to other components (#643)

* Adapting ParallaxComponent constructors to other components (#642)

* Adapating ParallaxComponent constructors to other components

* Removing wrongly commited folder

* Update doc/components.md

Co-authored-by: Jochum van der Ploeg <jochum@vdploeg.net>

* Update doc/components.md

Co-authored-by: Jochum van der Ploeg <jochum@vdploeg.net>

Co-authored-by: Jochum van der Ploeg <jochum@vdploeg.net>

* Updating mds with recently 0.x patches

* 🏷 Null safety support

Co-authored-by: Lukas Klingsbo <lukas.klingsbo@gmail.com>
Co-authored-by: Jochum van der Ploeg <jochum@vdploeg.net>
Co-authored-by: Erick <erickzanardoo@gmail.com>
Co-authored-by: Renan <6718144+renancaraujo@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants