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.

Sign up for GitHub

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

Jump to bottom

Automapping creates too many undo events #2166

Closed
JoeCreates opened this issue Jul 23, 2019 · 3 comments
Closed

Automapping creates too many undo events #2166

JoeCreates opened this issue Jul 23, 2019 · 3 comments
Assignees
@bjorn
Labels
usability Generally about making something more intuitive or efficient.

Comments

@JoeCreates
Copy link

It seems impractical that auto mapping creates undo events as if tiles were individually placed. Applying rules should create a single event.
@bjorn
Copy link
Member

bjorn commented Jul 23, 2019

Applying the rules does create a single event, but when interleaved with the events you create while editing they do not merge and hence you get a lot of undo steps.

The challenge is to somehow allow the automapping steps to merge with the undo commands created by the tile layer editing tools.

If you find this disrupts your flow, currently the only option is to disable "Map > Automap While Drawing" and to trigger the applying of the rules manually from time to time instead.

@bjorn bjorn added the usability Generally about making something more intuitive or efficient. label Jul 23, 2019
@JoeCreates
Copy link
Author

Ah thanks for the workaround!

@bjorn bjorn added this to Tiled 1.9 Mar 28, 2022
@bjorn bjorn moved this to Todo in Tiled 1.9 Mar 28, 2022
@bjorn bjorn self-assigned this Mar 28, 2022
bjorn added a commit that referenced this issue Apr 5, 2022
@bjorn
AutoMapping: Introduced AutoMappingContext
55b4bb3 
This context stores temporary data specific to applying the active
AutoMappers to a target map. Previously this data was stored inside each
AutoMapper and hence the rule maps had to be reloaded to be applied to a
different map. Now this reloading is no longer necessary.

At the same time, the context stores any new tilesets, layers and
objects that may need to be added to the target map. They are no longer
added directly by the AutoMapper, which avoids the need to remove unused
tilesets and empty layers in a cleanup pass at the end.

This change also makes progress towards using the AutoMapper as part of
the preview or to be able to merge undo steps (see issue #2166), though
for now that is still not possible since tile layers in the target map
are still changed in-place.

Finally I figured out how to make QtConcurrent::blockingMapped compile
on Qt 5, making AutoMapping with Qt 5 almost twice as fast (matching the
Qt 6 speed). The functor was simplified by re-using the lambda
internally.
bjorn added a commit that referenced this issue Apr 5, 2022
@bjorn
AutoMapping: Introduced AutoMappingContext
d75e85e 
This context stores temporary data specific to applying the active
AutoMappers to a target map. Previously this data was stored inside each
AutoMapper and hence the rule maps had to be reloaded to be applied to a
different map. Now this reloading is no longer necessary.

At the same time, the context stores any new tilesets, layers and
objects that may need to be added to the target map. They are no longer
added directly by the AutoMapper, which avoids the need to remove unused
tilesets and empty layers in a cleanup pass at the end.

This change also makes progress towards using the AutoMapper as part of
the preview or to be able to merge undo steps (see issue #2166), though
for now that is still not possible since tile layers in the target map
are still changed in-place.

Finally I figured out how to make QtConcurrent::blockingMapped compile
on Qt 5, making AutoMapping with Qt 5 almost twice as fast (matching the
Qt 6 speed). The functor was simplified by re-using the lambda
internally.
@bjorn bjorn moved this from Todo to In Progress in Tiled 1.9 Apr 6, 2022
bjorn added a commit that referenced this issue Apr 6, 2022
@bjorn
AutoMapping: The AutoMapper no longer changes tile layers directly
1369418 
Instead, the changes are made to cloned layers and are then applied
after the AutoMapping. It's not that much different from before, since
these clones were already being made by the AutoMapperWrapper in order
to figure out which areas were changed by the AutoMapping.

Also fixed the order of any newly added layers, which was reversed since
change d75e85e.

Progress towards resolving #2166.
bjorn added a commit that referenced this issue Apr 6, 2022
@bjorn
WIP: AutoMap While Drawing no longer produces additional undo commands
46d71fa 
Still work in progress, since this change breaks redo for painting
operations somewhere.

Closes #2166
@bjorn bjorn closed this as completed in 0fd4747 Apr 7, 2022
Repository owner moved this from In Progress to Done in Tiled 1.9 Apr 7, 2022
@bjorn
Copy link
Member

bjorn commented Apr 7, 2022 •
edited
Loading

Finally, in the upcoming Tiled 1.9, "AutoMapping While Drawing" will no longer create additional undo commands! :-)

Only remaining exception is when it creates or deletes objects. It's a rare case, but maybe that could be resolved later as well.

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

@bjorn bjorn

Labels
usability Generally about making something more intuitive or efficient.
Projects
No open projects
Tiled 1.9
Status: Done
Milestone
No milestone
Development

No branches or pull requests
2 participants
@bjorn @JoeCreates