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.

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

Added functionality to swap tiles #866

Closed
wants to merge 5 commits into from

Conversation

theHacker
Copy link
Contributor

During development of my game I am frequently changing the graphics file of the tileset. It is pretty anonying that I destroy the map when moving around the tiles on the graphic. First I wrote a script to switch two ids in the map.

Because it is not very fun to look up ids which to switch, I decided that it would be nice if I can do this in Tiled directly. I found no such function... so I wrote one myself :-)
If exactly two tiles are selected you can selected "Swap tiles" in the contextmenu to swap these tiles on the current TileLayer.


const TilesetModel *model = tilesetModel();
Tile *tile1 = model->tileAt(selectionModel()->selectedIndexes()[0]);
Tile *tile2 = model->tileAt(selectionModel()->selectedIndexes()[1]);
Copy link
Member

Choose a reason for hiding this comment

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

You're retrieving the list of selected indexes three times in this function. Please put it in a local variable.

@bjorn
Copy link
Member

bjorn commented Feb 19, 2015

Overall a nice and clean patch! One question though, why did you decide to only swap the tiles for the current layer? This is going to be painful for people who want to swap the tiles globally and have many layers, which seems to be part of your main use-case.

Also, I guess it would make sense to support this on object layers for tile objects as well.

@bjorn bjorn mentioned this pull request Feb 21, 2015
@bjorn bjorn added 1 - Ready and removed 1 - Ready labels Feb 22, 2015
- Swapping tiles triggers emitRegionChanged() for redrawing
- "Swap tiles" -> "Swap Tiles". Added mnemonic as well in English.
- Refactoring: Extracted variable
@theHacker
Copy link
Contributor Author

Thx 😄

To answer your question why only for tile layers and the current layer... I never worked with Qt before and it was my first time reading Tiled's source code as well, so my goal was to change so few as possible to avoid breaking something. Because I only have exactly one layer (a tile layer) I stuck with currentLayer().

When doing this on all layers you would imagine that the tiles would be swapped on all layers - object layers and tile layers all at once?

@bjorn
Copy link
Member

bjorn commented Mar 5, 2016

When doing this on all layers you would imagine that the tiles would be swapped on all layers - object layers and tile layers all at once?

Sorry for the late answer, but yes that's what I would expect to happen.

In general, I'm a little concerned that this action will be mistaken for a way to rearrange the tileset itself, rather than modifying the map. But I'll look into giving this a try and merging it sometime soon.

@bjorn bjorn added the feature It's a feature, not a bug. label Mar 5, 2016
@bjorn bjorn self-assigned this Mar 5, 2016
@bjorn bjorn added the Urgent label Jul 17, 2016
@bjorn
Copy link
Member

bjorn commented Feb 28, 2017

Rebased on current master and pushed as 2c6cde6. The behavior is still the same (works on current tile layer only), but I'll change it to apply to the whole map.

@bjorn bjorn closed this Feb 28, 2017
@bjorn
Copy link
Member

bjorn commented Feb 28, 2017

I made the swap affect the entire map in change b52338c.

@tangorri
Copy link

tangorri commented Nov 3, 2018

Hi, interested by TileSet rearrangement, can't find any way to use these on interface

@bjorn
Copy link
Member

bjorn commented Nov 4, 2018

@tangorri Currently you can't modify the tileset image in Tiled. The change that was merged here is meant to help users who modified their tileset image externally, to adjust the maps they have already created.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature It's a feature, not a bug.
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants