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 layer hit detection and event handling #15

Merged
merged 22 commits into from
Feb 4, 2023
Merged

Add layer hit detection and event handling #15

merged 22 commits into from
Feb 4, 2023

Conversation

dnass
Copy link
Owner

@dnass dnass commented Feb 4, 2023

No description provided.

Copy link
Contributor

@LeoDog896 LeoDog896 left a comment

Choose a reason for hiding this comment

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

I do like the idea of using a second canvas to detect collisions and it should cover most cases. I'm sure a few edge cases will pop up as issues (for both the color detection && the Proxy handler, but I can't think of any as of now.

I'm unfamiliar with using Proxy so I'm not able to do too thorough of a review there.

If I were to avoid the Proxy usage, I would make a new object that spreads the functions of the current canvas and overrides any that I personally need, while keeping the same signature as the 2dContext, though that itself is also probably a lot of work (and maybe justification for a second PR)

Also, you should add the examples to the list in the main page.

src/lib/components/layerEvent.ts Outdated Show resolved Hide resolved
src/lib/util/LayerManager.ts Outdated Show resolved Hide resolved
src/lib/util/LayerManager.ts Outdated Show resolved Hide resolved
@LeoDog896
Copy link
Contributor

LeoDog896 commented Feb 4, 2023

By the way, in regards to your Proxy type issue, it seems that Proxy just doesn't have the capabilities for this yet in TS.

Also, probably best to update the README with extra info for the API as well.

@dnass
Copy link
Owner Author

dnass commented Feb 4, 2023

@LeoDog896 Thanks for your notes — made some changes to the touch handler. Other than the lack of proper TS support, the approach I'm using here feels to me like a good use case for Proxies so I'm going to stick with it for now unless I discover any major roadblocks. From the POV of library users I don't think it'll make much different. Will update the example page and readme to document the new features.

@dnass dnass merged commit 6f1ba87 into master Feb 4, 2023
@dnass dnass deleted the layer-events branch February 4, 2023 23:06
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