Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix peripherals started before strings are loaded #12518

Merged
merged 1 commit into from
Jul 22, 2017

Conversation

garbear
Copy link
Member

@garbear garbear commented Jul 18, 2017

This fixes the following error:

screenshot000

How Has This Been Tested?

The labels show up now.

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

@tamland
Copy link
Member

tamland commented Jul 18, 2017

Why is this? Is something caching it? If the labels are translated when they are displayed this won't matter as they will get the correct one once the strings are loaded.

@garbear
Copy link
Member Author

garbear commented Jul 18, 2017

The labels are cached, but translating at runtime would work too.

@garbear
Copy link
Member Author

garbear commented Jul 18, 2017

Updated with @tamland's suggestion

@garbear
Copy link
Member Author

garbear commented Jul 20, 2017

Some users found more places that this update didn't address

peripheral settings

I've restored the original solution to address this and possibly other locations in peripherals that rely on language strings early on.

@garbear garbear added Type: Fix non-breaking change which fixes an issue v18 Leia labels Jul 20, 2017
@garbear garbear added this to the L 18.0-alpha1 milestone Jul 20, 2017
@garbear
Copy link
Member Author

garbear commented Jul 20, 2017

It's worth noting that initializing peripherals in stage 3 was what originally happened in #11824. I can't say if the move to stage 2 has other negative side effects besides strings breaking.

@FernetMenta
Copy link
Contributor

stage 3 is for components that require ui. this looks more like a work around but does not address the root cause.

@garbear
Copy link
Member Author

garbear commented Jul 21, 2017

A bunch of stuff happens between stages 2 and 3, and peripherals could depend on any number of it. I don't know the reason peripherals were originally brought up near stage 3, but restoring this behavior seems like the least intrusive change.

@quthla
Copy link

quthla commented Jul 21, 2017

Will CEC be fixed maybe?

@FernetMenta
Copy link
Contributor

@garbear still a long way to go with our globals, isn't? could you place a comment in the code with this info?

This was causing game.controller.default to have empty labels in the
controller window.
@garbear
Copy link
Member Author

garbear commented Jul 22, 2017

@FernetMenta added a comment, is that what you had in mind?

@FernetMenta
Copy link
Contributor

ok, thanks

@garbear
Copy link
Member Author

garbear commented Jul 22, 2017

Jenkins build this please

@tamland
Copy link
Member

tamland commented Jul 22, 2017

I've restored the original solution to address this and possibly other locations in peripherals that rely on language strings early on.

Still a really bad idea though. Unless you register callback for all these things, it breaks language switching and updating. If string translation is simply kept to the gui layer (and not cached a second time), it just works.

@garbear
Copy link
Member Author

garbear commented Jul 22, 2017

True, language switching will still be broken. Although in this case it's just add-on strings. I'll address this in a future PR. My main argument is that Peripherals doesn't need to be brought up so early, and indeed the change has already broken some stuff in the past, including causing a segfault in tests. Once this change is in I'll start to decouple peripherals from the GUI.

Jenkins build this please

@garbear garbear merged commit ff2fa3f into xbmc:master Jul 22, 2017
@garbear garbear deleted the fix-strings branch July 22, 2017 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Fix non-breaking change which fixes an issue v18 Leia
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants