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

Fixed map reload in case map is open as part of a world #3939

Merged
merged 2 commits into from
Jun 11, 2024

Conversation

bjorn
Copy link
Member

@bjorn bjorn commented May 1, 2024

This is a re-implementation of the way maps are reloaded, to make sure the reload affects all instances and not just the opened editor tab. Previously, the reload was implemented by closing and reopening that tab, which was only a viable method before the implementation of worlds.

This is a rather risky change since the code was never prepared for the map and its internal structures to be replaced entirely without any specific change events. There's a high probability of leaving a roaming pointer somewhere.

Marked as draft because there are a few open tasks:

  • Needs much more testing.
  • Would be nice to restore object and layer selection after the reload when possible.

Fixes part of issue #3927 (though not entirely, since that would require watching map files for changes also when they aren't open in their own tab).

src/tiled/mapitem.cpp Outdated Show resolved Hide resolved
@bjorn bjorn marked this pull request as ready for review May 16, 2024 15:47
@bjorn bjorn force-pushed the fix-map-reload branch 3 times, most recently from d46cff7 to 6d75839 Compare May 24, 2024 19:07
@bjorn bjorn force-pushed the fix-map-reload branch 2 times, most recently from 1ad8eb9 to d0246cc Compare May 28, 2024 16:01
@bjorn
Copy link
Member Author

bjorn commented May 28, 2024

(though not entirely, since that would require watching map files for changes also when they aren't open in their own tab)

I've extended the changes in this PR with another change that makes sure any loaded document is watched for changes on disk and reloaded when appropriate (when it doesn't have unsaved changes). This not only applies to maps loaded as part of a world, but also to tilesets that are loaded as part of maps (issue #1785).

Maps are now reloaded the same way as tilesets, by swapping the internal
data of the MapDocument rather than replacing the entire document. This
also means undo history is kept, since the reload is added as an
undoable action.

This fixes map reload to also affect instances that are open as part of
a world. Previously, the old version would stay open in the world.
However, maps that are only open as part of a world are not yet watched
for changes.

This is a somewhat risky change since the code was never prepared for
the map and its internal structures to be replaced entirely without any
specific change events. New "DocumentAboutToReload" and
"DocumentReloaded" events were added, which are hopefully handled in all
the right places.

Selected layers and objects, as well as expanded group layers, are
restored after reload by ID.

This change also addresses several issues with reloading tilesets.

Added tiled.assetReloaded signal to the scripting API. Most scripts will
not have to do anything,, but some might.

Fixed enabled state of Reload action, which shouldn't be always enabled
for maps (since we can only reload maps when they have a file name and
we know the file format).

Closes mapeditor#3927
When a map is loaded as part of a world, or a tileset is loaded as part
of a map, but not open in its own editor, previously its file was not
watched for changes.

Now these files are also watched. When a change is detected and no
unsaved changes are present, the affected map or tileset will be reloaded.

Closes mapeditor#1785
@bjorn bjorn merged commit 839d9b3 into mapeditor:master Jun 11, 2024
12 of 14 checks passed
@bjorn bjorn deleted the fix-map-reload branch June 11, 2024 12:56
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.

1 participant