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 a universal Text node to supersede Label and RichTextLabel #826

Open
rdmtt opened this issue May 11, 2020 · 4 comments
Open

Add a universal Text node to supersede Label and RichTextLabel #826

rdmtt opened this issue May 11, 2020 · 4 comments

Comments

@rdmtt
Copy link

rdmtt commented May 11, 2020

Describe the project you are working on:
A story rich, dialogue heavy RPG game / framework.

Describe the problem or limitation you are having in your project:

  • RichTextLabel has bbcode and custom effects but doesn't have auto-wrap option, ignores grow property, does not resize it's parent container, etc.

  • Label can do all above things but does not support bbcode formatting and effects.

Both those nodes lack something. Moreover, I can see how they might be really confusing for newcomers, as there is no explanation why Godot needs two nodes for essentially the same job - showing text on screen.

https://i.imgur.com/UfZZK2O.png
image

Describe the feature / enhancement and how it helps to overcome the problem or limitation:
We need one, universal and consistent node for showing text on the screen.

Describe how your proposal will work, with code, pseudocode, mockups, and/or diagrams:
Codes for Label and RichTextLabel should be merged under a new Control node - simply named Text.

If this enhancement will not be used often, can it be worked around with a few lines of script?:

  1. I think showing gui text is something developers do a lot.
  2. My workaroud: I made a custom text node (.tscn) that consists of Label as the scene root and RichTextLabel as the child. In the root I add clean text (without bbcode) to let the node figure out it's margins, size, etc. and in the child I add the whole text (with bbcode). Then I make Label invisible (self modulate alpha set to 0). That way I can have Label's margins, grow directions and auto-wrap, and RichTextLabels' bbcode.
    This sort of works. But it's clearly too much fiddling for such a simple thing as showing text on the screen.

Is there a reason why this should be core and not an add-on in the asset library?:
It's not just about adding the Text node but also cleaning up nodes. The engine will be easier to use and code easier to maintain with this new node replacing the old ones.

Additonal notes:
There is a big difference between the amount of code lines in Label.cpp and RichTextLabel.ccp (725 vs 3005) which leads me to believe that there must be a good reason why these nodes were created separately. But I still think that combining these nodes would simply lead to less confusion and more understanding from newcomers.
Also, there are things like, the same properties are called differently in label and richtextlabel:
https://i.imgur.com/stvDyS2.png
image

They are overall so confusing.

@rdmtt rdmtt changed the title Add a one, universal 'Text' node Add a new, universal 'Text' node May 11, 2020
@Calinou Calinou changed the title Add a new, universal 'Text' node Add an universal Text node to supersede Label and RichTextLabel May 12, 2020
@willnationsdev
Copy link
Contributor

Well, afaik, the reason the two are engineered separately is because the fundamental internal structure of the RichTextLabel is much more complex. It maintains a tree hierarchy of "items" that refer to nested blocks of content, e.g. a bolded and italicized link sitting inside of a table that rests underneath an image. The Label node, in contrast, is much more simplified than that, so it's codebase is likely easier to adapt with usability features like the ones you are wanting. Additionally, I surmise that the Label node existed before the RichTextLabel and so rather than hijack and mess up the Label's implementation, they opted to write an entirely new label class to handle rich text.

I think the idea of having a single codebase for text nodes is an interesting idea, as it could provide much needed consistency to their relationship. Basically, if you took the time to implement all of Label's features inside of RichTextLabel, and then renamed RichTextLabel to Text, you'd more or less have what you're asking for.

However, is there perhaps an even better, more abstraction-oriented approach one could take for this? If you're going to take the time to refactor the design of these classes, it might also be good to engineer them to better support other markup languages besides just rich text, e.g. Markdown, ReStructuredText, etc.

If it were possible to define a base class with abstractions to potentially handle a variety of implementations to support rendering different languages, it would be pretty useful. Markdown is part of #139. ReStructuredText would be useful if one wanted to try rendering the godot-docs repo directly in the Godot Editor (though, this would probably be a plugin rather than an integrated feature if done). Who knows what else people might try doing with a Text node that has a good low-level API for building rendered content from a string variable.

@rdmtt
Copy link
Author

rdmtt commented May 13, 2020

@willnationsdev

Basically, if you took the time to implement all of Label's features inside of RichTextLabel, and then renamed RichTextLabel to Text, you'd more or less have what you're asking for.

Yes, that's what I tried to say. We don't need an entirely new node, just a merge of one into another.

However, is there perhaps an even better, more abstraction-oriented approach one could take for this?

One idea I had is that maybe it should behave kind of like Physics Bodies (bear with me).
In this example, the collision node handles the collision detection independently of the body. But they are working together:
kinematic

With text, it would be like this:
text
That way, code could still kept separate but now Label only handles size and position, while the child node handles ONLY the text formatting.

@willnationsdev
Copy link
Contributor

Oh, I see. that'd be an interesting approach. You engineer the text format renderer to source all of its positional information from the parent Text node. That could be possible.

@MaaaxiKing

This comment has been minimized.

@Calinou Calinou changed the title Add an universal Text node to supersede Label and RichTextLabel Add a universal Text node to supersede Label and RichTextLabel Jun 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants