TerryCavanagh / VVVVVV

The source code to VVVVVV! http://thelettervsixtim.es/
Other
6.9k stars 553 forks source link

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

Closed InfoTeddy closed 5 months ago

InfoTeddy commented 5 months ago

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.

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:

Legal Stuff:

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

Daaaav commented 5 months ago

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:

  1. 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?
  2. 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...

Daaaav commented 5 months ago

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

Discord messages