Skip to content
Permalink
Browse files

Some improvements to the move up/down buttons for objects

* Made sure "up" and "down" refer to the stacking order, so it makes sense
  for object layers using Manual rendering order. This does mean it no
  longer matches with the Objects dock display order, but I think that the
  display order needs to be reversed.

* Used the icons from the theme when available.

* Set tool tips and disable the actions when no objects are selected.

* Fixed strange reordering happening when some set of objects is already
  at the bottom or top. Just do nothing instead.

* Used RangeSet<int> to replace the sorting and reduce the number of
  ChangeMapObjectsOrder instances.

* Replaced a bunch of 'foreach' loops with C++11 for loops.
  • Loading branch information...
bjorn committed Nov 28, 2016
1 parent f943d7b commit 9ee3cf465126b5bd64ad0f629d2f81d4e00a37b1
@@ -33,8 +33,10 @@ ChangeMapObjectsOrder::ChangeMapObjectsOrder(MapDocument *mapDocument,
ObjectGroup *objectGroup,
int from,
int to,
int count)
: mMapDocument(mapDocument)
int count,
QUndoCommand *parent)
: QUndoCommand(parent)
, mMapDocument(mapDocument)
, mObjectGroup(objectGroup)
, mFrom(from)
, mTo(to)
@@ -38,7 +38,8 @@ class ChangeMapObjectsOrder : public QUndoCommand
ObjectGroup *objectGroup,
int from,
int to,
int count);
int count,
QUndoCommand *parent = nullptr);

void undo() override;
void redo() override;
@@ -44,6 +44,7 @@
#include "orthogonalrenderer.h"
#include "painttilelayer.h"
#include "pluginmanager.h"
#include "rangeset.h"
#include "resizemap.h"
#include "resizetilelayer.h"
#include "rotatemapobject.h"
@@ -347,7 +348,7 @@ void MapDocument::resizeMap(const QSize &size, const QPoint &offset, bool remove

// Remove objects that will fall outside of the map
if (removeObjects) {
foreach (MapObject *o, objectGroup->objects()) {
for (MapObject *o : objectGroup->objects()) {
if (!visibleIn(visibleArea, o, mRenderer)) {
mUndoStack->push(new RemoveMapObject(this, o));
} else {
@@ -416,7 +417,8 @@ void MapDocument::rotateSelectedObjects(RotateDirection direction)
mSelectedObjects.size()));

// TODO: Rotate them properly as a group
foreach (MapObject *mapObject, mSelectedObjects) {
const auto &selectedObjects = mSelectedObjects;
for (MapObject *mapObject : selectedObjects) {
const qreal oldRotation = mapObject->rotation();
qreal newRotation = oldRotation;

@@ -762,7 +764,8 @@ void MapDocument::unifyTilesets(Map *map)

if (!undoCommands.isEmpty()) {
mUndoStack->beginMacro(tr("Tileset Changes"));
foreach (QUndoCommand *command, undoCommands)
const auto &commands = undoCommands;
for (QUndoCommand *command : commands)
mUndoStack->push(command);
mUndoStack->endMacro();
}
@@ -917,7 +920,7 @@ void MapDocument::deselectObjects(const QList<MapObject *> &objects)
setCurrentObject(nullptr);

int removedCount = 0;
foreach (MapObject *object, objects)
for (MapObject *object : objects)
removedCount += mSelectedObjects.removeAll(object);

if (removedCount > 0)
@@ -972,7 +975,7 @@ void MapDocument::removeObjects(const QList<MapObject *> &objects)
return;

mUndoStack->beginMacro(tr("Remove %n Object(s)", "", objects.size()));
foreach (MapObject *mapObject, objects)
for (MapObject *mapObject : objects)
mUndoStack->push(new RemoveMapObject(this, mapObject));
mUndoStack->endMacro();
}
@@ -986,7 +989,7 @@ void MapDocument::moveObjectsToGroup(const QList<MapObject *> &objects,
mUndoStack->beginMacro(tr("Move %n Object(s) to Layer", "",
objects.size()));

foreach (MapObject *mapObject, objects) {
for (MapObject *mapObject : objects) {
if (mapObject->objectGroup() == objectGroup)
continue;

@@ -997,48 +1000,92 @@ void MapDocument::moveObjectsToGroup(const QList<MapObject *> &objects,
mUndoStack->endMacro();
}

static bool mapObjectIndexLessThan(MapObject *o1, MapObject *o2){
return o1->objectGroup()->objects().indexOf(o1) < o2->objectGroup()->objects().indexOf(o2);
}
typedef QMap<ObjectGroup*, RangeSet<int>> Ranges;
typedef QMapIterator<ObjectGroup*, RangeSet<int>> RangesIterator;

static Ranges computeRanges(const QList<MapObject *> &objects)
{
Ranges ranges;

static bool mapObjectIndexGreaterThan(MapObject *o1, MapObject *o2){
return o1->objectGroup()->objects().indexOf(o1) > o2->objectGroup()->objects().indexOf(o2);
for (MapObject *object : objects) {
ObjectGroup *group = object->objectGroup();
auto &set = ranges[group];
set.insert(group->objects().indexOf(object));
}

return ranges;
}

void MapDocument::moveObjectsUp(const QList<MapObject *> &objects)
{
if (objects.isEmpty())
return;

QList<MapObject *> objectsSorted = QList<MapObject *>(objects);
qSort(objectsSorted.begin(), objectsSorted.end(), mapObjectIndexLessThan);
const auto ranges = computeRanges(objects);

QScopedPointer<QUndoCommand> command(new QUndoCommand(tr("Move %n Object(s) Up",
"", objects.size())));

RangesIterator rangesIterator(ranges);
while (rangesIterator.hasNext()) {
rangesIterator.next();

ObjectGroup *group = rangesIterator.key();
const RangeSet<int> &rangeSet = rangesIterator.value();

const RangeSet<int>::Range it_begin = rangeSet.begin();
RangeSet<int>::Range it = rangeSet.end();
Q_ASSERT(it != it_begin);

do {
--it;

int from = it.first();
int count = it.length();
int to = from + count + 1;

if (to <= group->objectCount())
new ChangeMapObjectsOrder(this, group, from, to, count, command.data());

mUndoStack->beginMacro(tr("Move %n Object(s) index(ices) up", "", objects.size()));
foreach(MapObject *mapObject, objectsSorted) {
ObjectGroup *group = mapObject->objectGroup();
int index = group->objects().indexOf(mapObject);
if (index > 0)
mUndoStack->push(new ChangeMapObjectsOrder(this, group, index, index-1, 1));
} while (it != it_begin);
}
mUndoStack->endMacro();

if (command->childCount() > 0)
mUndoStack->push(command.take());
}

void MapDocument::moveObjectsDown(const QList<MapObject *> &objects)
{
if (objects.isEmpty())
return;

QList<MapObject *> objectsSorted = QList<MapObject *>(objects);
qSort(objectsSorted.begin(), objectsSorted.end(), mapObjectIndexGreaterThan);
QScopedPointer<QUndoCommand> command(new QUndoCommand(tr("Move %n Object(s) Down",
"", objects.size())));

RangesIterator rangesIterator(computeRanges(objects));
while (rangesIterator.hasNext()) {
rangesIterator.next();

ObjectGroup *group = rangesIterator.key();
const RangeSet<int> &rangeSet = rangesIterator.value();

RangeSet<int>::Range it = rangeSet.begin();
const RangeSet<int>::Range it_end = rangeSet.end();

for (; it != it_end; ++it) {
int from = it.first();

mUndoStack->beginMacro(tr("Move %n Object(s) index(ices) down", "", objects.size()));
foreach(MapObject *mapObject, objectsSorted) {
ObjectGroup *group = mapObject->objectGroup();
int index = group->objects().indexOf(mapObject);
if (index < group->objectCount() - 1)
mUndoStack->push(new ChangeMapObjectsOrder(this, group, index+1, index, 1));
if (from > 0) {
int to = from - 1;
int count = it.length();

new ChangeMapObjectsOrder(this, group, from, to, count, command.data());
}
}
}
mUndoStack->endMacro();

if (command->childCount() > 0)
mUndoStack->push(command.take());
}

void MapDocument::setProperty(Object *object,
@@ -56,8 +56,6 @@ ObjectsDock::ObjectsDock(QWidget *parent)
mActionObjectProperties = new QAction(this);
mActionObjectProperties->setIcon(QIcon(QLatin1String(":/images/16x16/document-properties.png")));

Utils::setThemeIcon(mActionObjectProperties, "document-properties");

connect(mActionObjectProperties, SIGNAL(triggered()), SLOT(objectProperties()));

MapDocumentActionHandler *handler = MapDocumentActionHandler::instance();
@@ -81,6 +79,10 @@ ObjectsDock::ObjectsDock(QWidget *parent)
mActionMoveDown = new QAction(this);
mActionMoveDown->setIcon(QIcon(QLatin1String(":/images/16x16/go-down.png")));

Utils::setThemeIcon(mActionObjectProperties, "document-properties");
Utils::setThemeIcon(mActionMoveUp, "go-up");
Utils::setThemeIcon(mActionMoveDown, "go-down");

QToolBar *toolBar = new QToolBar;
toolBar->setFloatable(false);
toolBar->setMovable(false);
@@ -111,8 +113,8 @@ ObjectsDock::ObjectsDock(QWidget *parent)
connect(DocumentManager::instance(), SIGNAL(documentAboutToClose(MapDocument*)),
SLOT(documentAboutToClose(MapDocument*)));

connect(mActionMoveUp, SIGNAL(triggered()), SLOT(moveObjectsUp()));
connect(mActionMoveDown, SIGNAL(triggered()), SLOT(moveObjectsDown()));
connect(mActionMoveUp, &QAction::triggered, this, &ObjectsDock::moveObjectsUp);
connect(mActionMoveDown, &QAction::triggered, this, &ObjectsDock::moveObjectsDown);
}

void ObjectsDock::moveObjectsUp()
@@ -165,27 +167,35 @@ void ObjectsDock::retranslateUi()

mActionNewLayer->setToolTip(tr("Add Object Layer"));
mActionObjectProperties->setToolTip(tr("Object Properties"));
mActionMoveUp->setToolTip(tr("Move Objects Up"));
mActionMoveDown->setToolTip(tr("Move Objects Down"));

updateActions();
}

void ObjectsDock::updateActions()
{
int count = mMapDocument ? mMapDocument->selectedObjects().count() : 0;
bool enabled = count > 0;
mActionObjectProperties->setEnabled(count == 1);

if (mMapDocument && (mMapDocument->map()->objectGroupCount() < 2))
enabled = false;
mActionMoveToGroup->setEnabled(enabled);
mActionMoveToGroup->setToolTip(tr("Move %n Object(s) to Layer", "", count));
int selectedObjectsCount = 0;
int objectGroupCount = 0;

if (mMapDocument) {
selectedObjectsCount = mMapDocument->selectedObjects().count();
objectGroupCount = mMapDocument->map()->objectGroupCount();
}

mActionObjectProperties->setEnabled(selectedObjectsCount > 0);
mActionMoveToGroup->setEnabled(selectedObjectsCount > 0 && objectGroupCount >= 2);
mActionMoveToGroup->setToolTip(tr("Move %n Object(s) to Layer", "", selectedObjectsCount));
mActionMoveUp->setEnabled(selectedObjectsCount > 0);
mActionMoveDown->setEnabled(selectedObjectsCount > 0);
}

void ObjectsDock::aboutToShowMoveToMenu()
{
mMoveToMenu->clear();

foreach (ObjectGroup *objectGroup, mMapDocument->map()->objectGroups()) {
const auto &objectGroups = mMapDocument->map()->objectGroups();
for (ObjectGroup *objectGroup : objectGroups) {
QAction *action = mMoveToMenu->addAction(objectGroup->name());
action->setData(QVariant::fromValue(objectGroup));
}
@@ -210,17 +220,19 @@ void ObjectsDock::objectProperties()
void ObjectsDock::saveExpandedGroups(MapDocument *mapDoc)
{
mExpandedGroups[mapDoc].clear();
foreach (ObjectGroup *og, mapDoc->map()->objectGroups()) {

const auto &objectGroups = mMapDocument->map()->objectGroups();
for (ObjectGroup *og : objectGroups) {
if (mObjectsView->isExpanded(mObjectsView->model()->index(og)))
mExpandedGroups[mapDoc].append(og);
}
}

void ObjectsDock::restoreExpandedGroups(MapDocument *mapDoc)
{
foreach (ObjectGroup *og, mExpandedGroups[mapDoc])
const auto objectGroups = mExpandedGroups.take(mapDoc);
for (ObjectGroup *og : objectGroups)
mObjectsView->setExpanded(mObjectsView->model()->index(og), true);
mExpandedGroups[mapDoc].clear();
}

void ObjectsDock::documentAboutToClose(MapDocument *mapDocument)
@@ -106,7 +106,7 @@ void RaiseLowerHelper::raiseToTop()
return;

RangeSet<int> ranges;
foreach (MapObjectItem *item, selectedItems)
for (MapObjectItem *item : selectedItems)
ranges.insert(static_cast<int>(item->zValue()));

// Iterate backwards over the ranges in order to keep the indexes valid
@@ -149,7 +149,7 @@ void RaiseLowerHelper::lowerToBottom()
return;

RangeSet<int> ranges;
foreach (MapObjectItem *item, selectedItems)
for (MapObjectItem *item : selectedItems)
ranges.insert(static_cast<int>(item->zValue()));

RangeSet<int>::Range it = ranges.begin();
@@ -184,7 +184,7 @@ ObjectGroup *RaiseLowerHelper::sameObjectGroup(const QSet<MapObjectItem *> &item
// All selected objects need to be in the same group
ObjectGroup *group = (*items.begin())->mapObject()->objectGroup();

foreach (const MapObjectItem *item, items)
for (const MapObjectItem *item : items)
if (item->mapObject()->objectGroup() != group)
return nullptr;

@@ -215,7 +215,7 @@ bool RaiseLowerHelper::initContext()

QPainterPath shape;

foreach (const MapObjectItem *item, selectedItems) {
for (const MapObjectItem *item : selectedItems) {
if (item->mapObject()->objectGroup() != mObjectGroup)
return false;

@@ -224,18 +224,18 @@ bool RaiseLowerHelper::initContext()

// The list of related items are all items from the same object group
// that share space with the selected items.
QList<QGraphicsItem*> items = mMapScene->items(shape,
Qt::IntersectsItemShape,
Qt::AscendingOrder);
const QList<QGraphicsItem*> items = mMapScene->items(shape,
Qt::IntersectsItemShape,
Qt::AscendingOrder);

foreach (QGraphicsItem *item, items) {
for (QGraphicsItem *item : items) {
if (MapObjectItem *mapObjectItem = dynamic_cast<MapObjectItem*>(item)) {
if (mapObjectItem->mapObject()->objectGroup() == mObjectGroup)
mRelatedObjects.append(mapObjectItem);
}
}

foreach (MapObjectItem *item, selectedItems) {
for (MapObjectItem *item : selectedItems) {
int index = mRelatedObjects.indexOf(item);
Q_ASSERT(index != -1);
mSelectionRanges.insert(index);
@@ -252,7 +252,7 @@ void RaiseLowerHelper::push(const QList<QUndoCommand*> &commands,

QUndoStack *undoStack = mMapDocument->undoStack();
undoStack->beginMacro(text);
foreach (QUndoCommand *command, commands)
for (QUndoCommand *command : commands)
undoStack->push(command);
undoStack->endMacro();
}

0 comments on commit 9ee3cf4

Please sign in to comment.
You can’t perform that action at this time.