Add text_inserted and text_deleted signals to LineEdit.

[ndarilek]

This continues my work on a Godot accessibility addon for blind users.

Without these signals, the only way to determine if text has been inserted or deleted is to do the following:

• Cache the current text.
• Hook the text_changed signal, then do the following:
  • Loop through the new text, comparing it with the old, to determine what text has been added. Correctly manage the index to avoid off-by-one errors in the diff, and also handle the corner case where text is pasted so more than one character was inserted.
  • Essentially do the opposite on deletion.
• Cache the new text for future iterations.

This is certainly doable, but a huge mess, and realistically I'm probably going to be the only one developing this addon for quite some time. So if it doesn't need complex diffing logic right out of the gate, that's probably a massive plus.

It's probably also better to handle this directly in the engine, because the engine already knows when text is being inserted or deleted. As such, the logic is much simpler.

I'll gladly implement this in TextEdit once we've agreed on the design.

I'm a bit unclear how to handle the undo/redo case. Given that the only use for this is a screen reader, I'm happy just stating that the screen reader won't perfectly handle the undo/redo case for now, then punting implementation down the road for anyone else who may need it.

Thanks.

[groud]

It's a little bit unclear what the usecase is. Why would you need that ?

In any case this feels a little bit too specific. I wonder if it is worth merging.
I think there's probably a better way to handle that, either via input, either via checking what's in the undo-redo queue. If it's not possible, I think it would probably be cleaner if we use one of the text_changed signal parameters, instead of defining new signals.

[ndarilek]

#14011, and https://gitlab.com/lightsoutgames/godot-accessibility For context, I built a major Android screen reader, so when I add features like these, they're not random. It's based on my expertise as a screen reader developer. I've also done a bit of work on other screen readers, so in general I know what is needed for a good accessibility API. The PRs I'll be submitting are essentially building an accessibility API into Godot, and they're going to be needed if folks are interested in this feature. It may be possible to implement an obfuscated solution for each issue with which I struggle. Then again, the accessibility addon I'm building is already fairly complicated, and Godot offers a clean way to achieve much of what I want in the form of signals. If I can't solve these problems in a cleaner way, and instead have to layer hack upon hack, I may just have to give up on this project. Sorry, I don't mean to seem rude. This project is already complicated, I'm essentially working on it without help, and if I'm going to end up with a hot mess that's confusing and difficult to maintain, I think I'd rather just abandon it and move on.

[ndarilek]

Sorry for the noise. To add a bit less frustrated/emotional context to this:

I submitted #19522 about a week ago, and was advised to pursue a solution other than signals. I did that, then when I needed to improve the accessibility of LineEdit even further to account for typing/deleting characters, the caret-tracking code I added to compensate for not having a signal started triggering when it shouldn't. So because I implemented one feature without a signal, which I specifically designed not to trigger in this case, I have to add additional code to distinguish between text moves because the cursor is navigating around, and text moves because characters are inserted and deleted. I recognize that this is only a single issue, but multiply it by 10, 20, etc. and maybe you can see my concern. Eventually my accessibility addon, which I'll probably be the only or significant maintainer of, is going to have so many hacks to perform things everyone else has signals for, that it's going to be a pain or impossible to maintain, and that's already starting. I want to be sensitive to folks' needs and not add a signal for every little thing, but at the end of the day I'm going to need something if folks truly support this project, and typing text/arrowing around isn't exactly a path where every last bit of performance is needed. :)

Thanks.

[groud]

Well, we provide a generic API, not a bloated one with all possible use cases. The text_changed signal is a generic one and works in 99% of use case. So yeah, it might need a little bit of workarounds in your situation, since your use case is really specific.

But to be honest, I dont even find the workaround you presented that dirty.

[groud]

That being said, adding the signals might be worth it, but i think it would be better to have more than one specific use case to be sure if it is worth adding to the core.

[ndarilek]

    But to be honest, I dont even find the workaround you presented that dirty. You mean this PR? I haven't presented another workaround for the fact that a built-in screen reader needs to be able to distinguish between text that is inserted/deleted, and trust me, a loop through the text performing comparisons, then tracking whether text is adjacent or not to know whether to speak it as a single chunk or character, will *not* be elegant. :)

[ndarilek]

That may be too high of a bar. Accessibility APIs in operating systems only really have one use case--exposing necessary data and changes to accessibility APIs to consume them. I realize the analogy isn't perfect, but I'm trying to change core as minimally as possible, and I shouldn't need to introduce code into the actual rendering path to do it. I don't anticipate adding a million signals, but text editing is one of those corner cases that needs a lot of underlying support to be accessible, and I just don't see something that may slow down typing very very slightly being worth some sort of hacky solution in my addon.

[groud]

a loop through the text performing comparisons, then tracking whether text is adjacent or not to know whether to speak it as a single chunk or character, will not be elegant. :)

Yeah, that one. it might not be elegant, but that's not the purpose of code. If it's 10 lines more on your side, but bloat for everyone in the API, it might not be worth adding. Since the workaround is not that complicated and the use case is very specific.

Just follow the chart ^^:
https://twitter.com/reduzio/status/941799197774315525

Anyway, it should probably be discussed with other on the next PR meeting.

[ndarilek]

Sorry, I thought this would be obvious from the links I provided. I'm blind, and am trying to build accessibility support into Godot so I can use its editor with a screen reader to build audio games. Since I can't see, I can't see the chart you posted to derive any useful benefit from it. :) Either way, I like to think I'm not so much *asking* for features as building them myself, but I'm going to need some buy-in if folks are actually interested in Godot becoming accessible to blind developers. Anyhow, when is this upcoming PR meeting? I can try attending if it doesn't conflict with $dayjob.

[groud]

Oh sorry I did not figured out. Obviously the picture isn't useful... ^^
Well for now the PR meetings are quite happing wherever @reduz is available. There's probably going to be one next week (with 200 pending PRs, it's pretty urgent now...).

Regarding your project, I think this is going to be difficult to build it using gdscript though. Maybe I'm wrong, but I think that might be the the kind of things that could be implemented directly in the engine itself, instead of plugins (screen reader compatibility I mean). Since it would benefit for both the editor and games. But I have no clue how such thing could be implemented.

[ndarilek]

I mean, if bloat is the concern, then implementing all the accessibility functionality from my addon directly into the engine is going to add *lots* of bloat. :) For an idea of what I'm doing, see https://gitlab.com/lightsoutgames/godot-accessibility/blob/master/addons/accessibility/accessible.gd. You can see that it intercepts focus events and prints details on those to the console. Eventually this will be routed to TTS, but I haven't built that addon yet. So the addon minimizes engine bloat, but the problem with doing *everything* in the addon is that the engine knows things the addon can't. So for instance, the engine with this PR now emits `text_inserted` and `text_deleted` events specifically when text is added or removed. I decided not to cover the undo/redo case because that may make multiple changes in different areas of the edited text. Maybe someone does a find/replace in a `TextEdit` for instance, and suddenly 50 versions of "teh" are replaced with "the". In that instance, a pure text diff has no context to disambiguate inserting "t" and changing 50 "teh"s to "the". So what you'd get is 50 "teh"s spoken when text is removed, then 50 "the"s spoken when it is added. With this PR, we simply don't handle the undo/redo case. It might eventually be wise to add undo/redo signals so my addon can just speak "Undo" or "Redo" when these events trigger, or maybe I can catch actions and speak text without handling the event, but I'm happy punting that complexity down the road and figuring it out later. When I make these changes, I don't just grab a signal just because. I do so because I know that screen readers need to work on this highly granular level, and as I've already experienced, implementing #19522 as part of my addon is already interacting with this PR in odd ways I'll have to work around. Anyhow, thanks for the feedback. Hopefully I'm in the channel at the next PR meeting.

[groud]

I mean, if bloat is the concern, then implementing all the accessibility functionality from my addon directly into the engine is going to add lots of bloat. :)

Yeah, but that's for a good cause I think. And it's not bloat in the API, but more code in general, which is fine (as it should not really hurt performances). Built-in accessibility features would be a really good thing in my opinion. Gnome has a built-in accessibility layer, we could probably do something like that.

However, I thing we have no clue where to start to do that. I cannot figure out, from what I read on the net, what is needed. I can't find any API that we could intergrate or something like that. It's a bit annoying. Maybe you could open an issue (or comment on #14011) to describe exactly what is needed ? Writting down what you are trying to achieve might be a good start to get people involved.
(thinking about it that would have been a good "google summer of code" project)

[ndarilek]

Honestly, integrating with a native accessibility API is going to be *lots* of work. You'd essentially have to do it on every platform, too. But in actuality, most game engines already implement the beginnings of an accessibility API. They just don't call it that. So maybe the `focus_enterd` signal is used to render some sort of animation or play a sound when a widget gets focus. It can just as easily be used to implement a built-in screen reader. In contrast, integrating a native accessibility API is going to involve modifying every widget extensively. Instead of just hooking `focus_entered`, I'm going to need to send ATK events, UIA events, whatever MacOS needs, whatever Android/iOS needs, etc. And that's just one signal. And, in the end, this PR would still be necessary. Except, instead of just a few lines of code sending native Godot signals, I'd need multiple `#ifdef WINDOWS/LINUX/MACOS/ANDROID` blocks, integrations with each of their accessibility APIs, etc. So native integrations wouldn't obviate the need for this. They'd just make it more complex. See [this Unity accessibility plugin](https://assetstore.unity.com/packages/tools/gui/ui-accessibility-plugin-uap-87935) as an example of what I'd eventually like to achieve, except I think it only supports playing accessible games, not building them. So this path is definitely viable, but I'm going to either need serious buy-in from core, or to fork the engine and add what I need. I *really* don't want to fork. :)

[groud]

Honestly I don't think this is that much work to call a function when a Control node enters focus.
Most of those nodes share the same code, so its probably not that much work to call a function when they enter focus.

I hoped a generic API would exist, but if there are none, we can still have a platform dependant code somewhere, it's not a problem. I think the best solution would be to create an "accessibility API" within Godot. Like the visual server but for accessibility functions. Still, what it should be able to do is to be defined.

[ndarilek]

What I'm saying is that you already *have* one. `focus_entered` is essentially a piece of an accessibility API, even if for 99% of folks it doesn't seem that way. Similarly, `text_inserted` and `text_deleted` are pieces that would be essential for an accessibility API but are currently missing. Adding a handful of signals is *significantly* lighter than implementing support for 4-5 native APIs directly into the engine. FWIW, I don't know that I'll need too many additional signals. Looking at the editor main screen, for instance, I think I can add keyboard support to things like lists and trees directly from the addon. I may need an accessible label property on image-only buttons similar to an `alt` attribute in HTML because there do seem to be some icon-only buttons, but a lot of what I need is already present. But text-editing is one of those corner cases where I really need deep support in the underlying widget to provide a meaningful screen reader interface. Anyhow, time to get to $dayjob, so I'll lay off commenting for a bit. :)

[mhilbrunner]

I hoped a generic API would exist, but if there are none, we can still have a platform dependant code somewhere, it's not a problem.

Having worked on accessbility in the past, I think we can reach 90% with a plugin and some generic additions in Godot.

As @ndarilek states, most accessbility APIs are mostly overlapping with things that are useful in general. Like the signals added here: they are actually less cumbersome to use in some cases in general, not only for the accessbility use case.

So I think for the time being, an explicit accessbility server in Godot and related infrastructure would be overkill.

I propose we just try to support @ndarilek with this plugin work and make changes where it makes sense if we can accommodate them, especially in cases like this were they may generally be useful and where they are not breaking compatibility.

[akien-mga]

Sorry for the late update.

We didn't really reach a consensus on this issue, and after re-discussing it today on IRC, we think that it's not a good idea to add those signals if they won't have an actual use case in production.

I'd really like to have accessibility features in Godot, either as a plugin or built-in, but our development philosophy is to add features only when there are direct use cases for them. In this case it seems the accessibility plugin is still in its early days, so there wouldn't really be any actual user for those features.

I understand that it's a bit a catch 22 situation, as those features are needed to develop an accessibility plugin, but there's little incentive to develop such plugin if those features don't get merged. There are still ways to implement what the plugin needs without changing the engine though, or alternatively the plugin can initially be developed with a custom build of the engine. Once proven working, the features which need to be added to the engine can be reviewed and likely merged more easily -- it's always easier to justify changing the API when the benefit for the end users is immediately clear.

So I'm closing this for now, but if you're still interested in developing an accessibility plugin for Godot, I'd be happy to discuss further what kind of features are needed to do so exactly, and help you find the right API already provided by Godot, or which one should indeed be added to make accessibility a reality.

[ndarilek]

Thanks, I'd given up on pushing this forward for a number of reasons which you've done a great job of summarizing here, but I regret not having the energy to advocate more for it. In the last few months, I believe the Unreal folks have been planning an embedded screen reader, and more and more console games are launching with speaking menus (See the latest Mortal Kombat, and Madden NFL 20 is doing some amazing work in this area too) so I'd really like to see a compelling open source option. I've been doing my own game development with libraries like Love2D/Pygame, and while those are nice, getting them working on non-desktop platforms ranges from challenging to impossible. So I really wish I had access to something like Unity or Godot which, I gather, makes that process a bit easier. If I picked this up again, how can I get in touch with you, or anyone else who is interested in helping with this? Since I myself am blind, I found it a bit challenging to both discover the interface and make it accessible, and there were many components I was unsure about. Asking those questions on IRC was challenging since IRC lacks context and is easy to lose conversations in. And most of my questions weren't really conducive to something like a GitHub issue. Thanks.