-
-
Notifications
You must be signed in to change notification settings - Fork 53
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
refactor: internal getKaboomCtx
and getInternalCtx
, modulizate components
#54
Conversation
src/kaboom.ts
Outdated
|
||
// TODO: A better way to detect KaboomCtx | ||
export const isKaboomCtx = (obj: any): obj is KaboomCtx => { | ||
return Object.keys(obj).includes("loadAseprite"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line doesn't look very nice. It can be tiring to convert the object to an array and check the keys one by one each time. If a component is added and removed frequently, this process will be repeated each time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, maybe only check obj["loadAseprite"]
Is this what you mean by context? const k = kaboom({ global: false }) // One context.
const a = kaboom({ global: false }) // Another context. |
Yes! |
I changed more things, adding In return, it is not necessary to use Array.find to get the internal api to be used in other files. I don't know if it's a good thing to export the internal api, I'm thinking about the possibilities for plugin creators. But beyond that, does anyone have an opinion? |
Good work, it looks better now. I have no idea at the moment what to do to make it better. Maybe this pull request can stay a little more open. |
getKaboomCtx
and getInternalCtx
getKaboomCtx
and getInternalCtx
, modulizate components
Okay, this is my new modulization idea
Because some components have to access to internal api (
app
,gfx
) etc, I propose the followinggetKaboomCtx() gets the context from the global scope (the defined
ctx
variable) inkaboom.ts
, with also the option of get the context fromthis
.For example, in a component
getInternal
should work for the same, but the internal apis are not exported to any place, so it works as an array of internal contexts linked to the kaboom context, and when you usegetInternalCtx
, it finds the element with the contextLet me know what do you think, for now I was trying to mantain compatibility with
multipleboom
example, so I think there's no breaking changes.