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

swap translate <--> scale transforms #385

Merged
merged 3 commits into from
Sep 4, 2024
Merged

Conversation

dragoncoder047
Copy link
Contributor

ref: #381 (comment)

@mflerackers : "translate and scale transforms were done in the wrong order before drawing for some reason. It's an easy fix..."

is this the fix?

closes #381 if it is

Copy link

pkg-pr-new bot commented Sep 3, 2024

Open in Stackblitz

pnpm add https://pkg.pr.new/kaplayjs/kaplay@385

commit: 1a8b712

@mflerackers
Copy link
Member

No, certainly not. That would completely break collision detection itself.

@dragoncoder047
Copy link
Contributor Author

No, certainly not. That would completely break collision detection itself.

So, where is the "out-of-order" transformations at?

@dragoncoder047
Copy link
Contributor Author

This PR is empty now, but if I find the out-of order transformations I'll fix it here

@mflerackers
Copy link
Member

Possibly here, applying the scale is done before offset. But you shouldn't only change the code, but test it as well.

worldArea(this: GameObj<AreaComp | AnchorComp>): Polygon {
const localArea = this.localArea();

        const transform = this.transform
            .clone()
            .scale(vec2(this.area.scale ?? 1))
            .translate(this.area.offset);

@dragoncoder047
Copy link
Contributor Author

dragoncoder047 commented Sep 3, 2024

ok I tried it there

how do I re-trigger the pkg.pr.new thing so I can so I can just npm install my changes to test them? never mind, I noticed the bot updates its own comment automatically

@mflerackers
Copy link
Member

Didn't it update itself? I think it did.

@dragoncoder047
Copy link
Contributor Author

Didn't it update itself? I think it did.

Yeah it did (I noticed like 30 seconds after I added that comment, see the edit)

But you shouldn't only change the code, but test it as well.

I installed it and tested, it seems to work for me.

Copy link
Member

@mflerackers mflerackers left a comment

Choose a reason for hiding this comment

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

👍

@lajbel lajbel added the v3001&v4000 A change for stable and compatibility layer label Sep 4, 2024
@lajbel lajbel merged commit bd4b9b8 into kaplayjs:master Sep 4, 2024
2 checks passed
@dragoncoder047 dragoncoder047 deleted the patch-1 branch September 4, 2024 13:51
lajbel added a commit that referenced this pull request Sep 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v3001&v4000 A change for stable and compatibility layer
Projects
None yet
3 participants