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

BitmapText not wrapping lines in some cases #6860

Closed
bagyoni opened this issue Jul 10, 2024 · 17 comments · May be fixed by jonesrussell/nishman#6
Closed

BitmapText not wrapping lines in some cases #6860

bagyoni opened this issue Jul 10, 2024 · 17 comments · May be fixed by jonesrussell/nishman#6
Assignees

Comments

@bagyoni
Copy link
Contributor

bagyoni commented Jul 10, 2024

Version

  • Phaser Version: 3.80.1
  • Operating system: Debian
  • Browser: Chromium, Firefox

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.

@zekeatchan
Copy link
Collaborator

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
Copy link
Contributor Author

bagyoni commented Jul 11, 2024

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
Copy link
Collaborator

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
Copy link
Contributor Author

bagyoni commented Jul 12, 2024

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
Copy link
Collaborator

zekeatchan commented Jul 12, 2024

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
Copy link
Collaborator

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

@bagyoni
Copy link
Contributor Author

bagyoni commented Jul 12, 2024

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
Copy link
Contributor Author

bagyoni commented Jul 12, 2024

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
Copy link
Contributor

rgk commented Jul 12, 2024

👋 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
Copy link
Collaborator

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
Copy link
Contributor

rgk commented Jul 15, 2024

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
Copy link
Collaborator

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

@zekeatchan zekeatchan reopened this Jul 15, 2024
Copy link

greptile-apps bot commented Jul 15, 2024

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 · Edit Issue Bot Settings

@AlvaroNeuronup
Copy link

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?

@zekeatchan
Copy link
Collaborator

Hi @bagyoni @rgk @AlvaroNeuronup. A fix has been pushed 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.

@rgk
Copy link
Contributor

rgk commented Sep 3, 2024

This has been reintroduced:
#6860 (comment)

Look at the how you handle wrappedLine.

@phaserjs phaserjs deleted a comment from greptile-apps bot Sep 3, 2024
@bagyoni
Copy link
Contributor Author

bagyoni commented Sep 6, 2024

The problem I mentioned in #6860 (comment) still persists.

const text = `first line first line first line
second line second line second line 
third line third line third line`;

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

	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);
		console.log("text length: " + bt.text.length);
	}
}

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

const game = new Phaser.Game(config);

Console output:

last idx: 107
text length: 102

According to docs the idx property is "The index of this character within the BitmapText text string", so that 107 is definitely out of bounds here.
@zekeatchan Should I open a new issue for this? (AFAIK this was not present in 3.80 and probably introduced by one of the patches to this issue.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants