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
base: master
from

Conversation

2 participants
@theHacker
Contributor

theHacker commented Jan 19, 2015

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.

Show outdated Hide outdated src/tiled/swaptiles.cpp
@bjorn

This comment has been minimized.

Show comment
Hide comment
@bjorn

bjorn Feb 19, 2015

Owner

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.

Owner

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 referenced this pull request Feb 21, 2015

Closed

BucketErase / Magic Wand #128

@bjorn bjorn added 1 - Ready and removed 1 - Ready labels Feb 22, 2015

theHacker added some commits Feb 23, 2015

Implemented review notes
- Swapping tiles triggers emitRegionChanged() for redrawing
- "Swap tiles" -> "Swap Tiles". Added mnemonic as well in English.
- Refactoring: Extracted variable
@theHacker

This comment has been minimized.

Show comment
Hide comment
@theHacker

theHacker Feb 23, 2015

Contributor

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?

Contributor

theHacker commented Feb 23, 2015

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

This comment has been minimized.

Show comment
Hide comment
@bjorn

bjorn Mar 5, 2016

Owner

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.

Owner

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 label Mar 5, 2016

@bjorn bjorn self-assigned this Mar 5, 2016

@bjorn bjorn added the urgent label Jul 17, 2016

@bjorn bjorn added this to TODO in Roadmap Feb 13, 2017

@bjorn

This comment has been minimized.

Show comment
Hide comment
@bjorn

bjorn Feb 28, 2017

Owner

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.

Owner

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 bjorn moved this from Tiled 1.0 to Recently Completed in Roadmap Feb 28, 2017

@bjorn

This comment has been minimized.

Show comment
Hide comment
@bjorn

bjorn Feb 28, 2017

Owner

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

Owner

bjorn commented Feb 28, 2017

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment