Fixed map reload in case map is open as part of a world #3939
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
bjorn
commented
May 2, 2024
Since they get collapsed due to the model reset. todo: unduplicate code
* Unset mWangSet pointers on WangSet destruction. I'd really like to stop using shared pointers instead, since the WangSet should really own its WangColors, but that'll be a bigger change. * WangDock resets its current WangSet before reload and re-initializes it afterwards. * WangSetModel resets on reload and WangDock was adjusted to make sure all rows are expanded after a model reset. * When a Document is about to be reloaded, make sure any current object referenced from another Document is reset. * TilesetView refreshes the column count after tileset reload, which might have changed when dynamic wrapping is enabled and the tile size changed. * TilesetDocument now clears mWangColorModels upon reload. * TileSelectionItem refreshes its geometry after map reload. * Addressed code duplication in LayerView. * TilesetWangSetModel refreshes as appropriate after tileset reload.
When restoring object selection after the layers, and all selected objects are from the same layer, it would reset the layer selection to only that layer.
Because some scripts will want to respond to this, especially since it means a new editable has been created, which needs signals to be reconnected (it would be nice if we could reuse the existing editable, though this is problematic since it might have created many child editables that point directly to the previous contents)
Also avoid multiple connections to DocumentManager::templateTilesetReplaced.
…tilesets on reload
Cause in that case, the BrokenLinksModel would have connected first to the Document::changed signal.
It 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. Also made some "final" tweaks.
Instead, the script wrapper now handles the AboutToReload and Reloaded signals, in order to detach relevant script objects and to update its object reference.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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:
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).