Factorio-Access / FactorioAccess

An accessibility mod for the video game Factorio, making the game accessible to the blind and visually impaired.
Other
24 stars 9 forks source link

add better localization for lists #52

Open LevFendi opened 6 months ago

LevFendi commented 6 months ago

EDITED: This issue originated from trying to localize technology descriptions with too many reward items listed for them. Error message: "Too many parameters for localised string: X > 20 (limit)" Example: Fluid handling.

The crash was fixed by trimming long reward lists but the longterm issue remains for fully localizing lists.

LevFendi commented 6 months ago

Perhaps we need to analyze the whole tech tree for the few cases that have this. For fluid handling specifically, we can perhaps disregard all the recipes that involve barrels.

EphDoering commented 6 months ago

I think we should make a localise_list function. Wherever we want to speak a list we just call the function to get the localised string version of the list. The function could save the latest list (more on that later), format it as required, truncate the list as required, substituting the remainder for some {fa.and-more} localised string, and return the localised string. Then we could have a last list review capability that lets you review the list item by item. This would be a generic reusable solution we could use anytime we want to print a list of stuff. In case the list gets unexpected long it's automatically dealt with.

LevFendi commented 5 months ago

I like the proposed generic and reusable solution. Until we have that, the crash can be patched by simply cutting off the reading after the first 10 or so unlocks. In general, a technology that unlocks more than 10 things is most likely to be unlocking a large group of incredibly similar things.

LevFendi commented 5 months ago

The crash has been resolved with the latest change of limiting the reward list size to 5, but the issue may pop up again elsewhere. The longterm issue of the list localizing function remains open.

ahicks92 commented 5 months ago

@EphDoering Sadly I think the long term solution is going to be printing this in multiple parts, which would imply a launcher command to delay saying stuff until everything is printed. Reusable solutions that allow for more than the 20 limit are either that or concatenating localised strings and hoping that nesting gets us passed it. The third solution requires a tick to fetch the localisations and a hope that we know how to assemble lists manually for that language, neither of which work out. In fact getting the localisations isn't even a problem in this case because of networking, it's a problem because we have to either delay all speech by one tick or break the speech order if someone is looking at a long enough list.

I do like something generic for review last list. My in progress UI system thing should be able to handle it. I dunno how much I'd use it, but it's elegant. Also some things like logistic network contents should really just open it by default and, e.g., have sorting.

@LevFendi Can this be triggered on Vanilla? It's either very very easy or very very hard, and I don't mind taking 5 minutes to figure out if it is very very easy.

EphDoering commented 5 months ago

I don't get why this would be hard to do in the Lua. We have a last list variable that's on the player's global. Whenever we're going to speak a list, we format via the localize list function. The trigger for the error is trying to use more than 20 parameters to a localized string template. The only time I ever foresee having 20 or more parameters to a localized string would be when we use the built in concatenation template to make lists of arbitrary length. So if instead of doing that, we always just call the function we shouldn't run into this error.

ahicks92 commented 5 months ago

You can't limit it though. I already got into it on Discord and I think those questions are (maybe) resolved--if not I'll follow up.

To cross post/whatever, the problems are twofold: firstly I at least do want to hear more than 20 items, for a simple case consider a chest where you want to find stuff and put your cursor on it and get a useless "and other items" at the end, that makes me vaguely sad every time it happens. It's even worse if you're trying to debug say the logistic network and don't know what "and other items is": in that case ctrl+f in the inventory fails you. List review would turn a slightly annoying "listen to this long thing" into a "Open this UI and arrow through everything" (to be clear I agree it's useful, just not for these cases; we have a bigger meta-problem wherein you can't tell what's in an inventory or find total quantities or...in general we need to work on that UX, but it is out of scope).

Secondly we don't have loops so even if we did limit it to 20, the "easy" way is that we'd have to localize 20 strings, not one. If we accept a limitation that is at least fine for English and Spanish, and I think everything else "western", we can do arbitrarily long lists provided the engine lets us do 20 params at each level of the 20 allowed nesting levels.

Note that as regards the UI, the mod does not have anything centralized at the moment, in particular the ability to remember where you came from when another thing opens. As a result we either eat list review closing all the menus that have lists, or we'll have to hack in a special case in quite a few places, turning what is already rather horrible magic unnamed constants into...worse. I don't know all the places myself; only @LevFendi could feasibly do it unless you know how. I'm disinclined to say we should because that's just more tech debt and this time it's literally extending something already very shaky. I can for example sometimes trick our UI into also moving the cursor when navigating menus (no I don't have a repro, it's rare, thus no issue yet, plus I'd not be surprised if it's that I switch branches and work directly from source or whatever anyhow).

ahicks92 commented 5 months ago

Ok, so I was wrong considering the math again: we can do this with one-argument or two-argument strings because that'd actually be 2^20 roughly. You would do:

top=__1__ AND __2__
pair=__1__, __2__,
single=__1__,

And then:

{ "top", { "pair", { "pair", thing, thing } }, last item  }

And so on, using single to make up for ending up with a list whose length is even (because the left side would be odd, and that's not pairs). I still need to test if this works but even if we had to have some sort of buffer speech command in the launcher, that'd match any language which can do lists of the form item, item, conjunction? final item. Put briefly it's a binary tree.

Unfortunately ultimately the localisation system is basically going to limit us to "English-like" languages. I know there is a name for it but can't remember or find it quickly. I suspect that Japanese and Chinese speakers for example are probably not super happy with even the official localisation, though perhaps that is also why the game avoids text in a lot of places.

We would of course also need something to localise 10 iron plate or what have you where it's quantity and item.

If there is a hidden feature then maybe that's helpful. The tutorial on the wiki documents the features, the main doc documents basically that you can use __1__, nothing about plurals or localising keyboard inputs, nothing.

EphDoering commented 5 months ago

Yes, this is similar to what I had in mind, except, I didn't think making an arbitrarily long list was actually going to be helpful. For example say you just grabbed the contents of a steel chest that had 30 different items in it and you want to know how much uranium 235 you just grabbed. If we just list everything, you'd have to sit and wait for the whole list to go by waiting for the count of uranium 235. If you're not paying close enough attention you'd have to replay it. I thought it would be helpful if we had the ability to review the list, item by item, that way you can skip ahead to the items you care about and go back if you skipped too far. This ability could be accomplished by just saving the last list from the Lua list formatting function along with a menu that lets you review it. Then, if we have the ability to review full lists, in my opinion we can safely truncate the original spoken list. We could do the binary tree thing and just let the player decide when they've had enough of listening to a long list, but then we'd better make sure we never say anything important after a list. I think the ability to review a long list is important regardless of whether we truncate or make the lists effectively arbitrarily long.

Regarding localizing 10 iron plate, I was thinking that the list function would be passed a table of localised strings, so one of the items within that table argument would be the table:

{ "fa.item-quantity", "item-name.iron-plate", 10 }

That way the list formatting function could take lists of anything and the caller would be responsible for preformatting each item.

Then in our cfg file we could have:

[fa]
item-quantity=__2__ __1____plural_for_parameter_1_{1=|rest=s}__

or we could get fancy and make an item_quantity function that would pick a localization template based on a player's setting so they could choose between that and the item x quantity that we currently use.

ahicks92 commented 5 months ago

I'm less concerned about what list items are than I am about the "spine" of the list that everything hangs off of. I'm also on board for a list reviewer, except that the more I play the more I want the mod to just expose an alphabetized menu for the inventory and just collapse stacks into each other, rather than stacks in a grid.

Screen readers stop speech by just tapping ctrl, or (at the cost of actually doing something) whenever a key is pressed. We get the stop listening part of this for free. Self-interruption when keys are pressed is another problem that's out of scope and not one we can do anything about anyway.

Put another way tapping whatever to open the list review will stop the old one.

We have at least two kinds of list:

I think we should limit the scope of this specific issue to getting it to work no matter how long the list is and, at least for my part, I see a bunch other possible issues: list review, better inventory presentation, searching the whole logistic network for storage chests, searching chests in a selected area/radius of the player, after slot filters searching all cars in a train, way to find out how many of x is in the inventory, probably some more...

EphDoering commented 5 months ago

Yes, we've discussed several related but separate issues. However, I think the best solution for this specific issue is dependent on if list review exists. The recipe ingredients are an example where a list is usually short and fine, but then a mod like rubber duck comes along and makes a recipe with hundreds of ingredients. I don't remember the format for how we read a recipe but if it starts with ingredients and then goes on to talk about speed or something, then you'd have to sit through the whole list just to find out it crafts in 1 second. That's why I'd like to truncate long lists. Say more than 7 items get's truncated to 6 plus fa.more-in-list. There's no vanilla recipe that uses more than 7 ingredients and it's pretty rare in mods so most of the time there wouldn't be any truncation. But truncating is only acceptable if we can review lists.

If we want to resolve this issue with higher priority than list review we can do the binary tree and start using the list formatting function for everything, then decide if we want to truncate after we have a reviewer working.

I'd propose the following for the cfg:

[fa]
list-one-item=__1__
list-two-item=__1__ and __2__
list-first-mid-last=__1__, __2__, and __3__
#note trailing space in the mid list separator
list-mid-separator=, 

and then just using the concatenate localised string to assemble tables:

local function get_mid_localised_list_section(list, from, len)
    local ret = { "" }
    if len == 1 then
        return list[from]
    elseif len <= 10 then
        for j = from, from + len - 2 do
            table.insert(ret, list[j])
            table.insert(ret, { "fa.list-mid-separator" })
        end
        table.insert(ret, list[from + len - 1])
        return ret
    end
    local size = math.pow(10, math.floor(math.log10(len - 1)))
    while len > size do
        table.insert(ret, get_mid_localised_list_section(list, from, size))
        table.insert(ret, { "fa.list-mid-separator" })
        len = len - size
        from = from + size
    end
    table.insert(ret, get_mid_localised_list_section(list, from, len))
    return ret
end
function mod.format_localised_list(list)
    local len = #list
    if len == 0 then
        error("format_localised_list should only be called on non empty lists", 2)
    elseif len == 1 then
        return { "fa.list-one-item", list[1] }
    elseif len == 2 then
        return { "fa.list-two-item", list[1], list[2] }
    end
    return { "fa.list-first-mid-last", list[1], get_mid_localised_list_section(list, 2, len - 2), list[len] }
end
EphDoering commented 5 months ago

hmm... just noticed that the <= 10 check and code aren't actually necessary. It saves len function calls, but makes the code more complicated... I swear my indecision is the slowest part of my coding.

ahicks92 commented 5 months ago

Note that if you just do the binary tree as I mentioned above, and which still has quite enough room, you can avoid log10 and loops. Unless Factorio specifically removed it, Lua supports proper tail calls, so recursion is not an issue.

There is also a formulation where you call a function that groups tables by 2 in a loop until the table it's grouping is 1 item. That does work on tables of odd length if you're careful or allow a group of 3, and for problems like this that's generally my preference because it is obvious to a reader who is not good at recursion what's going on and is not hard to read or maintain for those who do. Plus, other languages have stack overflow issues. In this very highly specific case, I don't think the case of 3 even becomes a special case because an extra layer of nesting doesn't matter.

But okay so. On the one hand if a list review is too long and information comes after it--which is indeed how we do recipes--I agree with you. But if the list is the contents of a chest then you have to read it anyway, and you're not going to skip very much. The spectrum of whether or not truncation plus review is appropriate depends on the context, at least in the sense that we both have a context where both our viewpoints are entirely the right thing to do. In yours information is "hidden" behind the end, in mine we've just switched it around so that instead of a long list it's a few extra keys then arrowing through the long list one by one. That's why I've said both. They both make some use cases but not all easier.

But truncation just isn't necessary in most contexts. We can move important info to the front, e.g. "Copper cable requires 0.5 sec and list list listy list". But it's trivial to tap control as a screen reader user. Truncation literally doesn't matter: you move on from the list because you heard what you want in default screen reader settings, and it shuts up, no problem. No matter how you slice this all we have to do is move things to the front if they're important, after that truncation doesn't add anything, it's just annoying in the cases someone does want to go that far. Anyone who isn't in default settings did it on purpose, the settings that stop it from shutting up are advanced, very useful in some contexts, and very obvious and annoying if turned on globally. Screen readers all do per-app config if you turn that on, which again is a "they're advanced enough to know they did it because it can't be done by accident" thing.

If there is an example of why we can't change the order of info or of why screen readers shutting up if you move somewhere else aren't enough, then I'm happy to revisit truncation. At the moment though I just don't see it, reordering info seems pretty trivial, and a list review isn't going to help anyway because those strings contain a list, they're not a list, at least not now. I guess they could maybe easily be made one, but at that point it's more of "string segment viewer" (which would also be useful to a point).

Your localization schema seems good for English. Mine might or might not be better for other languages. I wish we had someone whose language is far from English to ask but given the choice and assumption that lists are like this everywhere we can handle, yours is better because it composes more simply. Perhaps we should define a list of "official" languages that we want to explicitly support, then check their basic grammar. This is easy to change though so for now shrug.