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

Add ability to change FlxText leading #2334

Closed
wants to merge 4 commits into from
Closed

Add ability to change FlxText leading #2334

wants to merge 4 commits into from

Conversation

DigiEggz
Copy link
Contributor

No description provided.

@Gama11
Copy link
Member

Gama11 commented Jun 29, 2021

I'm not sure if it's usfeul enough to warrant a new top-level field, evidently there hasn't been much of a demand for it thus far. It's a bit more verbose, but I think you should already be able to set the leading via text.addFormat().

@DigiEggz
Copy link
Contributor Author

I'm not sure if it's usfeul enough to warrant a new top-level field, evidently there hasn't been much of a demand for it thus far. It's a bit more verbose, but I think you should already be able to set the leading via text.addFormat().

addFormat() takes in FlxTextFormat, but unfortunately it doesn't accept leading as a parameter. Would it be better to expand what FlxTextFormat accepts?

@Gama11
Copy link
Member

Gama11 commented Jun 30, 2021

I don't think it needs to, this should work?

var format = new FlxTextFormat();
format.format.leading = something;

@DigiEggz
Copy link
Contributor Author

DigiEggz commented Jun 30, 2021

I don't think it needs to, this should work?

var format = new FlxTextFormat();
format.format.leading = something;

format is local, so it cannot be accessed from a FlxTextFormat variable. If the variable was set to public and then added via addFormat(), I am still unsure of the correct way to call updateDefaultFormat to redraw the sprite.

@Gama11
Copy link
Member

Gama11 commented Jun 30, 2021

Oh right, it's private. I guess it could simply be added as a constructor parameter of FlxTextFormat.

I am still unsure of the correct way to call updateDefaultFormat to redraw the sprite.

I don't think that's necessary, addFormat() sets _regen, which leads to a redraw.

@DigiEggz
Copy link
Contributor Author

DigiEggz commented Jul 1, 2021

Oh right, it's private. I guess it could simply be added as a constructor parameter of FlxTextFormat.

I am still unsure of the correct way to call updateDefaultFormat to redraw the sprite.

I don't think that's necessary, addFormat() sets _regen, which leads to a redraw.

I tried adding a parameter for leading when creating a new FlxTextFormat, but using addFormat() does not reflect the changes. Things such as the text color are updated for the sprite, but the leading property isn't visually updated.

FlxBitmapText has lineSpacing, which lets you adjust the distance between lines. Would naming it something like this make it more obvious / useful? As it stands, I can't find a way to adjust it on FlxText.

@Gama11
Copy link
Member

Gama11 commented Jul 3, 2021

I guess copyTextFormat() also needs to be updated?

Thinking on it again, a public leading property on FlxTextFormat is probably better than a new constructor parameter. Otherwise we'll end up with too many parameters if we want to expose more TextFormat properties in the future.

@DigiEggz
Copy link
Contributor Author

DigiEggz commented Jul 3, 2021

I guess copyTextFormat() also needs to be updated?

Thinking on it again, a public leading property on FlxTextFormat is probably better than a new constructor parameter. Otherwise we'll end up with too many parameters if we want to expose more TextFormat properties in the future.

Thanks for copyTextFormat() clue!

I've changed it so that it adds a public field to change the leading property on TextFormat.

Edit: In the steep part of my GitHub learning curve. Accidentally reverted the update. Not sure how to go back to the correct changes, which are in dd72a4a

@Gama11 Gama11 closed this in 5bc8e5f Jul 13, 2021
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 this pull request may close these issues.

2 participants