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

Automapping: Output objects don't contribute to output region #3473

Closed
eishiya opened this issue Sep 18, 2022 · 5 comments · Fixed by #3601
Closed

Automapping: Output objects don't contribute to output region #3473

eishiya opened this issue Sep 18, 2022 · 5 comments · Fixed by #3601
Assignees
Labels
bug Broken behavior.
Milestone

Comments

@eishiya
Copy link
Contributor

eishiya commented Sep 18, 2022

When determining what parts of a rule to output, Automapping only seems to look at tiles, and if objects are present, they are only output if they overlap an output tile as well. This is unintuitive and feels like a bug.

For example, these two rules that look for "NonEmpty" on the input layer and have just an object output layer will output nothing:
image
My input, unmodified by this rule:
image
But this is what I expect:
image

And this rule that outputs a tile in a location other than the object's location will output the tile, but not the object:
image
image

However, this rule, which outputs both an object and a tile at the same location, will work correctly:
image
image

If I want to make a rule that outputs only an object at some location, I have to have a tile output layer with a tile on it that will be ignored:
image
image

I think any tile touched by an output object should count as part of the output region. For example, this small object would add the selected tile to the output region:
image
Care would need to be taken with non-rectangular objects. For example, these are the total regions I expect these two rules to have:
image
But if the bounding box of the object is used instead of checking whether a tile actually overlaps the object, then the regions would be like this, and considered a single rule:
image

Alternatively, if it's intended that objects don't contribute to the output region (e.g. to avoid specifically this kind of weirdness with the shapes), then the fact that they don't contribute and may need output Tile Layers with Ignore tiles to output should be documented (I can add it to my rewrite draft).

@eishiya eishiya added the bug Broken behavior. label Sep 18, 2022
bjorn added a commit to bjorn/tiled-dev that referenced this issue Mar 6, 2023
With this change, output object layers are taken into account when
determining the rule output regions.

The support for outputting objects by AutoMapping is still lacking
support for many cases, like polygon/polyline objects or the object
alignment setting, but it is at least meant to work for basic rectangle
or tile objects.

This change also adjusts tileRegionOfObjectGroup to the change that made
object position and size be stored in pixels rather than tiles
(55e7767), as was already done for
objectsToErase before (521a40a).

Closes mapeditor#3473
@bjorn bjorn self-assigned this Mar 6, 2023
@bjorn bjorn added this to the Tiled 1.10 milestone Mar 6, 2023
@bjorn
Copy link
Member

bjorn commented Mar 6, 2023

Care would need to be taken with non-rectangular objects. For example, these are the total regions I expect these two rules to have:

The linked PR fixes the general issue, but does not make AutoMapping work correctly for polygon or polyline objects. I believe since it works only with the object's rectangle, it will handle these as if they were point objects.

When I would fix it, I'd likely use the bounding box, even if it may cause unintentional merging of rules in some cases. I just think that any more complicated handling of that case is not worth the effort given how rare it must be in practice. Eventually, it would be good to add a rule regions visualization, which would help spot such cases when they do happen.

@eishiya
Copy link
Contributor Author

eishiya commented Mar 6, 2023

The PR fixes the issue as described, but misses the same thing for DeleteTiles: that still only deletes those Objects that overlap a tile in the match region. This rule's output objects will never be deleted:
image
But this one's objects will be deleted:
image

Perhaps this is fine, I don't know as I haven't done practical Automapping with Objects. On one hand, I expect DeleteTiles to prevent stacking outputs like this, but on the other, I wouldn't want it to delete Objects that I placed manually that happen to be near the output area.

I checked the behaviour for Polygons, and it indeed looks like their bounds are handled like Points. If their position is exactly at the edge of the cell (e.g. because of Snapping), they're not considered part of the region, their origin point has to be within the cell. Using the bounding box would indeed be a big improvement over this, and I guess it's fair enough to use that rather than doing something more complex, given how rare this scenario is.

@bjorn
Copy link
Member

bjorn commented Mar 6, 2023

@eishiya I haven't tested it yet, but isn't this behavior consistent with tile layers? As far as I know, DeleteTiles has always only worked for regions where any of the input layers have any tiles. This is to have it only delete tiles in regions which can be expected to be "refreshed" by Automapping rules.

@eishiya
Copy link
Contributor Author

eishiya commented Mar 6, 2023

DeleteTiles is the only way to delete Objects from a map via AutoMapping, so preventing the stacking of Objects due to repeated Automapping is one of the uses I expected of it. I could be wrong, as I never found DeleteTiles intuitive even for just tile work xP

bjorn added a commit that referenced this issue Mar 6, 2023
With this change, output object layers are taken into account when
determining the rule output regions.

The support for outputting objects by AutoMapping is still lacking
support for many cases, like polygon/polyline objects or the object
alignment setting, but it is at least meant to work for basic rectangle
or tile objects.

This change also adjusts tileRegionOfObjectGroup to the change that made
object position and size be stored in pixels rather than tiles
(55e7767), as was already done for
objectsToErase before (521a40a).

Closes #3473
@bjorn
Copy link
Member

bjorn commented Mar 6, 2023

@eishiya I'm open to explore better options for deletion, but I've closed this issue since I think that one has been fixed now. Thanks for testing and the additional feedback! :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Broken behavior.
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants