Suspect comparision(?) in TRoom.cpp

Post Reply
User avatar
SlySven
Posts: 1019
Joined: Mon Mar 04, 2013 3:40 pm
Location: Deepest Wiltshire, UK
Discord: SlySven#2703

Suspect comparision(?) in TRoom.cpp

Post by SlySven »

I note on line 348 of the current code there are multiple comparisons which throw up multiple GCC warnings: "warning: suggest parentheses around comparison in operand of ‘==’" what are these tests supposed to be establishing - is it equality of all four variables because if I recall C/C++ expression evaluation correctly non-equal values such as min_x = -5, min_y = -4, max_x=0, max_y=1 will still cause the overall expression to be evaluated as true?

Code: Select all

void TRoom::calcRoomDimensions()
{
    QMapIterator<QString, QList<QPointF> > it(customLines);
    while( it.hasNext() )
    {
        it.next();
        const QString & _e = it.key();
        const QList<QPointF> & _pL= it.value();
        if( _pL.size() < 1 ) continue;
        if( min_x == min_y == max_x == max_y == 1 )
        {
            min_x = _pL[0].x();
            max_x = min_x;
            min_y = _pL[0].y();
            max_y = min_y;
        }
        for( int i=0; i<_pL.size(); i++ )
        {
            qreal _x = _pL[i].x();
            qreal _y = _pL[i].y();
            if( min_x > _x )
                min_x = _x;
            if( max_x < _x )
                max_x = _x;
            if( min_y > _y )
                min_y = _y;
            if( max_y < _y )
                max_y = _y;
        }
        qDebug()<<"custom lines span: x("<<min_x<<"/"<<max_x<<") y("<<min_y<<"/"<<max_y<<")";
    }
}
That being the case wouldn't it be better to use:

Code: Select all

        if( min_x == max_y && min_y == max_y && min_x == min_y )
?

User avatar
Heiko
Site Admin
Posts: 1548
Joined: Wed Mar 11, 2009 6:26 pm

Re: Suspect comparision(?) in TRoom.cpp

Post by Heiko »

SlySven wrote: wouldn't it be better to use:

Code: Select all

        if( min_x == max_y && min_y == max_y && min_x == min_y )
?
No, that's a very different meaning. It's good that you bring up custom lines to the agenda because custom lines definitely needs some love. I put this on my todo list.

User avatar
SlySven
Posts: 1019
Joined: Mon Mar 04, 2013 3:40 pm
Location: Deepest Wiltshire, UK
Discord: SlySven#2703

Re: Suspect comparision(?) in TRoom.cpp

Post by SlySven »

OK so that bit of code needs some lovin' but what IS it testing for? I was thinking some sort detection that the four variables needed initialisation - and its always good that variables get initialised as I heard that all sorts of weirdness results when uninitialised ones get used... :?

User avatar
Vadi
Posts: 5035
Joined: Sat Mar 14, 2009 3:13 pm

Re: Suspect comparision(?) in TRoom.cpp

Post by Vadi »

I think you're looking at old code, this was removed in the last commit (https://sourceforge.net/p/mudlet/code/c ... fb941eade5) and isn't there anymore: https://sourceforge.net/p/mudlet/code/c ... m.cpp#l281

User avatar
Heiko
Site Admin
Posts: 1548
Joined: Wed Mar 11, 2009 6:26 pm

Re: Suspect comparision(?) in TRoom.cpp

Post by Heiko »

The entire custom lines feature has never been completed and still needs a lot of work. It's on my todo list for the next release.

Post Reply