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

Resizing a NineSliceSprite after creation does not work properly when using Animated sprite #1115

Open
NemoStein opened this issue Aug 6, 2022 · 9 comments
Labels

Comments

@NemoStein
Copy link

OS platform / Browser

Windows 10, Chrome 105

melonJS version

v13.0.0

Bug description

The NineSliceSprite internal properties nss_width and nss_height never gets updated, making it impossible to change the NineSliceSprite size.

Setting the (undocumented) properties nss_width/nss_height instead of width/height seems to work, even if the sprite is animated.

I tried to patch the NineSliceSprite class with something like this:

get width() { return super.width; }
set width(value) {
    super.width = value
    this.nss_width = value;
}

But every frame calls the Sprite.setRegion method, which resets the width/height of the sprite

I think the easiest (but far from ideal) approach is a new method NineSliceSprite.changeSize(w, h) (or something like this)
NineSliceSprite.resize can be used, but resize() is already taken by Rect.
Furthermore, it would create even more confusion when trying to set the width/height properties directly (why resize() works but width/height doesn't?)

The ideal solution is to properly fix the get/set width/height behaviour.

Steps to reproduce the bug

// Create NineSliceSprite with size 32x32
const sprite = new NineSliceSprite(0, 0, {
  image: 'nineslice',
  width: 32,
  height: 32
})

// Change width/height
sprite.width = 128

// NineSliceSprite don't change size
game.world.addChild(sprite)
@obiot
Copy link
Member

obiot commented Aug 6, 2022

oh boy, I could swear this one was working ! I'll fix it anyway and add a corresponding test unit to prevent breaking it again if this is/was a regression. Thanks for reporting it 👍

@obiot
Copy link
Member

obiot commented Aug 7, 2022

actually just setting the private nss_width and nss_height to the proper value works, you can do that for now, and I'll see how to properly fix this, but I agree, most certainly through a setter/getter.

@obiot
Copy link
Member

obiot commented Aug 8, 2022

@NemoStein

adding the following getter/setter to the NineSliceSprite class seems to do the trick on my side :

    /**
     * width of the NineSliceSprite
     * @public
     * @type {number}
     * @name width
     */
    get width() {
        return super.width;
    }
    set width(value) {
        super.width = this.nss_width = value;
    }

    /**
     * height of the NineSliceSprite
     * @public
     * @type {number}
     * @name height
     */
    get height() {
        return super.height;
    }
    set height(value) {
        super.height = this.nss_height = value;
    }

however I do not have any animated example for dialog box, so if you could also kindly confirm it's working on your side it would be great ! if it works I'll push those changes to the master branch, if not can you maybe share a minimal example here I can reuse for debugging?

@obiot
Copy link
Member

obiot commented Aug 8, 2022

I committed the changes, maybe easier to pull the latest changes on your side :
b0ee2b2

let me know, once you confirm you are good with it I'll publish the 13.1 version.

thanks again!

@NemoStein
Copy link
Author

Sadly the fix wasn't enough.
Everytime the Sprite.setRegion() is called the sprite width/height is reset to the sprite original frame width/height

this.width = this.current.width = region.width;
this.height = this.current.height = region.height;

Here is a sample that reproduces the issue: https://github.com/NemoStein/melonjs-9slice-sample

@obiot
Copy link
Member

obiot commented Aug 9, 2022

oh I see...... honestly I don't know how to fix this one quickly... the thing is that the sprite resizing/scaling feature rely on Matrix transformation to display the final sprite "size" and if you look at the Sprite Class code we actually never change the size of the sprite. So I really need to think about this one...

a quick and dirty fix is certainly to override the update method, call the

update(dt) {
    // .... do your stuff
   var dirty = super.update(dt);
   if (dirty) {
        // most likely the sprite frame was changed
        // Change width/height
       sprite.width = sprite.nss_width = 128;
   }
   return dirty;
}

I'm afraid I have nothing better to offer for now, as I know you are on a tight schedule with the game jam !

@NemoStein
Copy link
Author

Don't worry, I'll keep using the nss_* workaround for now...

It's just a progress loading bar, after all! ;D

@obiot
Copy link
Member

obiot commented Aug 9, 2022

ok, I'll keep it like this for now and go on with the 13.1 release. I will see later how we can properly implement this.

I must admit though I never thought about using a 9-slice sprite for a loading bar :)

@obiot obiot changed the title NineSliceSprite never update it's width/height Resizing a NineSliceSprite after creation/instantiation does not work properly when using Animated sprite Aug 9, 2022
@obiot obiot changed the title Resizing a NineSliceSprite after creation/instantiation does not work properly when using Animated sprite Resizing a NineSliceSprite after creation does not work properly when using Animated sprite Aug 9, 2022
@obiot
Copy link
Member

obiot commented Aug 10, 2022

13.1.0 was just released ! Followed by a quick 13.1.1 fix but just for date and change tracking :)

@obiot obiot added the Bug label Aug 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants