TextStyle.setStyle mutates the given style parameter object

[stormpanda]

Version

• Phaser Version: 3.80.1
• Operating system: MacOS
• Browser: all browsers

Description

https://github.com/phaserjs/phaser/blob/master/src/gameobjects/text/TextStyle.js

TextStyle.setStyle normalizes the given fontSize by adding "px" if it is just a number. However, the function will mutate the original given "style" parameter, which is a counterintuitive side-effect that is also unnecessary. This makes hard to define global font styles for reuse, since after the first setStyle call a font size of for example 32 (number) will be "32px" (string) afterwards, meaning it cannot be used for calculation of relative font sizes anymore.

The "style" parameter should not be mutated. Cloning it before changing it would fix the problem, I guess a shallow copy would also suffice.

Example Test Code

https://phaser.io/sandbox/zvb8G5ux

[greptile-apps]

For internal use only

To resolve the issue of TextStyle.setStyle mutating the given style parameter object, you can clone the style object before making any modifications. Here are the specific steps to implement this:

1. Import the Clone utility at the top of TextStyle.js:

var Clone = require('../../utils/object/Clone');

2. Clone the style object at the beginning of the setStyle method:

setStyle: function (style, updateText, setDefaults) {
    if (updateText === undefined) { updateText = true; }
    if (setDefaults === undefined) { setDefaults = false; }

    // Clone the style object to avoid mutation
    style = Clone(style);

    // Avoid type mutation
    if (style && style.hasOwnProperty('fontSize') && typeof style.fontSize === 'number') {
        style.fontSize = style.fontSize.toString() + 'px';
    }

    for (var key in propertyMap) {
        var value = (setDefaults) ? propertyMap[key][1] : this[key];

        if (key === 'wordWrapCallback' || key === 'wordWrapCallbackScope') {
            this[key] = GetValue(style, propertyMap[key][0], value);
        } else {
            this[key] = GetAdvancedValue(style, propertyMap[key][0], value);
        }
    }

    var font = GetValue(style, 'font', null);

    if (font !== null) {
        this.setFont(font, false);
    }

    this._font = [ this.fontStyle, this.fontSize, this.fontFamily ].join(' ').trim();

    var fill = GetValue(style, 'fill', null);

    if (fill !== null) {
        this.color = fill;
    }

    var metrics = GetValue(style, 'metrics', false);

    if (metrics) {
        this.metrics = {
            ascent: GetValue(metrics, 'ascent', 0),
            descent: GetValue(metrics, 'descent', 0),
            fontSize: GetValue(metrics, 'fontSize', 0)
        };
    } else if (updateText || !this.metrics) {
        this.metrics = MeasureText(this);
    }

    if (updateText) {
        return this.parent.updateText();
    } else {
        return this.parent;
    }
}

This change ensures that the original style object is not mutated.

References

/src/gameobjects/text/TextStyle.js

About Greptile

This response provides a starting point for your research, not a precise solution.

Help us improve! Please leave a 👍 if this is helpful and 👎 if it is irrelevant.

Ask Greptile · Edit Issue Bot Settings

[stormpanda]

Good bot.

[photonstorm]

No need to waste CPU/mem cloning the object, can just handle it a different way.

Thank you for submitting this issue. We have fixed this and the fix has been pushed to the master branch. It will be part of the next release. If you get time to build and test it for yourself we would appreciate that.

[stormpanda]

Wow, that was quick! Thanks a lot :) Looks good to me and works as expected 👍