[Kde-games-devel] Review Request 110446: This patch allows the player to resize the window. The playarea gets adjusted accordingly. This is very convenient for players using high resolution deplay.

[Ian Wadham]

On 16/05/2013, at 7:24 AM, Anant Pushkar wrote:
> On May 15th, 2013, 8:38 p.m. UTC, Roney Gomes wrote:
> 
> mainwindow.cpp (Diff revision 1)
> 102	
>     m_main->m_size-=33;
> Please, try to make it a constant. 33 by itself has absolutely no meaning.
> 
> That's why Albert had to ask.
> 
> On May 15th, 2013, 8:44 p.m. UTC, Albert Astals Cid wrote:
> 
> A constant would be better, but I don't think a it is correct, from your description it seems to me that this is probably style dependant. Also the old code doesn't seem to need this, why do you need it?
> On May 15th, 2013, 9:16 p.m. UTC, Roney Gomes wrote:
> 
> Yes. In the end it's all a matter of style. But in this is case it's a good one.
> 
> A constant would help future developers to maintain the code, making it clear what's the purpose of that magic number. Also, if he wants to stop the scroll bars from appearing he could have used QGraphicsView's methods:
> 
> setHorizontalScrollBarPolicy(Qt::ScrollBarAlwaysOff);
> setVerticalScrollBarPolicy(Qt::ScrollBarAlwaysOff);
> 
> Its not about just the scrollbars. Earlier QLayout::SetFixedSize was used (at line 51 , mainwindow.cpp) that set the size to that given by sizeHint() and did not allow any further resizing. So, the earlier code never needed any such hard coded thresholding, but now that this patch allows resizing, we need to change the size of the basic layout itself otherwise things start to look weird. Anyways I have added a constant. Hope this works fine.

Please excuse me replying on the list.  I can't get review 110446 to work online.

You should need no magic number 33, symbolic or otherwise, to avoid the
status bar.  In any case, the size of the status bar could change in future …
And that is not under control of application writers.

The setCentralWidget(); call to the main window is telling it to size your
view widget to fit the main window automatically as main menu, status bar,
etc. are added or as the main window is resized.

To avoid clipping in games such as this, you need to make your QGraphicsView
fit the scene, the whole scene and nothing but the scene (your QGraphicsScene
object, that is).  Many games turn off scrollbars and use the same pixel-based
co-ordinate system and origin for both the view and the scene, then keep changing
the scene co-ordinates as the size of the view changes (as seen in resizeEvent()).

Have a look at the code of some other games, such as KBounce, to see how it
can be done.

All the best, Ian W.