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

TileEngine sx/sy setter doesn't account for scaling #412

Open
snowfrogdev opened this issue Aug 22, 2024 · 0 comments
Open

TileEngine sx/sy setter doesn't account for scaling #412

snowfrogdev opened this issue Aug 22, 2024 · 0 comments
Labels
bug Bug fixes

Comments

@snowfrogdev
Copy link

snowfrogdev commented Aug 22, 2024

I tried changing the sx value on my TileEngine and I noticed that it wasn't working. I thought that was weird as the current value was 0 and I was just trying to increment it by 1. It would remain at 0.

A look at the setter made me understand the problem.

  set sx(value) {
    let max = Math.max(0, this.mapwidth - getCanvas().width);
    this._sx = clamp(0, max, value);
  }

  set sy(value) {
    let max = Math.max(0, this.mapheight - getCanvas().height);
    this._sy = clamp(0, max, value);
  }

It is comparing the mapwidth with the canvas' width. In my particular case the mapwidth value was 1024 and the canvas width was 1920. Which at face value seems right. That's like saying that the map width is contained within the canvas. But the thing is, it wasn't. For one very simple reason, I'm scaling using context.scale(scaleFactorX, scaleFacorY).

I would think that scaling like this is a pretty common thing to do. Is it not?

Anyway, I'm thinking that it would be useful to account for scaling in the sx and sy setters to make it possible for people who are doing it to use the TileEngine.

Modifying those setters like this should do the trick:

  set sx(value) {
    let scaleFactor = getContext().getTransform().a;
    let max = Math.max(0, this.mapwidth * scaleFactor - getCanvas().width);
    this._sx = clamp(0, max, value);
  }

  set sy(value) {
    let scaleFactor = getContext().getTransform().d;
    let max = Math.max(0, this.mapheight * scaleFactor - getCanvas().height);
    this._sy = clamp(0, max, value);
  }

I'd be happy to make a PR if you think this is a good idea.

@straker straker added the bug Bug fixes label Oct 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug fixes
Projects
None yet
Development

No branches or pull requests

2 participants