Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Selection of underlying objects #1494

Merged
merged 17 commits into from Mar 27, 2017
Merged

Selection of underlying objects #1494

Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
04678b5
Selection of underlying objects
Acuion Mar 16, 2017
c7c7863
Names in submenu
Acuion Mar 16, 2017
19e5c2a
Merge remote-tracking branch 'refs/remotes/bjorn/master'
Acuion Mar 16, 2017
37481ce
mouse movement without loss of order
Acuion Mar 17, 2017
e4a7d6a
Merge branch 'master' of https://github.com/Acuion/tiled
Acuion Mar 17, 2017
85a4260
Iterating is now Alt+click
Acuion Mar 17, 2017
874fc7e
Removed mCurrMouse...
Acuion Mar 17, 2017
cf1117f
codestyle
Acuion Mar 17, 2017
cea4d0a
Not my code, but a little refactoring
Acuion Mar 17, 2017
9c5dcfb
return of the return, lol
Acuion Mar 18, 2017
ff0a8ed
Rotation is now based on pointer, shift/ctrl support
Acuion Mar 18, 2017
5a0cf89
First element is now enabled
Acuion Mar 18, 2017
6eb96a3
find&erase->remove
Acuion Mar 18, 2017
344c0e9
nullptr check, qt-style finding, unselection opt
Acuion Mar 20, 2017
250125d
refactoring
Acuion Mar 20, 2017
5459383
Selected-based rotation
Acuion Mar 21, 2017
f1c2fa2
Object name and codestyle
Acuion Mar 22, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 14 additions & 0 deletions src/tiled/abstractobjecttool.cpp
View file
Open in desktop
Expand Up @@ -86,6 +86,7 @@ void AbstractObjectTool::keyPressed(QKeyEvent *event)
duplicateObjects();
return;
}
break;
}

event->ignore();
Expand Down Expand Up @@ -133,9 +134,22 @@ ObjectGroup *AbstractObjectTool::currentObjectGroup() const
return dynamic_cast<ObjectGroup*>(mapDocument()->currentLayer());
}

QList<MapObjectItem*> AbstractObjectTool::objectItemsAt(QPointF pos) const
{
const QList<QGraphicsItem *> &items = mMapScene->items(pos);

QList<MapObjectItem*> objectList;
for (auto item : items) {
if (MapObjectItem *objectItem = qgraphicsitem_cast<MapObjectItem*>(item))
objectList.append(objectItem);
}
return objectList;
}

MapObjectItem *AbstractObjectTool::topMostObjectItemAt(QPointF pos) const
{
const QList<QGraphicsItem *> &items = mMapScene->items(pos);

for (QGraphicsItem *item : items) {
if (MapObjectItem *objectItem = qgraphicsitem_cast<MapObjectItem*>(item))
return objectItem;
Expand Down
1 change: 1 addition & 0 deletions src/tiled/abstractobjecttool.h
View file
Open in desktop
Expand Up @@ -65,6 +65,7 @@ class AbstractObjectTool : public AbstractTool

MapScene *mapScene() const { return mMapScene; }
ObjectGroup *currentObjectGroup() const;
QList<MapObjectItem*> objectItemsAt(QPointF pos) const;
MapObjectItem *topMostObjectItemAt(QPointF pos) const;

private slots:
Expand Down
2 changes: 2 additions & 0 deletions src/tiled/mapobjectitem.h
View file
Open in desktop
Expand Up @@ -107,3 +107,5 @@ class MapObjectItem : public QGraphicsItem

} // namespace Internal
} // namespace Tiled

Q_DECLARE_METATYPE(Tiled::Internal::MapObjectItem*)
56 changes: 53 additions & 3 deletions src/tiled/objectselectiontool.cpp
View file
Open in desktop
Expand Up @@ -47,6 +47,7 @@
#include <QKeyEvent>
#include <QTransform>
#include <QUndoStack>
#include <QMenu>

#include <cmath>

Expand Down Expand Up @@ -533,6 +534,42 @@ void ObjectSelectionTool::mousePressed(QGraphicsSceneMouseEvent *event)

break;
}
case Qt::RightButton:
if (event->modifiers() & Qt::AltModifier) {
QList<MapObjectItem*> underlyingObjects = objectItemsAt(event->scenePos());
if (underlyingObjects.empty())
break;
QMenu selectUnderlyingMenu;

for (int levelNum = 0; levelNum < underlyingObjects.size(); ++levelNum) {
const QString& objectName = underlyingObjects[levelNum]->mapObject()->name();
QString actionName = QLatin1String(levelNum < 9 ? "&" : "") + tr("%n) ", "", levelNum + 1)
Copy link
Contributor Author

@Acuion Acuion Mar 21, 2017 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any better way to construct a "%d) " string in this case? I think it doesn't require any translation, so can be made without tr.
And how to properly update a .ts file? I can provide a russian translation for strings in my code (now it is only "Unnamed object")

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mostly just make it a translatable string anyway, because you never know (especially since some languages have different writing directions). But since this number does not relate to a plural form, you should not use the plural form override of tr.

I would probably just do:

if (objectName.isEmpty())
    objectName = tr("Unnamed object");

QString actionName;
if (levelNum < 9)
    actionName = tr("&%1) %2").arg(levelNum + 1).arg(objectName)
else
    actionName = tr("%1) %2").arg(levelNum + 1).arg(objectName)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, but what about a .ts file update? Can it be done manually or only via lupdate?

+ (objectName.isEmpty() ? tr("Unnamed object") : objectName);
QAction *action = selectUnderlyingMenu.addAction(actionName);
action->setData(QVariant::fromValue(underlyingObjects[levelNum]));
}

QAction *action = selectUnderlyingMenu.exec(event->screenPos());

if (!action)
break;

if (MapObjectItem* objectToBeSelected = action->data().value<MapObjectItem*>()) {
auto selection = mapScene()->selectedObjectItems();
if (event->modifiers() & (Qt::ShiftModifier | Qt::ControlModifier)) {
if (selection.contains(objectToBeSelected))
selection.remove(objectToBeSelected);
else
selection.insert(objectToBeSelected);
} else {
selection.clear();
selection.insert(objectToBeSelected);
}
mapScene()->setSelectedObjectItems(selection);
}
} else
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coding style: if some if/else chain has brackets somewhere, it should have them everywhere. Please add { here.

AbstractObjectTool::mousePressed(event);
break;
default:
AbstractObjectTool::mousePressed(event);
break;
Expand All @@ -551,11 +588,24 @@ void ObjectSelectionTool::mouseReleased(QGraphicsSceneMouseEvent *event)
break;
}
const Qt::KeyboardModifiers modifiers = event->modifiers();
QSet<MapObjectItem*> selection = mapScene()->selectedObjectItems();
if (modifiers & Qt::AltModifier) {
auto underlyingObjects = objectItemsAt(event->scenePos());
if (underlyingObjects.isEmpty())
break;

int lastSelectedIndex = -1;
for (auto selected : selection)
lastSelectedIndex = std::max(lastSelectedIndex, underlyingObjects.indexOf(selected));
do lastSelectedIndex = (lastSelectedIndex + 1) % underlyingObjects.size();
while (selection.contains(underlyingObjects.at(lastSelectedIndex))
&& lastSelectedIndex != underlyingObjects.size() - 1);
mClickedObjectItem = underlyingObjects.at(lastSelectedIndex);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was something wrong with the loop I wrote earlier? I think it was easier to understand and more efficient than this approach.

Copy link
Contributor Author

@Acuion Acuion Mar 22, 2017 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't support a last->first->second iteration. If the bottom part of the stack is Shift-selected, your code always point at the topmost element.

}
if (mClickedObjectItem) {
QSet<MapObjectItem*> selection = mapScene()->selectedObjectItems();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't use the selection anymore outside of this condition, so please move it back here.

if (modifiers & (Qt::ShiftModifier | Qt::ControlModifier)) {
if (selection.contains(mClickedObjectItem))
selection.remove(mClickedObjectItem);
if (!(modifiers & Qt::AltModifier) && selection.contains(mClickedObjectItem))
selection.remove(mClickedObjectItem);// Removal is not supported in alt+click mode
Copy link
Member

@bjorn bjorn Mar 21, 2017 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why you would block removal. I don't think there is a negative side-effect to it.

Copy link
Contributor Author

@Acuion Acuion Mar 22, 2017 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise, if the whole stack is selected, it select and unselect the same object in a cycle. I think it looks buggy

else
selection.insert(mClickedObjectItem);
mapScene()->setSelectedObjectItems(selection);
Expand Down