phaser icon indicating copy to clipboard operation
phaser copied to clipboard

GridAlign() origin seems incorrect

Open samme opened this issue 3 years ago • 1 comments

Version

  • Phaser Version: 3.52.0 (and earlier)

Description

Call Phaser.Actions.GridAlign() with x and y options.

Optionally place the top-left of the final grid at this coordinate.

I expected this puts the top-left of the first grid cell at (x, y). Instead it puts the center of the first grid cell at (x, y).

Example Test Code

https://codepen.io/samme/pen/bGBbExE

This is a GridAlign() call with { x: 0, y: 0 }. The top-left of the first cell is (-32, -32), and its center is (0, 0).

Results of GridAlign() with origin (0, 0)

samme avatar Jan 28 '21 01:01 samme

Yep. This is due to the fact that "tempZone" in https://github.com/photonstorm/phaser/blob/master/src/actions/GridAlign.js has default origin as 0.5 and is unchangeable through the params. It seems like this could be easily fixed by adding a field to the params to take in a desired origin for the tempZone.

I don't have time to fork the repo, rebuild and reroute my dependencies but I was able to fix the issue by copy pasting the contents of GridAlign.js into a new file with "setOrigin(0)" on the tempZone as a quick and dirty solution. Maybe someone else can make a clean solution for this but here is what worked for me:

import { Scene } from "phaser";

var NOOP = function ()
{
    //  NOOP
};

 var GetFastValue = function (source, key, defaultValue)
 {
     var t = typeof(source);
 
     if (!source || t === 'number' || t === 'string')
     {
         return defaultValue;
     }
     else if (source.hasOwnProperty(key) && source[key] !== undefined)
     {
         return source[key];
     }
     else
     {
         return defaultValue;
     }
 };

 
 var SetLeft = function (gameObject, value)
 {
     gameObject.x = value + (gameObject.width * gameObject.originX);
 
     return gameObject;
 };

 var SetTop = function (gameObject, value)
{
    gameObject.y = value + (gameObject.height * gameObject.originY);

    return gameObject;
};

var GetLeft = function (gameObject)
{
    return gameObject.x - (gameObject.width * gameObject.originX);
};

var GetTop = function (gameObject)
{
    return gameObject.y - (gameObject.height * gameObject.originY);
};

 var TopLeft = function (gameObject, alignIn, offsetX, offsetY)
{
    if (offsetX === undefined) { offsetX = 0; }
    if (offsetY === undefined) { offsetY = 0; }

    SetLeft(gameObject, GetLeft(alignIn) - offsetX);
    SetTop(gameObject, GetTop(alignIn) - offsetY);

    return gameObject;
};

 
 var tempZone = new Phaser.GameObjects.Zone({ sys: { queueDepthSort: NOOP, events: { once: NOOP } } } as unknown as Scene, 0, 0, 1, 1).setOrigin(0);
 

 export const FixedGridAlign = function (items, options)
 {
     if (options === undefined) { options = {}; }
 
     var widthSet = options.hasOwnProperty('width');
     var heightSet = options.hasOwnProperty('height');
 
     var width = GetFastValue(options, 'width', -1);
     var height = GetFastValue(options, 'height', -1);
 
     var cellWidth = GetFastValue(options, 'cellWidth', 1);
     var cellHeight = GetFastValue(options, 'cellHeight', cellWidth);
 
     var x = GetFastValue(options, 'x', 0);
     var y = GetFastValue(options, 'y', 0);
 
     var cx = 0;
     var cy = 0;
     var w = (width * cellWidth);
     var h = (height * cellHeight);
 
     tempZone.setPosition(x, y);
     tempZone.setSize(cellWidth, cellHeight);
 
     for (var i = 0; i < items.length; i++)
     {
        TopLeft(items[i], tempZone, 0, 0);
 
         if (widthSet && width === -1)
         {
             //  We keep laying them out horizontally until we've done them all
             tempZone.x += cellWidth;
         }
         else if (heightSet && height === -1)
         {
             //  We keep laying them out vertically until we've done them all
             tempZone.y += cellHeight;
         }
         else if (heightSet && !widthSet)
         {
             //  We keep laying them out until we hit the column limit
             cy += cellHeight;
             tempZone.y += cellHeight;
 
             if (cy === h)
             {
                 cy = 0;
                 cx += cellWidth;
                 tempZone.y = y;
                 tempZone.x += cellWidth;
 
                 if (cx === w)
                 {
                     //  We've hit the column limit, so return, even if there are items left
                     break;
                 }
             }
         }
         else
         {
             //  We keep laying them out until we hit the column limit
             cx += cellWidth;
             tempZone.x += cellWidth;
 
             if (cx === w)
             {
                 cx = 0;
                 cy += cellHeight;
                 tempZone.x = x;
                 tempZone.y += cellHeight;
 
                 if (cy === h)
                 {
                     //  We've hit the column limit, so return, even if there are items left
                     break;
                 }
             }
         }
     }
 
     return items;
 };

Phaser is massively infested with these types of origin issues which cause insidious misalignments which can take hours to fix. I'm not sure why it is assumed that everyone loves using origin = 0.5 enough that it is the unchangable default on all of the internal code. I never, ever want to use origin = 0.5 on any of my game objects because it always requires more code to align everything correctly. Origin = 0 allows things to be aligned through simple addition, but origin = 0.5 requires many extra width calculations and mental gymnastics to conceptualize the positioning. Frankly I'm using Phaser over raw Pixi.js because I thought common requirements like positioning would be easy to implement, but I find myself having to patch a lot of the internal code due to origin issues.

shealyrd avatar Aug 17 '22 22:08 shealyrd

The tempZone origin has been changed to 0,0 in the master branch for Beta 13.

photonstorm avatar Oct 19 '22 20:10 photonstorm