[Kde-games-devel] kspaceduel: porting to Graphics View

[Dirk]

Hi Dmitry,

thank you very much for taking a look at the code.

>> As animation support is removed from QGraphicsPixmapItem which seems to
>> be the replacement for QCanvasPixmap, I took some code from the "Ported
>> Asteroids Example"
>> http://doc.trolltech.com/4.2/graphicsview-portedasteroids.html
>> This code is GPL 2. kspaceduel is "GPL 2 or later". Is this a problem?
>> And is there maybe alrady existing code which meets better kspaceduel's
>> requirements?
>>     
> I think this is ok, since Qt's asteroids example was once taken from kdegames' 
> game "kasteroids" :-).
> Btw, I was the one who ported kasteroids to QGV, so you can look there for 
> similar QGV-based class. IIRC it is in kasteroids/sprite.h file and is named 
> KAstSprite. But IIRC it was created from same QGV porting example as you 
> did :-).
>
> Perhaps we should create some common class in libkdegames? Already several 
> games are using a variant of it. kasteroids, kbounce, now kspaceduel. Perhaps 
> some other, I don't remember.
>   
I really would appreciate this. Would probably remove a lot of 
duplicated code and makes porting easier.

>> In addition I encountered heavy performance problems when using a
>> QPixmap as background (field.setBackgroundBrush(QBrush(backgr));), so I
>> set the background to pure black. Maybe anyone has a solution for this?
>>     
> Yup. You should tell your view class to cache its background.
> Use QGraphicsView::setCacheMode( CacheBackground ) call for that.
>
> But you need to remember that if your background changes you must explicitly 
> call QGraphicsView::resetCachedContent() - it will repaint background then.
>   
Thank you, solves the problem.
>   
>> Another problem I encountered and couldn't solve by now is that you can
>> clicking on the view. If you do so, the arrow keys are "bound" to the
>> view and you can't navigate your ship anymore.
>>     
> Why? Are you using QGraphicsScene event handlers? If not, I think you should. 
> Perhaps this will involve some code redesign though.
>   
I will give it a try. Was just wondering why it worked with Q3Canvas.
> Here are some notes about your patch:
>
> Instead of using
>   
> QList<QGraphicsItem*>::Iterator it;
> for(it=unexact.begin(); it != unexact.end(); ++it) { ... }
> you can use:
> foreach( QGraphicsItem* sprite, unexact ) { ... }
>
> That looks more nicer IMO :).
> There are several for-loops in your patch that can use foreach instead of 
> iterators...
>   
One question: What to do when I have to do a cast on the iterator? For 
example:

QList<QGraphicsItem *> hitlist;
QList<QGraphicsItem *>::Iterator it;
for(it=hitlist.begin(); it != hitlist.end(); ++it) {
                  bullet=(BulletSprite*)(*it);

> Otherwise patch looks quite good for me! :-)
> ( Of course, I might have missed something - I'm not that experienced "patch 
> reader" :-) )
>
>   
Again, thanks for reviewing. I will wait until tomorrow before 
committing the patch.

Best regards,
Dirk