Skip to content

refactor: Convert hover handler to a system#986

Merged
lajbel merged 4 commits intomasterfrom
hover_system
Dec 30, 2025
Merged

refactor: Convert hover handler to a system#986
lajbel merged 4 commits intomasterfrom
hover_system

Conversation

@mflerackers
Copy link
Member

Please describe what issue(s) this PR fixes.

Summary

  • Changeloged

@mflerackers mflerackers self-assigned this Dec 30, 2025
@mflerackers mflerackers requested a review from lajbel December 30, 2025 00:12
@mflerackers mflerackers added the enhancement New feature or request label Dec 30, 2025

system("hover", () => {
if (_k.game.fakeMouse) {
mouseHover(toWorld(_k.game.fakeMouse.pos));
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be fakeMouseHover or is it intentionally wrong

Also, return if fake mouse exists?

Copy link
Member Author

Choose a reason for hiding this comment

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

right

Copy link
Member Author

Choose a reason for hiding this comment

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

Can't you use fakeMouse and Mouse together?

Copy link
Member Author

Choose a reason for hiding this comment

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

They have their own closure because I thought they can be used simultaneously.

Copy link
Contributor

Choose a reason for hiding this comment

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

Idk, I think the intention was that if the game used fake mouse then the fake mouse object will move to always follow the real mouse, so the hover handler should not run twice in that case.

Also, the hover handler is given the world position, and toWorld converts from screen space to world space, but the fakeMouse.pos is already world space right? At least it won't be screen space if the camera transform is not the identity

Copy link
Contributor

Choose a reason for hiding this comment

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

It should

Copy link
Member Author

@mflerackers mflerackers Dec 30, 2025

Choose a reason for hiding this comment

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

update(this: FakeMouse) {
            if (this.pos.x !== lastPos.x || this.pos.y !== lastPos.y) {
                this.trigger("fakeMouseMove", this.pos.clone()); // TODO: Create a deltaPos
            }

It doesn't pass world coordinates to its trigger, so if it is not in root, it's not working as expected.

Copy link
Contributor

@dragoncoder047 dragoncoder047 Dec 30, 2025

Choose a reason for hiding this comment

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

The fakeMouseMove event is new, not surprising that it's wrong in places, but also good cause it's unlikely someone is using this extremely unfinished branch just for that event

Copy link
Member Author

Choose a reason for hiding this comment

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

It's not in this branch, it's been there since fakemouse.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's not in this branch, it's been there since fakemouse.

@pkg-pr-new
Copy link

pkg-pr-new bot commented Dec 30, 2025

Open in StackBlitz

npm i https://pkg.pr.new/kaplayjs/kaplay@986

commit: 4a3c1ac

@lajbel lajbel changed the title Convert hover handler to a system refactor: Convert hover handler to a system Dec 30, 2025
@lajbel lajbel merged commit 47358ec into master Dec 30, 2025
7 checks passed
@lajbel lajbel deleted the hover_system branch December 30, 2025 01:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants