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

Fix RichTextLabel character line and paragraph index getters #76759

Merged
merged 1 commit into from
Jul 21, 2023

Conversation

Nidjo123
Copy link
Contributor

@Nidjo123 Nidjo123 commented May 5, 2023

Fix conditions for returning the character line and paragraph index for RichTextLabel.

Fixes #76758.

@Nidjo123 Nidjo123 requested a review from a team as a code owner May 5, 2023 19:16
@KoBeWi KoBeWi added this to the 4.1 milestone May 5, 2023
Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally (rebased against master a83eb16), it works.

Output from the MRP linked in #76758:

'T' is on line 0, paragraph 0
'h' is on line 0, paragraph 0
'i' is on line 0, paragraph 0
's' is on line 0, paragraph 0
' ' is on line 0, paragraph 0
'i' is on line 0, paragraph 0
's' is on line 0, paragraph 0
' ' is on line 0, paragraph 0
't' is on line 1, paragraph 0
'h' is on line 1, paragraph 0
'e' is on line 1, paragraph 0
' ' is on line 1, paragraph 0
'f' is on line 1, paragraph 0
'i' is on line 1, paragraph 0
'r' is on line 1, paragraph 0
's' is on line 1, paragraph 0
't' is on line 1, paragraph 0
' ' is on line 1, paragraph 0
'p' is on line 2, paragraph 0
'a' is on line 2, paragraph 0
'r' is on line 2, paragraph 0
'a' is on line 2, paragraph 0
'g' is on line 2, paragraph 0
'r' is on line 2, paragraph 0
'a' is on line 2, paragraph 0
'p' is on line 2, paragraph 0
'h' is on line 2, paragraph 0
'
' is on line 2, paragraph 0
'S' is on line 3, paragraph 1
'e' is on line 3, paragraph 1
'c' is on line 3, paragraph 1
'o' is on line 3, paragraph 1
'n' is on line 3, paragraph 1
'd' is on line 3, paragraph 1
'
' is on line 3, paragraph 1
'T' is on line 4, paragraph 2
'h' is on line 4, paragraph 2
'i' is on line 4, paragraph 2
'r' is on line 4, paragraph 2
'd' is on line 4, paragraph 2

Maybe @bruvzg wants to take a look at the logic just to make sure it's OK.

@akien-mga akien-mga requested a review from bruvzg June 18, 2023 14:07
Copy link
Member

@bruvzg bruvzg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logic seems fine to me, visible_character need to be tested to make sure its behavior is the same (might have the same issue).

@akien-mga akien-mga modified the milestones: 4.1, 4.2 Jun 23, 2023
@akien-mga akien-mga added cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release and removed cherrypick:4.0 labels Jun 23, 2023
@YuriSizov
Copy link
Contributor

@Nidjo123 @Calinou Would you mind checking if visible_character is similarly affected?

@YuriSizov YuriSizov merged commit e38dc58 into godotengine:master Jul 21, 2023
@YuriSizov
Copy link
Contributor

Let's merge what we have. Thanks!

But it would be great to look into the possible remaining issue and at least confirm in case it doesn't exist.

@Nidjo123
Copy link
Contributor Author

@YuriSizov, sorry, I somehow missed the previous notifications. Will check and open an issue/PR if visible_character is also affected.

@YuriSizov
Copy link
Contributor

Sounds great!

@YuriSizov YuriSizov removed the cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release label Sep 21, 2023
@YuriSizov
Copy link
Contributor

Cherry-picked for 4.1.2.

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

Successfully merging this pull request may close these issues.

RichTextLabel returns invalid line and paragraph index for first respective characters
6 participants