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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix RichTextLabel crash with out of bound exception #68325

Merged

Conversation

pfertyk
Copy link
Contributor

@pfertyk pfertyk commented Nov 6, 2022

Fixes #68242.

I am not completely sure how the _find_list method works 馃槄 This check will stop the error from happening, but perhaps the iteration itself is not valid now. We are iterating over list and prev_item and then using the index to access frame->lines, and I'm not sure if that process is correct. If this fix should be implemented differently, please let me know 馃槃

@pfertyk pfertyk requested a review from a team as a code owner November 6, 2022 12:11
@YuriSizov YuriSizov added this to the 4.0 milestone Nov 6, 2022
@YuriSizov
Copy link
Contributor

None of your builds seem to have to pass. Please do a rebase and try building it locally again to see if this is even valid.

@pfertyk pfertyk force-pushed the issue-68242-rich-text-label-crash branch from 1ff2d39 to 00c4ad7 Compare February 8, 2023 21:44
@pfertyk
Copy link
Contributor Author

pfertyk commented Feb 8, 2023

Rebase done, but I can see the warnings. I'll check if I can do something about them next week.

@YuriSizov
Copy link
Contributor

error: comparison of integers of different signs: 'int' and 'unsigned int'

is the error you're getting.

@akien-mga akien-mga modified the milestones: 4.0, 4.1 Feb 17, 2023
@pfertyk pfertyk force-pushed the issue-68242-rich-text-label-crash branch from 00c4ad7 to 7117c87 Compare February 28, 2023 21:47
@pfertyk pfertyk force-pushed the issue-68242-rich-text-label-crash branch from 7117c87 to 44592c8 Compare February 28, 2023 21:50
@YuriSizov
Copy link
Contributor

YuriSizov commented Mar 7, 2023

While this would fix the crash, I'm not sure why we arrive in this state in the first place. It feels like there is some underlying problem (and of course the reported issue is completely artificial, which might be the reason for this weird state).

But if @bruvzg approves, I think we can merge it as is.

@akien-mga akien-mga requested a review from bruvzg March 7, 2023 14:00
@YuriSizov YuriSizov merged commit dac2d8f into godotengine:master Mar 8, 2023
13 checks passed
@YuriSizov
Copy link
Contributor

Thanks!

@YuriSizov
Copy link
Contributor

Cherry-picked for 4.0.1.

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.

Executing RichTextLabel functions crashes Godot
4 participants