Adding Alphabetical sorting #145
Conversation
There was a problem hiding this comment.
Hi @lflo5727 !
It looks like you made your patch using a very old Celestia snapshot as it reverts some recent changes. Please rebase it to the current master.
Also you have style issues:
- We use 4 spaces to indent, tabs are disallowed.
- We don't use
elseafterif(...) retun ...
I've checked your commit and found that you have committed a lot of unneeded temporary files generated by qmake.
| @@ -62,20 +62,20 @@ class SolarSystemTreeModel : public QAbstractTableModel, public ModelHelper | |||
| }; | |||
|
|
|||
| void buildModel(Star* star, bool _groupByClass); | |||
|
|
|||
|
|
|||
| class SSTMPredicate | ||
| { | ||
| public: | ||
| enum Criterion |
There was a problem hiding this comment.
Tabs are not allowed, use 4 spaces.
|
|
||
| class SSTMPredicate | ||
| { | ||
| public: |
There was a problem hiding this comment.
Add 1 white space before public:/private:
| SolarSystemTreeModel::TreeItem::~TreeItem() | ||
| { | ||
| if (children != nullptr) | ||
| for (auto it = children.begin(); it != children.end(); ++it) |
| { | ||
| criterion = _criterion; |
There was a problem hiding this comment.
Use C++03 style members initializers here
SSTMPredicate::SSTMPredicate(Criterion _criterion, const Universe *_universe) :
criterion(_criterion),
universe(_universe)
{}
| else | ||
| { | ||
| return ""; | ||
| } |
There was a problem hiding this comment.
No need for braces here. Remove else so the the code becomes:
if ()
return
if ()
return
return
| switch(criterion) | ||
| { | ||
| case ObjectType: | ||
| return strcmp(objectTypeName(ti0->obj).toStdString().c_str(), objectTypeName(ti1->obj).toStdString().c_str()) < 0; |
There was a problem hiding this comment.
objectTypeName(ti0->obj) < objectTypeName(ti1->obj)
| return strcmp(objectTypeName(ti0->obj).toStdString().c_str(), objectTypeName(ti1->obj).toStdString().c_str()) < 0; | ||
|
|
||
| case Alphabetical: | ||
| return strcmp(this->getName(ti0->obj).c_str(), this->getName(ti1->obj).c_str()) < 0; |
There was a problem hiding this comment.
getName(ti0->obj) < getName(ti1->obj)
| SSTMFilterPredicate(); | ||
| bool operator()(const SolarSystemTreeModel::TreeItem* ti) const; | ||
|
|
||
| unsigned int objectTypeMask; |
There was a problem hiding this comment.
- Unused variable objectTypeMask please remove it
- As I see below it's RenderFlag, so uint64_t or better Renderer::RenderFlags should be used if you plan to implement it's usage. But Solar System Browser shouldn't use renderer flags to display objects.
|
@lflo5727 beside mentioned above, your code doesn't work properly. It changes positions of 1st level bodies, but ignores their children. |
|
No response for a half of a year so I'm closing this PR. |
Added sorting as per QT Celestial Browser sorting issue #7.