Improved readability of QColor constructors#971
Conversation
Replaced the more obvious ones with enums.
|
I'm aware |
src/run-lua-code-v4.xml
Outdated
| <?xml version="1.0" encoding="UTF-8"?> | ||
| <!DOCTYPE MudletPackage> | ||
| <MudletPackage version="1.0"> | ||
| <TriggerPackage/> |
SlySven
left a comment
There was a problem hiding this comment.
I think we can go further and get rid of the QColor(...) part in some/many/all {delete as appropriate} places?
src/TLuaInterpreter.cpp
Outdated
| QColor blue = QColor(Qt::blue); | ||
| QColor green = QColor(Qt::green); | ||
| QColor red = QColor(Qt::red); | ||
| QColor black = QColor(Qt::black); |
There was a problem hiding this comment.
I think you do not need these locals - I suspect you can drop the Qt global constants directly into the printDebug(...) argument list.
There was a problem hiding this comment.
That's not possible to do. The function expects a QColor so the constants don't work, and QColor(<constant>) says expression must be lvalue.
src/T2DMap.cpp
Outdated
| _poly.append( _pt ); | ||
| QBrush brush = p.brush(); | ||
| brush.setColor( QColor(0, 0 ,0) ); | ||
| brush.setColor( QColor(Qt::black) ); |
There was a problem hiding this comment.
In some places I think you can drop the constant in without a QColor(...) around the outside - especially if a colour is expected (as a const reference perhaps?)
|
Fixed the unrelated change, merge conflicts and accidental changes when resolving the merge conflicts. |
|
@SlySven updated. |
ahmedcharles
left a comment
There was a problem hiding this comment.
QColor has an implicit constructor that accepts GlobalColor, which is the enum type used for the Qt:: values. This means that anywhere that accepts a QColor explicitly, should also accept a GlobalColor implicitly.
src/TLuaInterpreter.cpp
Outdated
| QColor black = QColor(0,0,0); | ||
| QColor green = QColor(Qt::green); | ||
| QColor blue = QColor(Qt::blue); | ||
| QColor black = QColor(Qt::black); |
There was a problem hiding this comment.
QColor green = Qt::green; should work just fine.
There was a problem hiding this comment.
Oh, I see.
I'll go with auto green = QColor(Qt::green); as it's pretty much the same thing but more consistent in style.
All of them are obvious QColors from the `= QColor()`
|
I guess I'm less of an |
|
Comes with my Lua background where |
|
Hehe, but lua doesn't have types for variables, only values. |
|
That's right. |
|
Linux-Cmake-GCC CI build failed for Travis Infrastructure reasons - have kicked that one build to try it again... |
Replaced the more obvious ones with enums.