D17560: fully port away from kdelibs4support
Albert Astals Cid
noreply at phabricator.kde.org
Thu Feb 21 18:15:02 GMT 2019
aacid added inline comments.
INLINE COMMENTS
> sitter wrote in toplevel.cpp:236
> 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
Not attached to any solution, just want one that works :)
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/20190221/285135f8/attachment.html>
More information about the kde-games-devel
mailing list