Skip to content

Fix an issue where hitboxes didn't work on FlxEffectSprite#483

Open
ADA-Funni wants to merge 2 commits into
HaxeFlixel:devfrom
ADA-Funni:ada-funni/flxeffectsprite-temp-fix
Open

Fix an issue where hitboxes didn't work on FlxEffectSprite#483
ADA-Funni wants to merge 2 commits into
HaxeFlixel:devfrom
ADA-Funni:ada-funni/flxeffectsprite-temp-fix

Conversation

@ADA-Funni

Copy link
Copy Markdown

No description provided.

@Geokureli Geokureli left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

All PRs should include a small code example that reproduces the issue, and explains the expected behavior vs the actual behavior

Also you need to match the flixel's code style, curly brackets go on a new line

super.draw();
}

override public function updateHitbox() {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Don't cuddle brackets

@ADA-Funni ADA-Funni May 11, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

It's fixed now, sorry.
I also defined it as Void because all the other functions do that

@ADA-Funni

Copy link
Copy Markdown
Author

All PRs should include a small code example that reproduces the issue, and explains the expected behavior vs the actual behavior

Here's a couple diagrams of the behaviour

Before my PR:
image

After my PR:
image

@ADA-Funni

Copy link
Copy Markdown
Author

The new behaviour (shown below) makes the calculations from updateHitbox() work in terms of the target's height/width, whereas the original used the effect sprite's height/width, which got really weird due to the hitbox code staying untouched.
image

The old behaviour was just the regular updateHitbox() function applying to the effect sprite

@Geokureli

Copy link
Copy Markdown
Member

Could we solve this by setting frameWidth and frameHeight to the target's frame sizes? I'm pretty sure there are other methods that use this.frameWidth and will also need updating

@ADA-Funni

Copy link
Copy Markdown
Author

Could we solve this by setting frameWidth and frameHeight to the target's frame sizes? I'm pretty sure there are other methods that use this.frameWidth and will also need updating

I think that could work?
I'll try that later, thank you. :D

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.

2 participants