Fix several bugs relating to switching languages, active input devices, or Flip Mode while having a text box or menu open, and add debug keybind for cycling languages #1138

InfoTeddy · 2024-01-23T23:50:21Z

Previously, there used to be a bug where if you switched languages while a text box was open, the text box wouldn't update and would stay in the original language. This is because the text box translation was baked and couldn't be retranslated on-the-fly.

A kludge solution to this was implemented by disabling switching language with a text box open. However, a proper solution is to un-bake the translations and to retranslate them on-the-fly.

To do this, relevant state is saved for each text box, depending on the type of text box.

Untranslated: Nothing.
Cutscene: The original position and facing direction of the crewmate is saved, if the text box is positioned with respect to a crewmate. Additionally, the X and Y axes are independent of each other and can be overridden (or not) depending on the previous script commands issued.
All other text boxes: Use a function to retranslate the text box on-the-fly. This function must handle the padding, wrapping, and text centering.

All text boxes will save the state of their original position (without considering centering or crewmate position) and if they are individually centered on the X and Y axis or not (which can be overridden by crewmate position).

Additionally, a debug keybind has been added, CTRL+F8, to immediately cycle the current language (except inside a cutscene test and the cutscene test list menu, which are inherently language XML-specific). CTRL+SHIFT+F8 cycles the language backwards. This keybind is only enabled if the translator menu is also enabled. I've also made sure that this works within the title screen menus (even though it wasn't possible to immediately switch the language in a menu before).

With being able to retranslate text boxes on-the-fly, this PR also fixes the following bugs:

The Intermission 1 instructions don't update when toggling Flip Mode (they say "floor" or "ceiling" depending on Flip Mode).
The "Press ACTION to continue" text box doesn't update when the active input device is switched from keyboard to controller, or vice versa.

Legal Stuff:

By submitting this pull request, I confirm that...

My changes may be used in a future commercial release of VVVVVV
I will be credited in a CONTRIBUTORS file and the "GitHub Friends"
section of the credits for all of said releases, but will NOT be compensated
for these changes unless there is a prior written agreement

desktop_version/src/KeyPoll.cpp

Daaaav · 2024-01-25T04:24:00Z

I went through all the code and I don't think there are any big deal breakers...

But I do have to get used to the new textboxes API. It gets me thinking if there's a way to make it less complex? Maybe we need to redesign it from the ground up? It's probably fine to require a separate function for assembling textbox contents, padding, etc, so that that function can be run again later if needed. But it seems a bit unintuitive right now, e.g. to first have an empty textbox created, with all sorts of properties like color, print flags and positioning, and then to have a graphics.textboxtranslate(TEXTTRANSLATE_FUNCTION, compute_xx_textbox); which is where the contents can be found. Maybe for a cleaner API, everything to assemble the text box should go into the compute function, including things that don't need to be recomputed? I was going to give an example with as little other changes as possible, but I keep changing functions like graphics.createtextboxflipme() to immediately take the function in some form... So, If I'd design the API from scratch, I'd do something closer to...

static void textbox_actionprompt(textboxclass* textbox)
{
    textbox->init(-1, 196, TEXT_COLOUR("cyan"));
    textbox->lines.push_back("Some text contents");
    textbox->pad(1, 1);
}

// Callsite
create_textbox_with_function(textbox_actionprompt);

Here, create_textbox_with_function would be something you call at a callsite. This creates a new textbox object which it passes to the function you specified, and also stores your function pointer in that textbox. textbox->init() can use some initial details that are now given to graphics.createtextboxflipme() and the like. If the textbox needs to be reloaded, it simply calls your function again, where some "initialization" things are now no-ops or just do things like clearing the lines.

In general, functions called in a textbox' lifecycle would be:

Things that are only done at spawning time: spawning the not-yet-existent textbox itself, and remembering values that might never come back, like crewmate faces?
Everything else, which may give different results when repeated, but it may also not (however, it won't do something that wouldn't have been done if e.g. the new language had been the language when the textbox was created)

Function naming is another thing. What is textboxoriginalcontextauto()? Not really talking about the concatenatedtogethercase because it's consistent with everything else, but... Original context auto? Automatic original context? What does this do and when should you call it? There's no comment at the top of the function to explain it either. Apparently it's for textboxes that don't need to be changed anymore even if the language is changed. So if you don't call it, the textbox becomes blank when switching the language? Maybe it needs to be the other way around - if all textboxes need to be refreshed, only update those that had some kind of translation function called...

This allows switching languages while a text box is on screen by saving the necessary state for a text box to be retranslated when the language is switched. This saves the state of the position and direction of the crewmate that the text box position is based off of (if applicable), and the text case of the text box, the script name of the script, and the original (English) lines of the text box. I did not explicitly label the original lines as English lines except in a main game context, because technically, custom levels could have original lines in a different language. Unfortunately, this doesn't work for every text box in the game. Notably, the Level Complete, Game Complete, number of crewmates remaining, trinket collection, Intermission 1 guides, etc. text boxes are special and require further fixes, but that will be coming in later commits.

Originally I did a straight deep copy of the original lines, but this ignores the limit of either 12 or 26 lines in a text box. So we defer to addline() which will enforce the limit accordingly, just like it would do with the original text box.

This is another piece of state that needs to be kept and re-played when switching language, because a different language could change the dimensions of the text box, which affects how it's centered. Also, to make sure that crewmate positions override any text centering, the scriptclass variables textx and texty should be reset in the position and customposition commands.

This adds a way to save the text box state of the crew remaining, ACTION prompt, etc. text boxes by just letting there be a function that is called to retranslate the text box when needed. It also adds a way to ignore translating a text box and to leave it alone, in case there's actually no text in the text box, which is the case with Level Complete and Game Complete. Both ways are now in an enum, TextboxTranslate. The former is TEXTTRANSLATE_FUNCTION and the latter is TEXTTRANSLATE_NONE. The existing way of translating text boxes became TEXTTRANSLATE_CUTSCENE, since it's only used for cutscene scripts. Here's a quick guide to the three ways of creating a text box now. - TEXTTRANSLATE_NONE: You must call graphics.textboxoriginalcontextauto() to save the existing text to the original context of the text box, as that will be copied back to the text box after the text of the text box is updated due to not having a translation. - TEXTTRANSLATE_CUTSCENE: Translates the text from cutscenes.xml, and overrides the spacing (padding and text centering). Shouldn't need to be used outside of scriptclass. - TEXTTRANSLATE_FUNCTION: You must pass in a function that takes in a single parameter, a pointer to the textboxclass object to be modified. General advice when retranslating text is to clear the `lines` vector and then push_back the retranslated text. The function is also solely responsible for spacing. In most cases, you will also need to call graphics.textboxapplyposition() or graphics.textboxadjust() afterwards. (Some text boxes shouldn't use graphics.textboxadjust() as they are within the 10-pixel inner border around the screen that textboxclass::adjust tries to push the text box out of.) This commit doesn't fix every text box just yet, though. But it fixes the Level Complete, Game Complete, crew remaining, and ACTION prompt text boxes, for a start.

This splits the text wrapping functionality of Graphics::textboxwrap to a new function textboxclass::wrap, and with this new function, some more text boxes can be moved to the new TEXTTRANSLATE_FUNCTION system. Namely, Game Saved (specifically the game failing to save text box), instructional text boxes in Space Station 1, and the Intermission 1 instructional text boxes.

With the new system of retranslating text boxes on-the-fly, this also enables us to retranslate them whenever the player toggles Flip Mode. This is relevant because the Intermission 1 instructional text boxes refer to a floor when Flip Mode is off, but when it is on, it talks about the ceiling.

Several text boxes in the gamestate system are unused and are untranslated. To prevent them from becoming empty when retranslating text boxes, we need to save their original context by calling graphics.textboxoriginalcontextauto() (which is just graphics.textboxoriginalcontext() but automatically saving whatever is already in the text box at the time).

This transfers the responsibility of the adjust() call to applyposition(). This is because cutscene text boxes (TEXTTRANSLATE_CUTSCENE) will have adjust() called, but all other text boxes won't. And I can't place the adjust() call inside applyposition(), because adjust() also calls applyposition(), and that leads to an infinite loop which leads to a stack overflow, so I had to remove the applyposition() call from adjust(), and replace the other existing call to Graphics::textboxadjust() with Graphics::textboxapplyposition(), and then remove Graphics::textboxadjust() function because it's no longer used.

Just a small optimization. For example, consider the calls in adjust(). After the first resize(), the lines after only change the x-position and y-position of the text box and depend on the x-position, y-position, width, and height. However, resize() only changes the width and height if the contents of the text box change, which after the first call, they don't. So remove the second call to resize(), because it's completely unnecessary. By similar reasoning, the second calls to resize() in centerx() and centery() are unnecessary too.

These seemed annoying to do without copy-pasting, because I didn't want to make a separate function for every single dialogue, and I didn't know how to pass through the English text, until I realized that I can just use the existing original.lines vector in the text box to store the English text. After that, getting it translated on-the-fly isn't too bad.

This adds an assert to Graphics::textboxtranslate() to make sure that callers don't accidentally provide a function when specifying a translation type that isn't TEXTTRANSLATE_FUNCTION, because in that case the function won't be used, and then it will make them scratch their heads wondering why their function won't work. And yes, I am stupid enough to blindly type TEXTTRANSLATE_CUTSCENE when I meant to type TEXTTRANSLATE_FUNCTION. This assert has already caught one of my mistakes. :)

This adds an attribute to textboxclass to allow a text box to keep an index that references another text box inside the graphics.textboxes std::vector. This is needed because the second text box of a "You have found a shiny trinket!" or "You have found a lost crewmate!" pair of text boxes explicitly relies on the height of the first text box. With this, I have moved those text boxes over to the new text box translation system. Since the update order now matters, I added a comment to recomputetextboxes() that clarifies that the text boxes must be updated in linear order, starting from 0.

In order to be able to retranslate the game time text box in particular, I had to create new variables to bake the saved time, since the existing savetime variable is just an std::string. From there, the saved time can be retranslated on-the-fly.

These include the room name translator text boxes (which aren't translated) and the foundlab and foundlab2 text boxes.

This fixes a problem where it would incorrectly format the text because the width of the text box hadn't updated yet. This fixes a bug where the jukebox informational terminal would initially be created with too much padding in CJK languages, pushing the text box offscreen, even though switching languages while the text box is already open fixes it.

This adds a debug keybind to cycle the current language forwards, CTRL+F8. It also adds a debug keybind to cycle it backwards, CTRL+SHIFT+F8. This is only active if the translator menu is active (and so if the regular F8 keybind is also active). This is useful for quickly catching errors in translations and/or inconsistencies between translations. In fact, I've already caught several translation mistakes using this keybind which made me mildly panic that I screwed something up in my own code, only to realize that no, actually, it was the translation that was at fault. For now, this is only meant to be used in-game, as text boxes get retranslated instantly, whereas things like menu options don't. But menu options will be retranslated on-the-fly in a later commit.

This stores the original x-position and y-position of the text box, and when a text box gets repositioned, it will use those unless a crewmate position overrides it. This is the original position of the text box, before centering or crewmate position is considered. This fixes a bug where a cutscene text box can be "shifted" from its normal position via CTRL+F8 cycling if there is a translation that is too long for the screen and thus gets pushed by adjust(). I tested this with the text box in the Comms Relay cutscene that starts with "If YOU can find a teleporter". This is not applicable to function-based translations (TEXTTRANSLATE_FUNCTION), because the responsibility of correctly positioning the text box resides with the function.

This removes Graphics::textboxwrap(), as it is now an unused function. Additionally, this removes the return value of textboxclass::wrap(), as it is also now unused.

This fixes a bug where some text boxes wouldn't update the displayed button if the active input device changed from a keyboard to controller, or vice versa. Namely, the "Press ACTION to continue" text boxes.

Originally, textcase was reset in scriptclass::translate_dialogue(), which is called inside the `text` script command. However, this didn't really work with the new on-the-fly text box translation system, and that function is gone now, so I removed that and kind of forgot about it. Of course, this now causes a regression. Namely, that the text boxes after the VVVVVV-Man sequence in the Secret Lab entrance cutscene are not translated. I can't reset the text case in `text`, as the scripts assume that they can set the text case before `text`. So the next best thing is to reset it in speak/speak_active.

I saw that the only problem with cycling languages in a title screen menu is that the menu options don't get updated. So I was like, we can just recreate the menu, and then I was like "Sure, why not." So that's what I did. To accommodate the CTRL+F8 keybind in the language menu, it automatically updates the menu option when you cycle it. This is because otherwise using the keybind in the language menu wouldn't visibly update the language, but it still actually does change your language, and that can be seen by pressing Escape. Also, the menucountdown needs to be preserved because otherwise createmenu() resets it, even if it's the "same" menu (this behavior is needed so that the menu that is shown during the countdown isn't added as a stack frame which would make it a menu that could be returned to).

Otherwise, cycling languages through CTRL+F8 would result in mismatched languages.

Not gonna lie, I am a bit disappointed at having to do this, because it actually worked pretty well despite a few bugs depending on which language you entered with. But that's only because I'm working with the official translation files, which are in sync with each other. With translation files that are completely arbitrary, it would be apparent that switching languages during the cutscene test doesn't really make sense. Like, at all. That's because the list of cutscenes is populated entirely from language-specific XML and the cutscenes in them are also from language-specific XML. So keeping the same position in the menu doesn't really make sense, and keeping the same position in a cutscene definitely doesn't make sense.

This makes it work pretty well. It basically just resets the state of the limits check and starts from the first limit broken (if any), which is behavior that makes sense to me. Otherwise, without this, it seems to invalidate pointers and, on my machine, start pulling strings from the language XML, which is horrifying.

These weren't getting updated when cycling language with CTRL+F8. This is because they would be already baked. Luckily, at least the bool keeping track of whether or not to translate them in the first place already exists, so we can just rely on that.

While there's a check to recreate the menu if you cycle the language while in a menu, editor menus are a special case and need specific handling.

These are simple strings (no vformat), so we can just un-bake them to make sure that cycling languages with one of them onscreen updates them accordingly.

Daaaav · 2024-02-03T00:52:51Z

Judging by the "compare force-pushed changes" buttons, nothing bad has happened so this should be okay to merge.

This is so they will be updated when switching language with CTRL+F8. Most of the editor notes are simple text that don't use any string formatting. For the ones that aren't, some (saving and loading, changing map size) reference variables that wouldn't change without initiating a new note anyway. For the others, i.e. the ones that _do_ reference variables that could easily be changed (tileset name, speed) by switching the current room, we cache their values and use the cached values when drawing the note. Unfortunately, this requires adding a couple of ugly attributes to editorclass, but it'll be fine.

Daaaav reviewed Jan 25, 2024

View reviewed changes

desktop_version/src/KeyPoll.cpp Outdated Show resolved Hide resolved

InfoTeddy force-pushed the general-bug-fixes branch from 59576fd to 57e39af Compare February 2, 2024 22:59

InfoTeddy added 15 commits February 2, 2024 15:02

Move all other text boxes to new system

9a09165

These include the room name translator text boxes (which aren't translated) and the foundlab and foundlab2 text boxes.

InfoTeddy force-pushed the general-bug-fixes branch from 57e39af to 95f114d Compare February 2, 2024 23:02

InfoTeddy added 9 commits February 2, 2024 15:04

Remove textboxwrap() and wrap() return value

23f445a

This removes Graphics::textboxwrap(), as it is now an unused function. Additionally, this removes the return value of textboxclass::wrap(), as it is also now unused.

Recompute textboxes on active input device change

18d30de

This fixes a bug where some text boxes wouldn't update the displayed button if the active input device changed from a keyboard to controller, or vice versa. Namely, the "Press ACTION to continue" text boxes.

Translate NDM hardest room on-the-fly

7a1fc2d

Otherwise, cycling languages through CTRL+F8 would result in mismatched languages.

InfoTeddy force-pushed the general-bug-fixes branch from 95f114d to e2e7720 Compare February 2, 2024 23:05

InfoTeddy added 2 commits February 2, 2024 16:06

Fix cycling menus in editor not updating

f5ea256

While there's a check to recreate the menu if you cycle the language while in a menu, editor menus are a special case and need specific handling.

Translate editor prompt text on-the-fly

3b1f08b

These are simple strings (no vformat), so we can just un-bake them to make sure that cycling languages with one of them onscreen updates them accordingly.

InfoTeddy merged commit ec3de52 into TerryCavanagh:master Feb 3, 2024
3 checks passed

InfoTeddy deleted the general-bug-fixes branch February 3, 2024 02:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix several bugs relating to switching languages, active input devices, or Flip Mode while having a text box or menu open, and add debug keybind for cycling languages #1138

Fix several bugs relating to switching languages, active input devices, or Flip Mode while having a text box or menu open, and add debug keybind for cycling languages #1138

InfoTeddy commented Jan 23, 2024

Daaaav commented Jan 25, 2024 •

edited

Loading

Daaaav commented Feb 3, 2024

Fix several bugs relating to switching languages, active input devices, or Flip Mode while having a text box or menu open, and add debug keybind for cycling languages #1138

Fix several bugs relating to switching languages, active input devices, or Flip Mode while having a text box or menu open, and add debug keybind for cycling languages #1138

Conversation

InfoTeddy commented Jan 23, 2024

Legal Stuff:

Daaaav commented Jan 25, 2024 • edited Loading

Daaaav commented Feb 3, 2024

Daaaav commented Jan 25, 2024 •

edited

Loading