Fix "Detach templates" not working when export target is the same file#4466
Fix "Detach templates" not working when export target is the same file#4466Hijanhv wants to merge 1 commit into
Conversation
When exporting to the same file as the map/tileset, two issues caused the "Detach template instances" export option to stop working after the first export: 1. ExportHelper::prepareExportMap would clear exportFileName and exportFormat from the exported clone, causing the file on disk to lose the export settings. On the next reload (triggered by the file watcher), those settings would be gone and export-on-save would silently skip the export step. 2. The export write would update the file's timestamp but not mLastSaved, causing the file watcher to treat the export as an external change and auto-reload the document (which cleared the in-memory export settings). Fix both by: - Preserving exportFileName/exportFormat in the exported file when the export target and the map file are the same (canonically equal paths). - Calling refreshLastSaved() after a successful export to the same file so the file watcher ignores the export write and no unnecessary reload is triggered. Fixes mapeditor#4456
bjorn
left a comment
There was a problem hiding this comment.
I'm not sure we actually want to disable the reload, and the way to check for "self-export" is slightly broken as such. See inline comments.
| const auto canonicalMapFile = QFileInfo(map->fileName).canonicalFilePath(); | ||
| const auto canonicalExportFile = QFileInfo(map->exportFileName).canonicalFilePath(); | ||
| const bool isSelfExport = !canonicalMapFile.isEmpty() | ||
| && canonicalMapFile == canonicalExportFile; |
There was a problem hiding this comment.
I don't think this is the right way to do it, because it compares the previously used export file rather than the current export target. You'd need to pass the current export target into the prepareExportMap function.
| // When exporting to the same file, update the saved timestamp | ||
| // to avoid triggering an unnecessary auto-reload | ||
| if (exportFileName == document->fileName()) | ||
| document->refreshLastSaved(exportFileName); |
There was a problem hiding this comment.
This is a bit hacky and then even duplicated in 4 places.
I'm also not sure if the reload is really unnecessary when exporting to the file that is loaded? I guess when the user does this, they expect their export options (like detached templates) to affect their current file, and since the export options are only applied to a clone of the map, the reload in this case is necessary to replace the loaded map with the version that has those options applied.
Summary
Fixes #4456 — when Detach template instances and Repeat last export on save are both enabled and the export target is set to the same file as the map itself, templates were not being detached on subsequent saves.
Root cause (two interacting bugs):
ExportHelper::prepareExportMapalways clearsexportFileName/exportFormatfrom the exported clone — so the file on disk loses the export settings. On the next reload triggered by the file watcher, those settings are gone, causingexportDocumentto early-exit with an empty export filename.The export write updates the file's on-disk timestamp, but
mLastSavedis only updated during the regular save. The file watcher therefore treats the export write as an external change and auto-reloads the document, which replaces the in-memory map (including its export settings) with the freshly-loaded one that has no export settings.Fix:
ExportHelper::prepareExportMap, preserveexportFileNameandexportFormatin the exported file when the export target and the map file resolve to the same canonical path. This restores persistence across reloads and Tiled restarts.Document::refreshLastSaved(fileName)and call it after a successful export to the same file (inexportDocument,exportMapAs, andexportTilesetAs). This prevents the file watcher from treating the export write as an external change and triggering an unnecessary auto-reload.Test plan
.tmxfiletemplate=attribute)