phaserjs / phaser

Phaser is a fun, free and fast 2D game framework for making HTML5 games for desktop and mobile web browsers, supporting Canvas and WebGL rendering.
https://phaser.io
MIT License
36.88k stars 7.08k forks source link

BitmapText not wrapping lines in some cases #6860

Open bagyoni opened 1 month ago

bagyoni commented 1 month ago

Version

Description

If a line of BitmapText.text is (1) long enough to get wrapped, and (2) it ends with a space, then the next non-empty line will never be wrapped, no matter how long it is. In the example, the first and last lines are wrapped correctly, but the middle one is not.

Example Test Code

const text = `
the end is never the end is never the end is never the end is never the \n
the end is never the end is never the end is never the end is never the\n
the end is never the end is never the end is never the end is never the`

class Example extends Phaser.Scene
{
    preload ()
    {
        this.load.bitmapFont('shortStack', 'assets/fonts/bitmap/shortStack.png', 'assets/fonts/bitmap/shortStack.xml');
    }

    create ()
    {
        this.add.bitmapText(0, 200, 'shortStack', text, 16).setMaxWidth(400);
    }
}

const config = {
    type: Phaser.WEBGL,
    parent: 'phaser-example',
    scene: Example
};

const game = new Phaser.Game(config);

Additional Information

Might be related to #6717.

greptile-apps[bot] commented 1 month ago

For internal use only

To address the issue with BitmapText not wrapping lines correctly when a line ends with a space, you can modify the word wrapping logic in BitmapText.js. Specifically, ensure that the wrapping logic correctly handles trailing spaces. Here are the steps:

  1. Open BitmapText.js located in the src/gameobjects/bitmaptext/ directory.
  2. Locate the function responsible for word wrapping, likely within the updateText or similar method.
  3. Modify the logic to ensure that trailing spaces do not prevent the next line from wrapping correctly.

Example modification:

// Example modification in BitmapText.js
function wrapText(text, maxWidth) {
    const words = text.split(' ');
    let wrappedText = '';
    let line = '';

    words.forEach(word => {
        const testLine = line + word + ' ';
        const metrics = this.context.measureText(testLine);
        const testWidth = metrics.width;

        if (testWidth > maxWidth && line.length > 0) {
            wrappedText += line + '\n';
            line = word + ' ';
        } else {
            line = testLine;
        }
    });

    wrappedText += line.trim();
    return wrappedText;
}
  1. Save the changes and rebuild the Phaser library if necessary.

This should ensure that lines ending with a space do not affect the wrapping of subsequent lines.

References

/changelog/3.21/CHANGELOG-v3.21.md /changelog/3.60/BitmapTextGameObject.md

#### 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](https://app.greptile.com/chat/github/phaserjs/phaser/master) ยท [Edit Issue Bot Settings](https://app.greptile.com/apps/github)
zekeatchan commented 1 month ago

Hi @bagyoni. Thanks for submitting this issue. We have fixed this and pushed it to the master branch. It will be part of the next release. Do test it out and let us know if you encounter any issues.

bagyoni commented 1 month ago

Thanks for the quick response @zekeatchan. The change appears to cause the first two lines to overlap. (The one in the example I sent begins with a newline, so it won't show this). It also doesn't seem to solve the line wrapping issue on my end. ~(If this is an edge case, would it be a more conservative solution to just trim() each line before calculating the bounds, as the AI seems to suggest?)~ I guess that would create issues with fonts where spaces aren't empty.

zekeatchan commented 1 month ago

Noticed the first 2 lines overlapping as well.

Another push has been made to master with updated maxWidth calculations.

Could you kindly test it out and provide examples if you encounter other issues? Thanks!

bagyoni commented 1 month ago

I'm still seeing the same issues.

const text = `first line overlaps with itself
second line ends with a space 
third line not wrapped as it should be`;

class Example extends Phaser.Scene {
    preload() {
        this.load.bitmapFont(
            "atari",
            "assets/fonts/bitmap/atari-smooth.png",
            "assets/fonts/bitmap/atari-smooth.xml"
        );
    }

    create() {
        this.add.bitmapText(50, 50, "atari", text, 20).setMaxWidth(300);
    }
}

const config = {
    type: Phaser.AUTO,
    parent: "phaser-example",
    width: 800,
    height: 600,
    scene: Example
};

const game = new Phaser.Game(config);

Note that I used the "Dev Build" version in Labs, not sure if that's always up to date with master.

zekeatchan commented 1 month ago

Ok, I know why you're not seeing the update... I only pushed to master branch and have not pushed to the Labs dev build.

Here's the url you can try out: https://labs.phaser.io/edit.html?src=src\bugs\6860%20bitmap%20text%20line%20wrap.js&v=dev

Here's a screenshot using your example code: image

zekeatchan commented 1 month ago

Here's a screenshot using the code you first posted: image

bagyoni commented 1 month ago

Yeah, my bad, I didn't know which version it was using (also tried building from source, but the first version must have got stuck in a cache because that didn't work either). Lines no longer overlap, but the wrap width is still incorrect in some places - see the first screenshot you posted, the word "itself" extends beyond maxWidth.

bagyoni commented 1 month ago

More concerningly, character indexes are now also off. If you update the previous code with

create() {
    let bt = this.add
        .bitmapText(50, 50, "atari", text, 20)
        .setMaxWidth(300);
    let characters = bt.getTextBounds().characters;
    console.log("last idx: " + characters[characters.length - 1].idx); // prints 105
    console.log("text length: " + text.length); // prints 101
}

it will report that the index of the last BitmapTextCharacter within text is larger than its length.

rgk commented 1 month ago

๐Ÿ‘‹ Had to go for a quick test, placing spaces at the end of lines adds a newline before it.

without space after the s string Screenshot 2024-07-12 134902

with space after the s string Screenshot 2024-07-12 134853

const text = `first line overlaps with itself
second line ends with a space \n sssssssssssssssssssssssssssssssssssssssssssssssss
third line not wrapped as it should be`;

is the text.

also resetting lastGlyph and lastCharCode isn't needed anymore.

zekeatchan commented 1 month ago

Had another look at this over the weekend. Here's the updated screenshot. The labs dev version is not always up to date with the master branch so best is to get the master branch version for your use.

image

rgk commented 1 month ago

https://github.com/phaserjs/phaser/compare/944c6be993adb4a1a334ecb76622703a720d7cfc..6fd076991507075b293b4df7f90a73f1f2d58a6b#diff-2f451878b483f7a17db65a497c5b42d9072ac80ac6bfc3555779f5e2b0a83692 Ah much better but it looks like you introduced a new issue with a space at the end and a space at the start.

image

const text2 = `first line overlaps with itself 
 second line ends with a space
third line not wrapped as it should be`
zekeatchan commented 1 month ago

Thanks @rgk. Appreciate you sharing more examples so I can further fine tune.

greptile-apps[bot] commented 1 month ago

For internal use only

The issue with BitmapText not wrapping lines correctly when a line ends with a space can be addressed by modifying the word wrapping logic in the BitmapText class. Specifically, check the BitmapText.js file for the setMaxWidth method and the getTextBounds method. Ensure that the logic correctly handles trailing spaces and wraps the next line appropriately. You may need to adjust the conditions that determine when a line break occurs.

References

/changelog/3.21/CHANGELOG-v3.21.md /changelog/3.60/BitmapTextGameObject.md

#### 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](https://app.greptile.com/chat/github/phaserjs/phaser/master) ยท [Edit Issue Bot Settings](https://app.greptile.com/apps/github)
AlvaroNeuronup commented 1 month ago

The issue: BitmapText setMaxWidth() bug #6807 was solved in Beta 3.85.0-1 but its not working in Beta 3.85.0-2 again could be because of this changes?