D17560: fully port away from kdelibs4support

[Harald Sitter]

sitter planned changes to this revision.
sitter added inline comments.

INLINE COMMENTS

> aacid wrote in toplevel.cpp:236
> this is wrong, you moved the language.isEmpty up and now the else triggers in cases it should not trigger.

Hm. That is true. Truth be told even after pointing that out it still took me way to long to see the problem -.-

I think I wanted to avoid the extra level of nesting. With that motivation in mind, what do you think about splitting the if

  if (soundEnabled && language.isEmpty())
  {
    const QStringList &systemLanguages = KLocalizedString::languages();
    ....
  }
  
  if (!soundEnabled)
  {
    soundOff();
    language = QString();
  }

In my mind the two code blocks aren't two sides of the same coin. One is language fallback-handling the other is the unrelated disabling of all sound. The only thing that links them is the fact that the only thing language is used for is to figure out what sound set to use.

I don't mind particularly though, so if you'd like to have

  if (soundEnabled) {
    if (language.isEmpty()) { ... }
  } else { ... }

I'm fine with that too

REPOSITORY
  R418 KTuberling

REVISION DETAIL
  https://phabricator.kde.org/D17560

To: sitter, aacid, #kde_games
Cc: kde-games-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-games-devel/attachments/20190220/b8d25b44/attachment.html>