Fix LineEdit Unicode code-point/control character insertion failing to emit text_changed#111190
Conversation
bruvzg
left a comment
There was a problem hiding this comment.
It's only affecting user input, so should be safe.
|
Thanks for the code reviews, I've applied all the suggested changes and CI is showing up green. Let me know if any more modifications are needed. |
|
Could you squash your commits? See our pull request guidelines for more information |
dc28761 to
82849b8
Compare
Done! The extra code review commits have been squashed. |
|
Perfect! Now there's just one last thing: you'll need to remove the trailing |
…control character insertion.
82849b8 to
01b9208
Compare
All done! |
|
Thanks! Congratulations on your first merged contribution! 🎉 |
text_changed
text_changedtext_changed
Summary
LineEditdid not emittext_changedwhen text was inserted via Unicode code-point input and the “Insert Control Character” menu. This would break UI logic that depends ontext_changed.Reproduction
Minimal scene with a single
LineEditelement and the following attached script:Pressing ctrl + shift + u and typing
2020then pressing enter inserts the dagger character (†) but does not triggertext_changedi.e. no print statement is written to the console.Right click + Insert Control Character (pick any) also inserts but does not trigger
text_changed.Godot.Pre-Fix.mp4
Fix
After performing the two aforementioned insertions, defer
_text_changed()and set thetext_changed_dirtyguard (exact same process used inpaste_text()). Regular typing and paste behavior remains unaffected.set_text()is unchanged and still does not emit, as per the documentation. This is a small, localized fix inscene/gui/line_edit.cppGodot.Post-Fix.mp4
Testing
Verified fix on a Windows dev build, code-point submission and control character insertion now print once to the console and update any bound UI elements. Regular typing, paste, and deletions have been verified to still emit exactly once.
Formatting: clang-format 17
Fixes #110809
Note: There is an overlapping PR #110988 that instead moves the
text_changedemission logic to the core functioninsert_text_at_caret()while removingtext.length() != prev_lenchecks to determine whether to emittext_changedor not. Let me know if this more general change to a core function is preferred, as the changes in my PR attempt to fix two specific bugs without altering core functions while aligning with the existing architecture. A previous PR #84382 also attempted to alterinsert_text_at_caret()directly which resulted in duplicate signals.