Texture atlas breaks when changing option involving menu rebuild

[krzychu124]

Describe the problem

See steps below. Investigating... (TM:PE workshop TEST also affected so probably a bug introduced in texture rebuild fix)

Steps to reproduce

1. start new game
2. enter options and change PF counter visibility at Maintenance tab
3. close options modal and open TM:PE main menu
4. observe missing textures

Now the fun part

1. enter options once again and revert PF counter visibility setting
2. close options modal and open TM:PE main menu
3. observe working textures

Now the weird part

1. break textures like in the first step
2. disable one of the features like Lane Connector
3. close options and observe menu (working)
4. enable the feature that was changed in step 2.
5. main menu breaks again...

Log files

Screenshots?

Notes or questions?

It looks like every second rebuild of menu, texture source or whatever is happening there fixes the problem 🤷‍♂️ Everything works, just textures are missing.

[krzychu124]

Looks like TM:PE is setting wrong atlas when recalculates buttons. After next rebuild atlas is correct so must be a race condition or a bug in code that decides which atlas it should take

[originalfoo]

The menu rebuild is triggered by checkbox event handlers which call OptionsManager.RebuildMenu();

        internal static void RebuildMenu() {
            if (TMPELifecycle.Instance.Deserializing || ModUI.Instance == null) {
                Log._Debug("OptionsManager.RebuildMenu() - Ignoring; Deserialising or ModUI.Instance is null");
                return;
            }

            Log.Info("OptionsManager.RebuildMenu()");
            ModUI.Instance.RebuildMenu();

            // TM:PE main button also needs to be updated
            if (ModUI.Instance.MainMenuButton != null) {
                ModUI.Instance.MainMenuButton.UpdateButtonSkinAndTooltip();
            }

            RoadUI.Instance.ReloadTexturesWithTranslation();
            TrafficLightTextures.Instance.ReloadTexturesWithTranslation();
            TMPELifecycle.Instance.TranslationDatabase.ReloadTutorialTranslations();
            TMPELifecycle.Instance.TranslationDatabase.ReloadGuideTranslations();
        }

Note - the following should probably only ever be done when language changes?

            RoadUI.Instance.ReloadTexturesWithTranslation();
            TrafficLightTextures.Instance.ReloadTexturesWithTranslation();
            TMPELifecycle.Instance.TranslationDatabase.ReloadTutorialTranslations();
            TMPELifecycle.Instance.TranslationDatabase.ReloadGuideTranslations();

@kvakvs does that make sense?

[krzychu124]

Yeah that second part should be called when language changes - I don't see why it's now/was every time 🤷‍♂️ I'll check that code 😉

[originalfoo]

IIRC it was doing that originally so I just kind of left it.

Here's the original code before I stared changing stuff (it used to live in Options.cs):

https://github.com/CitiesSkylinesMods/TMPE/blob/768fe7f578b0124f9b33d4866960fa8221d49f69/TLM/TLM/State/Options.cs#L127-L144

[krzychu124]

Yeah, I've checked original code. All good, but I'll test if it's really necessary. No need to waste time if not needed.

Anyways.... it's a race condition when it comes to issue with UI texture. I need to figure out a better solution

[originalfoo]

I think the original RebuildMenu() (and thus my slightly tweaked version) was just a blunt "make sure everything gets updated".

The only 2 times I'm aware of that locales and textures should get updated:

1. When road sign theme changes (except during deserialization)
2. When language changes (except during deserialization)
3. OnAfterLoadData() to ensure textures are updated post-deserialization (the textures should not be loaded prior to deserialisation IMO)

So we could put the texture reload in to a method and only call it when required - vague mockup (needs improvement):

internal static void ReloadLocalisations(bool languageChanged = false) {

    if (TMPELifecycle.Instance.Deserializing || ModUI.Instance == null) {
        Log._Debug("OptionsManager.ReloadTextures() - Ignoring; Deserialising or ModUI.Instance is null");
        return;
    }

    // relaod whenever language code or road theme changes
    RoadUI.Instance.ReloadTexturesWithTranslation();
    TrafficLightTextures.Instance.ReloadTexturesWithTranslation();

    // only reload if language code changed
    if (languageChanged) {
        TMPELifecycle.Instance.TranslationDatabase.ReloadTutorialTranslations();
        TMPELifecycle.Instance.TranslationDatabase.ReloadGuideTranslations();
    }
}

[krzychu124]

I'm testing this one:

[krzychu124]

Ok, above code works good enough but there is a regression with regards to priority signs textures. I think we'll need new issue 😕 @aubergine10 @kvakvs I noticed that Chinese version of priority signs is never displayed despite we load textures when changing language. When we should display those modified version if we also have themes (but no Chinese one)?

[kvakvs]

I noticed that Chinese version of priority signs is never displayed despite we load textures when changing language. When we should display those modified version if we also have themes (but no Chinese one)?

I do not understand, there is no special handling of chinese signs in my theme code. But there is special handling of chinese TTL icons, where special versions of TTL icons for Chinese are loaded. Please make an issue with a screenshot.

[originalfoo]

Note also that there's two variants of Chinese - zh-cn and zh-tw IIRC? Simplified, and Traditional.

[kvakvs]

Locale is a combination of ISO language code (ZH for chinese) and country code where it is used (CN is for mainland China and TW for Taiwan, there's also SG for Singapore and HK for Hong Kong which we do not see in the game). Don't ask me how ZH means Chinese, probably some hieroglyph meaning China or Chinese transliterates to that and it became accepted by the ISO. Also this means most common Mandarin Chinese but there are a few more variations https://iso639-3.sil.org/code/zho

[originalfoo]

If lang is zh-cn (for example) but the filename is just zh would that cause issue? Maybe there's some code that trims anything from - so zh-cn becomes zh?

[krzychu124]

Files are loaded correctly, we just don't use them anymore because of introduced themes. There is even [Obsolete] label above both fields holding refs to loaded textures. Everything was "working" before introducing Speed limit themes but now we just need either do an exception for the rule we have or better build a basic theme for Chinese.

[kvakvs]

I do not think we should localize the signs when a user playing Chinese C:S would switch to another theme. Can we merge these signs or reuse from the existing theme engine? What's exactly not working?

[krzychu124]

I already explained. Yield and Stop signs as localized Chinese version -> https://github.com/CitiesSkylinesMods/TMPE/pull/1081 I was pretty simple implementation -> when selected Chinese lang, swap signs. Now it does not work since we only have themes and there is no Chinese one. Part for localized Traffic Lights buttons from mentioned PR still works since we didn't touch that.

[kvakvs]

I don't mind adding Chinese signs or several themes for different lands speaking Chinese. What you think?

[krzychu124]

If we find textures why not. The more complete set of textures the better.