Skip to content

Fix "Detach templates" not working when export target is the same file#4466

Open
Hijanhv wants to merge 1 commit into
mapeditor:masterfrom
Hijanhv:fix-detach-templates-same-file-export
Open

Fix "Detach templates" not working when export target is the same file#4466
Hijanhv wants to merge 1 commit into
mapeditor:masterfrom
Hijanhv:fix-detach-templates-same-file-export

Conversation

@Hijanhv
Copy link
Copy Markdown

@Hijanhv Hijanhv commented Mar 25, 2026

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):

  1. ExportHelper::prepareExportMap always clears exportFileName/exportFormat from 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, causing exportDocument to early-exit with an empty export filename.

  2. The export write updates the file's on-disk timestamp, but mLastSaved is 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:

  • In ExportHelper::prepareExportMap, preserve exportFileName and exportFormat in 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.
  • Add Document::refreshLastSaved(fileName) and call it after a successful export to the same file (in exportDocument, exportMapAs, and exportTilesetAs). This prevents the file watcher from treating the export write as an external change and triggering an unnecessary auto-reload.

Test plan

  • Enable Detach template instances and Repeat last export on save in preferences
  • Open a map, do File → Export As… targeting the same .tmx file
  • Add template instances to the map, then press Ctrl+S
  • Open the saved file in a text editor — template objects should be fully detached (no template= attribute)
  • Press Ctrl+S again — templates should continue to be detached on each save
  • Close and reopen Tiled, reload the file — export-on-save should still work (export settings persisted in file)
  • Verify that exporting to a different file still clears export settings from the exported file (no regression)

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
Copy link
Copy Markdown
Member

@bjorn bjorn left a comment

Choose a reason for hiding this comment

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

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;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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.

Comment thread src/tiled/mainwindow.cpp
// When exporting to the same file, update the saved timestamp
// to avoid triggering an unnecessary auto-reload
if (exportFileName == document->fileName())
document->refreshLastSaved(exportFileName);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

"Detach templates" does not work with "Repeat last export on save" when exporting to the same file.

2 participants