melonjs / melonJS

a fresh, modern & lightweight HTML5 game engine
https://melonjs.org
MIT License
5.93k stars 643 forks source link

NineSliceSprite has no settings to indicate inset dimensions #1084

Closed dynamo-foundation closed 2 years ago

dynamo-foundation commented 2 years ago

If you are submitting a bug report, please respect the below template :

OS platform / Browser

Any

melonJS version

10

Bug description

NineSliceSprite is the same as Sprite - there are not settings to indicate where the inset box is that delineates where scaling should occur

Steps to reproduce the bug

View documentation for NineSliceSprite or create a new me.NineSliceSprite object and log object to console - no settings available to create inset

Console log Extract

to show possible exception/errors raised by the engine or the browser

How to enable/use the console log
dynamo-foundation commented 2 years ago

see lines 81 to 83 of source, this is basically useless without being configurable - no two 9 patch are going to be the same and very few are 1/4

    // should this be configurable ?
    var corner_width = frame.width / 4,
        corner_height = frame.height / 4;
obiot commented 2 years ago

indeed we are missing an argument that defines the size of the "corner" size (and by extension left/right & top/bottom part), thanks for the reminder !

obiot commented 2 years ago

yup ! thanks, will add it to the 10.4, I need to release anyway, it as been too long since the last version :)

obiot commented 2 years ago

I will not make insety and insetx mandatory though, but rather give them a default value like 16 I guess (what value do you use ?)

[EDIT] or just keep the frame width & height value divided by 4 actually, if no settings are being given

dynamo-foundation commented 2 years ago

actually this is not working at all - the image is not resized despite changing the "width" and "height" values

debugging

obiot commented 2 years ago

be careful that in the line you copy/paste here you misspell inset with inseRt !

dynamo-foundation commented 2 years ago

line 24 and 25 have an error - the code is re-using existing width and height local variables - these need to be named something else

In my instance I changed them to virtual_width and virtual_height

the code below works as intended - what is in the repo does not change the size of the image regardless of width and height values used at construction

this code is testing/working:

import * as me from 'melonjs/dist/melonjs.module.js';

class NinePatchSprite extends me.Sprite {

/**
 * @ignore
 */
constructor(x, y, settings) {
    // call the super constructor
    super(x, y, settings);

    // ensure mandatory properties are defined
    if ((typeof settings.width !== "number") || (typeof settings.height !== "number")) {
        throw new Error("height and width properties are mandatory");
    }

    if ((typeof settings.insetx !== "number") || (typeof settings.insety !== "number")) {
        throw new Error("insetx and insety properties are mandatory");
    }

    // override the renderable sprite with the given one
    // resize based on the active frame
    this.virtual_width = settings.width;
    this.virtual_height = settings.height;
    this.insetx = settings.insetx;
    this.insety = settings.insety;

}

/**
 * @ignore
 */
draw(renderer) {
    // the frame to draw
    var frame = this.current;

    // cache the current position and size
    var dx = this.pos.x,
        dy = this.pos.y;

    var w = frame.width,
        h = frame.height;

    // frame offset in the texture/atlas
    var frame_offset = frame.offset;
    var g_offset = this.offset;

    // remove image's TexturePacker/ShoeBox rotation
    if (frame.angle !== 0) {
        renderer.translate(-dx, -dy);
        renderer.rotate(frame.angle);
        dx -= h;
        w = frame.height;
        h = frame.width;
    }

    var sx = g_offset.x + frame_offset.x,
        sy = g_offset.y + frame_offset.y;

    // should this be configurable ?
    var corner_width = this.insetx;  //frame.width / 4,
    var corner_height = this.insety;  //frame.height / 4;

    // OPTIMIZE ME !

    // DRAW CORNERS

    // Top Left
    renderer.drawImage(
        this.image,
        sx,                          // sx
        sy,                          // sy
        corner_width, corner_height, // sw,sh
        dx, dy,                      // dx,dy
        corner_width, corner_height  // dw,dh
    );

    // Top Right
    renderer.drawImage(
        this.image,
        sx + w - corner_width,          // sx
        sy,                             // sy
        corner_width, corner_height,    // sw,sh
        dx + this.virtual_width - corner_width, // dx
        dy,                             // dy
        corner_width, corner_height     // dw,dh
    );
    // Bottom Left
    renderer.drawImage(
        this.image,
        sx,                                 // sx
        sy + h - corner_height,             // sy
        corner_width, corner_height,        // sw,sh
        dx,                                 // dx
        dy + this.virtual_height - corner_height,   // dy
        corner_width, corner_height         // dw,dh
    );
    // Bottom Right
    renderer.drawImage(
        this.image,
        sx + w - corner_width,              // sx
        sy + h - corner_height,             // sy
        corner_width, corner_height,        // sw,sh
        dx + this.virtual_width - corner_width,     //dx
        dy + this.virtual_height - corner_height,   // dy
        corner_width, corner_height         // dw,dh
    );

    // DRAW SIDES and CENTER
    var image_center_width = w - (corner_width << 1);
    var image_center_height = h - (corner_height << 1);

    var target_center_width = this.virtual_width - (corner_width << 1);
    var target_center_height = this.virtual_height - (corner_height << 1);

    //Top center
    renderer.drawImage(
        this.image,
        sx + corner_width,         // sx
        sy,                        // sy
        image_center_width,        // sw
        corner_height,             // sh
        dx + corner_width,         // dx
        dy,                        // dy
        target_center_width,       // dw
        corner_height              // dh
    );

    //Bottom center
    renderer.drawImage(
        this.image,
        sx + corner_width,                  // sx
        sy + h - corner_height,             // sy
        image_center_width,                 // sw
        corner_height,                      // sh
        dx + corner_width,                  // dx
        dy + this.virtual_height - corner_height,   // dx
        target_center_width,                // dw
        corner_height                       // dh
    );

    // Middle Left
    renderer.drawImage(
        this.image,
        sx,                   // sx
        sy + corner_height,   // sy
        corner_width,         // sw
        image_center_height,  // sh
        dx,                   // dx
        dy + corner_height,   // dy
        corner_width,         // dw
        target_center_height  // dh
    );

    // Middle Right
    renderer.drawImage(
        this.image,
        sx + w - corner_width,          // sx
        sy + corner_height,             // sy
        corner_width,                   // sw
        image_center_height,            // sh
        dx + this.virtual_width - corner_width, // dx
        dy + corner_height,             // dy
        corner_width,                   // dw
        target_center_height            // dh
    );

    // Middle Center
    renderer.drawImage(
        this.image,
        sx + corner_width,    // sx
        sy + corner_height,   // sy
        image_center_width,   // sw
        image_center_height,  // sh
        dx + corner_width,    // dx
        dy + corner_height,   // dy
        target_center_width,  // dw
        target_center_height  // dh
    );
}

};

export default NinePatchSprite;

obiot commented 2 years ago

but wait, it should not matter, because this.width and this.height are anyway not modified after being set in the constructor.

dynamo-foundation commented 2 years ago

I dunno what to say - if you console.log width and height after construction they revert to the original image size and the image is not changed when you alter them. Code in the repo does not stretch the image, regardless of width and height at time of construction - it's easy to test:

    var x = {};
    x.image = "ui64";
    x.height = 60;      //CHANGE THIS - NO CHANGE IN IMAGE
    x.width = 400;     //CHANGE THIS - NO CHANGE IN IMAGE
    x.insetx = 16;
    x.insety = 16;
    x.framewidth = 64;
    x.frameheight = 64;
    var s = new NinePatchSprite(100,100,x);    //USE ORIGINAL NINE SLICE OBJECT 
    s.addAnimation("x",[7]);
    s.setCurrentAnimation("x");

    me.game.world.addChild(s);
obiot commented 2 years ago

there it is, it's basically the same code as above, give or take minor changes, so if you could just confirm it's also working on your side, I'll then close this ticket. thanks again!

dynamo-foundation commented 2 years ago

One small issue - after I laid out a few screens I noticed that sometimes a width or height with a non integer value could occasionally cause a horizontal or vertical line to appear due to rounding (the right side subtracts, where the middle adds). Technically it probably should build the 9 patch from left to right, but with the code as-is you can add Math.floor and it seems to work ok:

lines 47 and 48 in your revised code would be: this.virtual_width = Math.floor(settings.width); this.virtual_height = Math.floor(settings.height);

other than that seems to be working good

I ended up copying gui_object and extended it with the above code as well - things like textboxes and buttons really need 9 patch - ideally those GUI elements should extend the 9 patch sprite instead of just me.sprite to be truly useful and I dont think that would be a breaking change from a regression standpoint, although dont quote me on that :)

obiot commented 2 years ago

good point actually for GUI Object extending NineSliceSprite object, but it's just then I would not want to generalise it to all GUI Object, as NineSliceSprite are generating nine draw calls, versus just one for Sprite....