diff --git a/data/pref.xml b/data/pref.xml
index b4b82c31d..16f1110f7 100644
--- a/data/pref.xml
+++ b/data/pref.xml
@@ -429,6 +429,9 @@
+
diff --git a/data/strings/en.ini b/data/strings/en.ini
index 66ad96ebb..d29c47d1b 100644
--- a/data/strings/en.ini
+++ b/data/strings/en.ini
@@ -55,6 +55,13 @@ Automatic Remap
||&OK||&Cancel"
END
cannot_delete_all_layers = Error< Export
overwrite_files_on_export_sprite_sheet_alert = Show warning when overwriting files on Export Sprite Sheet
+delete_tilemap_delete_unused_tileset_alert = Show warning when deleting a tilemap will delete its unused tileset
image_format_alerts = Show options when saving files:
advanced_mode_alert = Show alert when we enter to Advanced Mode
invalid_fg_bg_color_alert = Show alert when drawing with index out of palette bounds
diff --git a/data/widgets/options.xml b/data/widgets/options.xml
index 9b7da391c..4e0504821 100644
--- a/data/widgets/options.xml
+++ b/data/widgets/options.xml
@@ -458,6 +458,8 @@
pref="export_file.show_overwrite_files_alert" />
+
tilesets()->add(tileset);
else
- sprite->tilesets()->set(m_tilesetIndex, tileset);
+ sprite->tilesets()->add(m_tilesetIndex, tileset);
sprite->incrementVersion();
sprite->tilesets()->incrementVersion();
diff --git a/src/app/commands/cmd_remove_layer.cpp b/src/app/commands/cmd_remove_layer.cpp
index 6e4944fc0..bbced5df3 100644
--- a/src/app/commands/cmd_remove_layer.cpp
+++ b/src/app/commands/cmd_remove_layer.cpp
@@ -11,20 +11,77 @@
#include "app/app.h"
#include "app/commands/command.h"
+#include "app/cmd/remove_tileset.h"
#include "app/context_access.h"
#include "app/doc_api.h"
#include "app/i18n/strings.h"
#include "app/modules/gui.h"
#include "app/tx.h"
+#include "app/pref/preferences.h"
+#include "app/ui/optional_alert.h"
#include "app/ui/status_bar.h"
#include "doc/layer.h"
+#include "doc/layer_tilemap.h"
#include "doc/sprite.h"
+#include "doc/tilesets.h"
#include "fmt/format.h"
#include "ui/alert.h"
#include "ui/widget.h"
namespace app {
+// Calculates the list of unused tileset indexes (returned in tsiToDelete parameter)
+// once the layers specified are removed.
+// Also, if the UI is available, shows a warning message about the deletion of unused
+// tilesets.
+// This function returns true in any of the following:
+// - There won't be deletion of tilesets, this means tsiToDelete is empty.
+// - The user accepts continuing despite the warning.
+// - There is no UI available.
+static bool continue_deleting_unused_tilesets(
+ Context* ctx, Sprite* sprite, const LayerList layers,
+ std::set>& tsiToDelete)
+{
+ std::vector tilemaps;
+ std::map timesSelected;
+ std::string layerNames;
+ for (auto layer : layers) {
+ if (layer->isTilemap()) {
+ auto tilemap = static_cast(layer);
+ timesSelected[tilemap->tilesetIndex()]++;
+ tilemaps.push_back(tilemap);
+ }
+ }
+ for (auto tilemap : tilemaps) {
+ auto ts = sprite->tilesets()->get(tilemap->tilesetIndex());
+ if (ts->tilemapsCount() == timesSelected[tilemap->tilesetIndex()]) {
+ tsiToDelete.insert(tilemap->tilesetIndex());
+ layerNames += tilemap->name() + ", ";
+ }
+ }
+
+#ifdef ENABLE_UI
+ // Just continue if UI is not available.
+ if (!ctx->isUIAvailable())
+ return true;
+
+ // Remove last ", "
+ if (!layerNames.empty()) {
+ layerNames = layerNames.substr(0, layerNames.length() - 2);
+ }
+
+ std::string message;
+ if (tsiToDelete.size() >= 1)
+ message = fmt::format(Strings::alerts_deleting_tilemaps_will_delete_tilesets(), layerNames);
+
+ return tsiToDelete.empty() ||
+ app::OptionalAlert::show(
+ Preferences::instance().tilemap.showDeleteUnusedTilesetAlert, 1, message) == 1;
+#else
+ return true;
+#endif
+}
+
class RemoveLayerCommand : public Command {
public:
RemoveLayerCommand();
@@ -55,6 +112,11 @@ void RemoveLayerCommand::onExecute(Context* context)
{
Tx tx(writer.context(), "Remove Layer");
DocApi api = document->getApi(tx);
+ // We need to remove all the tilesets after the tilemaps are deleted
+ // and in descending tileset index order, otherwise the tileset indexes
+ // get mixed up. This is the reason we use a tileset_index set with
+ // the std::greater Compare.
+ std::set> tsiToDelete;
const Site* site = writer.site();
if (site->inTimeline() &&
@@ -73,6 +135,10 @@ void RemoveLayerCommand::onExecute(Context* context)
return;
}
+ if (!continue_deleting_unused_tilesets(context, sprite, selLayers.toAllTilemaps(), tsiToDelete)) {
+ return;
+ }
+
for (Layer* layer : selLayers) {
api.removeLayer(layer);
}
@@ -84,10 +150,20 @@ void RemoveLayerCommand::onExecute(Context* context)
}
Layer* layer = writer.layer();
+ if (layer->isTilemap() && !continue_deleting_unused_tilesets(context, sprite, {layer}, tsiToDelete)) {
+ return;
+ }
+
layerName = layer->name();
api.removeLayer(layer);
}
+ if (!tsiToDelete.empty()) {
+ for (tileset_index tsi : tsiToDelete) {
+ tx(new cmd::RemoveTileset(sprite, tsi));
+ }
+ }
+
tx.commit();
}
diff --git a/src/doc/layer.cpp b/src/doc/layer.cpp
index 8bbf61014..734b93c43 100644
--- a/src/doc/layer.cpp
+++ b/src/doc/layer.cpp
@@ -529,6 +529,17 @@ void LayerGroup::allBrowsableLayers(LayerList& list) const
}
}
+void LayerGroup::allTilemaps(LayerList& list) const
+{
+ for (Layer* child : m_layers) {
+ if (child->isGroup())
+ static_cast(child)->allTilemaps(list);
+
+ if (child->isTilemap())
+ list.push_back(child);
+ }
+}
+
void LayerGroup::getCels(CelList& cels) const
{
for (const Layer* layer : m_layers)
diff --git a/src/doc/layer.h b/src/doc/layer.h
index b8e153f3d..dc291673f 100644
--- a/src/doc/layer.h
+++ b/src/doc/layer.h
@@ -211,6 +211,7 @@ namespace doc {
void allVisibleLayers(LayerList& list) const;
void allVisibleReferenceLayers(LayerList& list) const;
void allBrowsableLayers(LayerList& list) const;
+ void allTilemaps(LayerList& list) const;
void getCels(CelList& cels) const override;
void displaceFrames(frame_t fromThis, frame_t delta) override;
diff --git a/src/doc/selected_layers.cpp b/src/doc/selected_layers.cpp
index 3fa67d3d0..567b40162 100644
--- a/src/doc/selected_layers.cpp
+++ b/src/doc/selected_layers.cpp
@@ -99,6 +99,24 @@ LayerList SelectedLayers::toBrowsableLayerList() const
return output;
}
+LayerList SelectedLayers::toAllTilemaps() const
+{
+ LayerList output;
+
+ if (empty())
+ return output;
+
+ for (Layer* layer : *this) {
+ if (layer->isGroup()) {
+ auto group = static_cast(layer);
+ group->allTilemaps(output);
+ } else if (layer->isTilemap())
+ output.push_back(layer);
+ }
+
+ return output;
+}
+
void SelectedLayers::removeChildrenIfParentIsSelected()
{
SelectedLayers removeThese;
diff --git a/src/doc/selected_layers.h b/src/doc/selected_layers.h
index 81228c959..92bcf4298 100644
--- a/src/doc/selected_layers.h
+++ b/src/doc/selected_layers.h
@@ -41,6 +41,7 @@ namespace doc {
bool hasSameParent() const;
LayerList toBrowsableLayerList() const;
LayerList toAllLayersList() const;
+ LayerList toAllTilemaps() const;
void removeChildrenIfParentIsSelected();
void expandCollapsedGroups();
diff --git a/src/doc/sprite.cpp b/src/doc/sprite.cpp
index 0f7ede387..68623fdb6 100644
--- a/src/doc/sprite.cpp
+++ b/src/doc/sprite.cpp
@@ -736,6 +736,13 @@ LayerList Sprite::allBrowsableLayers() const
return list;
}
+LayerList Sprite::allTilemaps() const
+{
+ LayerList list;
+ m_root->allTilemaps(list);
+ return list;
+}
+
CelsRange Sprite::cels() const
{
SelectedFrames selFrames;
diff --git a/src/doc/sprite.h b/src/doc/sprite.h
index a91d7fde3..12efdf741 100644
--- a/src/doc/sprite.h
+++ b/src/doc/sprite.h
@@ -209,6 +209,7 @@ namespace doc {
LayerList allVisibleLayers() const;
LayerList allVisibleReferenceLayers() const;
LayerList allBrowsableLayers() const;
+ LayerList allTilemaps() const;
CelsRange cels() const;
CelsRange cels(frame_t frame) const;
diff --git a/src/doc/tileset.cpp b/src/doc/tileset.cpp
index ee0f035fe..e93a3717b 100644
--- a/src/doc/tileset.cpp
+++ b/src/doc/tileset.cpp
@@ -10,6 +10,9 @@
#include "doc/tileset.h"
+#include "doc/tilesets.h"
+#include "doc/layer.h"
+#include "doc/layer_tilemap.h"
#include "base/mem_utils.h"
#include "doc/primitives.h"
#include "doc/remap.h"
@@ -406,4 +409,15 @@ TilesetHashTable& Tileset::hashTable()
return m_hash;
}
+int Tileset::tilemapsCount() const {
+ auto tsi = sprite()->tilesets()->getIndex(this);
+ int count = 0;
+ for (auto layer : sprite()->allLayers()) {
+ if (layer->isTilemap() && static_cast(layer)->tilesetIndex() == tsi) {
+ count++;
+ }
+ }
+ return count;
+}
+
} // namespace doc
diff --git a/src/doc/tileset.h b/src/doc/tileset.h
index 58f656006..3be9a8eed 100644
--- a/src/doc/tileset.h
+++ b/src/doc/tileset.h
@@ -131,6 +131,9 @@ namespace doc {
// have to regenerate the empty tile with that new mask color.
void notifyRegenerateEmptyTile();
+ // Returns the number of tilemap layers that are referencing this tileset.
+ int tilemapsCount() const;
+
#ifdef _DEBUG
void assertValidHashTable();
#endif
diff --git a/src/doc/tilesets.h b/src/doc/tilesets.h
index 4155a70a0..355128cbd 100644
--- a/src/doc/tilesets.h
+++ b/src/doc/tilesets.h
@@ -8,6 +8,8 @@
#define DOC_TILESETS_H_INCLUDED
#pragma once
+#include "doc/layer_tilemap.h"
+#include "doc/sprite.h"
#include "doc/tileset.h"
#include
@@ -40,7 +42,7 @@ namespace doc {
return nullptr;
}
- tileset_index getIndex(Tileset *tileset) {
+ tileset_index getIndex(const Tileset *tileset) {
for (tileset_index i = 0; i < size(); ++i) {
if (m_tilesets[i] == tileset) {
return i;
@@ -55,23 +57,44 @@ namespace doc {
m_tilesets[tsi] = tileset;
}
+ void add(const tileset_index tsi, Tileset* tileset) {
+ if (tsi >= m_tilesets.size()) {
+ m_tilesets.push_back(tileset);
+ }
+ else {
+ m_tilesets.insert(m_tilesets.begin()+tsi, tileset);
+ // Update tileset indexes of the affected tilemaps. We have to shift the indexes
+ // for all the tilemaps pointing to a tileset index equals or greater than the added one.
+ shiftTilesetIndexes(tileset->sprite(), tsi, 1);
+ }
+ }
+
void erase(const tileset_index tsi) {
- // Do not m_tilesets.erase() the tileset so other tilesets
- // indexes/IDs are kept intact.
+ // When tsi is the last one, other tilemaps tilesets
+ // indexes are not affected.
if (tsi == size()-1) {
m_tilesets.erase(--m_tilesets.end());
}
else {
- // TODO Should we keep the empty slot? Or should we update all
- // indexes (even from external files?). Having a nullptr
- // tileset in the sprite adds a lot of complexity (each
- // for-loop must check the tileset)
- m_tilesets[tsi] = nullptr;
+ auto ts = m_tilesets[tsi];
+ m_tilesets.erase(m_tilesets.begin()+tsi);
+ // Update tileset indexes of the affected tilemaps. We have to shift the indexes
+ // for all the tilemaps pointing to a tileset index greater than the deleted one.
+ shiftTilesetIndexes(ts->sprite(), tsi+1, -1);
}
}
private:
Array m_tilesets;
+
+ void shiftTilesetIndexes(Sprite *sprite, tileset_index pos, int n) {
+ for (auto layer : sprite->allTilemaps()) {
+ auto tilemap = static_cast(layer);
+ if (tilemap->tilesetIndex() >= pos) {
+ tilemap->setTilesetIndex(tilemap->tilesetIndex()+n);
+ }
+ }
+ }
};
} // namespace doc
diff --git a/tests/scripts/tilemap.lua b/tests/scripts/tilemap.lua
index babf48467..af1acec37 100644
--- a/tests/scripts/tilemap.lua
+++ b/tests/scripts/tilemap.lua
@@ -1117,3 +1117,113 @@ do
expect_eq(app.activeLayer.cels[1].image.width, 1) -- width in tilemap terms
expect_eq(app.activeLayer.cels[1].image.height, 2) -- height in tilemap terms
end
+
+----------------------------------------------------------------------
+-- Tests removal of unused tilesets when deleting tilemaps
+----------------------------------------------------------------------
+
+do
+ local spr = Sprite(32, 32, ColorMode.INDEXED)
+ assert(spr.layers[1].isImage)
+ assert(not spr.layers[1].isTilemap)
+
+ -- Create some tilemaps
+ app.command.NewLayer{ tilemap=true }
+ assert(#spr.layers == 2)
+ local tilemapLay1 = spr.layers[2]
+ assert(tilemapLay1.isImage)
+ assert(tilemapLay1.isTilemap)
+ assert(#spr.tilesets == 1)
+ assert(spr.tilesets[1] == tilemapLay1.tileset)
+
+ app.command.NewLayer{ tilemap=true }
+ assert(#spr.layers == 3)
+ local tilemapLay2 = spr.layers[3]
+ assert(tilemapLay2.isImage)
+ assert(tilemapLay2.isTilemap)
+ assert(#spr.tilesets == 2)
+ assert(spr.tilesets[2] == tilemapLay2.tileset)
+
+ app.command.NewLayer{ tilemap=true }
+ assert(#spr.layers == 4)
+ local tilemapLay3 = spr.layers[4]
+ assert(tilemapLay3.isImage)
+ assert(tilemapLay3.isTilemap)
+ assert(#spr.tilesets == 3)
+ assert(spr.tilesets[3] == tilemapLay3.tileset)
+
+ -- Remove tilemap 2 and check that a tilemap was removed and
+ -- tilesets of remaining tilemaps are correct.
+ app.range.layers = { tilemapLay2 }
+ app.command.RemoveLayer()
+ assert(#spr.layers == 3)
+ assert(#spr.tilesets == 2)
+ assert(spr.tilesets[1] == tilemapLay1.tileset)
+ assert(spr.tilesets[2] == tilemapLay3.tileset)
+
+ -- Undo tilemap removal and check that it goes back to
+ -- previous state.
+ app.undo()
+
+ assert(#spr.layers == 4)
+ assert(#spr.tilesets == 3)
+ assert(spr.tilesets[1] == tilemapLay1.tileset)
+ assert(spr.tilesets[2] == tilemapLay2.tileset)
+ assert(spr.tilesets[3] == tilemapLay3.tileset)
+
+ -- Try removing 2 tilemaps now
+ app.range.layers = { tilemapLay1, tilemapLay2 }
+ app.command.RemoveLayer()
+ assert(#spr.layers == 2)
+ assert(#spr.tilesets == 1)
+ assert(spr.tilesets[1] == tilemapLay3.tileset)
+
+ app.undo()
+
+ assert(#spr.layers == 4)
+ assert(#spr.tilesets == 3)
+ assert(spr.tilesets[1] == tilemapLay1.tileset)
+ assert(spr.tilesets[2] == tilemapLay2.tileset)
+ assert(spr.tilesets[3] == tilemapLay3.tileset)
+
+ -- Assign same tileset to tilemap 1 and tilemap 3.
+ local oldTilemapLay3Tileset = tilemapLay3.tileset
+ tilemapLay3.tileset = tilemapLay1.tileset
+ -- We have to manually delete tilemap 3 tileset because
+ -- assigning a different tileset doesn't check for/remove
+ -- unused tilesets (TODO: should we add this?)
+ spr:deleteTileset(oldTilemapLay3Tileset)
+
+ assert(#spr.tilesets == 2)
+ assert(spr.tilesets[1] == tilemapLay1.tileset)
+ assert(spr.tilesets[2] == tilemapLay2.tileset)
+ assert(spr.tilesets[1] == tilemapLay3.tileset)
+
+ -- Remove tilemap 1 and check that no tileset was removed.
+ app.range.layers = { tilemapLay1 }
+ app.command.RemoveLayer()
+ assert(#spr.layers == 3)
+ assert(#spr.tilesets == 2)
+ assert(spr.tilesets[2] == tilemapLay2.tileset)
+ assert(spr.tilesets[1] == tilemapLay3.tileset)
+
+ -- Remove tilemap 3 and check that the tileset was removed now.
+ app.range.layers = { tilemapLay3 }
+ app.command.RemoveLayer()
+ assert(#spr.layers == 2)
+ assert(#spr.tilesets == 1)
+ assert(spr.tilesets[1] == tilemapLay2.tileset)
+
+ -- Undo all
+ app.undo()
+ app.undo()
+ app.undo()
+ -- Manually re-assign its tileset to tilemap 3.
+ tilemapLay3.tileset = spr.tilesets[3]
+
+ assert(#spr.layers == 4)
+ assert(#spr.tilesets == 3)
+ assert(spr.tilesets[1] == tilemapLay1.tileset)
+ assert(spr.tilesets[2] == tilemapLay2.tileset)
+ assert(spr.tilesets[3] == tilemapLay3.tileset)
+end
\ No newline at end of file