Fix some compiler warnings
Needs ReviewPublic

Authored by yurchor on Thu, Nov 7, 7:31 PM.

Details

Reviewers
None
Group Reviewers
KDE Games
Summary

There are several logic mistakes in the current code. Compiler warns about them and this patch tries to fix tnem.

Test Plan

Compiles and installs, XMPP part was not tested.

Diff Detail

Repository
R413 KsirK
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 18678
Build 18696: arc lint + arc unit
yurchor created this revision.Thu, Nov 7, 7:31 PM
Restricted Application added a subscriber: kde-games-devel. · View Herald TranscriptThu, Nov 7, 7:31 PM
yurchor requested review of this revision.Thu, Nov 7, 7:31 PM
chehrlic added inline comments.
ksirk/GameLogic/gameautomaton.cpp
2200–2201

better use Q_FALLTHROUGH()

ksirk/GameLogic/goal.h
73

Custom copy ctor but default operator=() ? Doesn't look correct.

ksirkskineditor/onu.cpp
781

later pointSize() (int) is assigned to m_font.size - so why not change m_font.size to be an int?

yurchor marked 3 inline comments as done.Thu, Nov 7, 8:02 PM

Fixed. Thanks.

yurchor updated this revision to Diff 69409.Thu, Nov 7, 8:03 PM

Remove = operator, rise the Qt requirement to 5.8 to use Q_FALLTHROUGH(), use (int).

aacid added a subscriber: aacid.Sat, Nov 9, 7:58 PM

Honestly i'm not convinced touching the iris xmpp code that is a copy of something else is the best of the ideas.

Maybe we can disable the extra warnings for that specific folder?

What do others think?

nhirsl added a subscriber: nhirsl.Sun, Nov 10, 4:54 PM

Last time I tried playing ksirk over jabber network was during upgrade to Qt5. At that time it was defunct and Gael removed the label from the game selection panel. It looks like there's still a way to chose it trough menu.
Has that changed in the meantime?

Almost all compiler warnings are coming from this part of code. I tried to upgrade it to upstream version and test it, but since it was still defunct I couldn't verify the change.

There's also note (jabbergameui.ui): Playing over the Jabber network is an experimental feature in KsirK.

Not sure what's the right way to proceed:

  1. Drop support/code for playing over Jabber network (remove obsolete iris/xmpp code from ksirk)
  2. Upgrade to latest upstram (which resolves _all_ compiler warnings in that part of code)
  3. Keep it like this and fix here and there some compiler warnings
yurchor updated this revision to Diff 69542.Sun, Nov 10, 5:41 PM

Revert changes in iris until future decision

I agree with these changes.
Is it possible to disable warnings for particular folder and does that make sense at all (since the code is still there and compiling)?

I agree with these changes.

Thanks.

Is it possible to disable warnings for particular folder and does that make sense at all (since the code is still there and compiling)?

This does not make sense for me. These warnings cannot be seen by an ordinary user and they are instructive to see what can be done to enhance the code at the same time.