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

Layers inside groups are not affected by automapping #1771

Closed
Lucrecious opened this issue Oct 5, 2017 · 8 comments
Closed

Layers inside groups are not affected by automapping #1771

Lucrecious opened this issue Oct 5, 2017 · 8 comments
Assignees
Labels
bug Broken behavior.

Comments

@Lucrecious
Copy link

Lucrecious commented Oct 5, 2017

Automapping and layers inside groups don't seem to work well together (unless I'm missing something with how I should setup my rules .tmx). I looked through the documentation so either groups aren't handled correctly or the documentation hasn't been updated. In either case, I don't know how to do automapping when my layers are inside groups.

I expect layers inside groups to be affected by automapping, but instead they are not affected.

I'm on the Mac OS Sierra 10.12.

Repo

Step 1: Make a regular rule .tmx
Step 2: Make a map (M) that uses the rules created in (1)
Step 3: Make the input layer in M and apply automapping (the output layer should have been created)
Step 4: Put the input layer in M inside a group and apply automapping
Bug: No rules apply to the output layer.
Step 5: Put the output layer inside the group and the input layer outside of the group and apply the automapping
Bug: A new output layer is created outside of the group.

@Lucrecious Lucrecious changed the title [Bug] Layers inside groups are not affected by automapping Layers inside groups are not affected by automapping Oct 5, 2017
@bjorn bjorn added the bug Broken behavior. label Oct 6, 2017
@bjorn
Copy link
Member

bjorn commented Oct 6, 2017

I'll tag this as a bug since that is what it feels like to the user, but it's really unimplemented functionality. A quick fix would be to add a warning about this to the documentation.

Maybe you can imagine, that actually supporting this could get quite complicated. Groups may exist in the rules map and in the target map, and it will need to be defined somehow how the layers are found. You could help with this by writing down your expectations.

@satu0king
Copy link
Contributor

I would like to try this, could you tell me what exactly should be done.

@bjorn
Copy link
Member

bjorn commented Feb 7, 2018

@satu0king I'm not sure if the bug described at step 4 is really a bug. If the input layer is in a group in the map file, but not in the rules file, should you really expect it to match? Similarly, I'm not sure if step 5 is really a bug. There's little reason for the automapping to consider the grouped "output" layer to be the same output layer as the one in the rules file, if it wasn't also in a group there. It depends on whether we do the matching of layers only based on their names, or also take into account their groups.

It seems @Lucrecious is expecting only the layer names to matter, so this may be the way to go. It would definitely also be the easier approach to implement, and it would be an improvement over the current behavior (which is to ignore groups).

The AutoMapperWrapper and the AutoMapper classes use a lot of Map::indexOfLayer, which only searches layers at the top-level. At least these places will need to be adjusted, probably to use LayerIterator to also search within groups.

@axboureau
Copy link

axboureau commented May 19, 2020

@satu0king I'm not sure if the bug described at step 4 is really a bug. If the input layer is in a group in the map file, but not in the rules file, should you really expect it to match? Similarly, I'm not sure if step 5 is really a bug. There's little reason for the automapping to consider the grouped "output" layer to be the same output layer as the one in the rules file, if it wasn't also in a group there. It depends on whether we do the matching of layers only based on their names, or also take into account their groups.

It seems @Lucrecious is expecting only the layer names to matter, so this may be the way to go. It would definitely also be the easier approach to implement, and it would be an improvement over the current behavior (which is to ignore groups).

The AutoMapperWrapper and the AutoMapper classes use a lot of Map::indexOfLayer, which only searches layers at the top-level. At least these places will need to be adjusted, probably to use LayerIterator to also search within groups.

The issue is a bit old (I was running under an older version of tiled until now), so I don't know how it works now when automapping with groups.
What I would expect would be for the groups to be the same both in input and output.
Ie, I would expect that input_Group1#layer1 in the rule file should only work for the layer name layer1 on Group1.

And that output_Group2#layer1 would output to a layer named layer1 in Group2, and create the layer and the group if needed.

Edit: That said, ignoring groups, and taking input from layers regardless of groups would work too, but it could be trickier four output (if an outputlayer of the same name exists in a group, should a new one be used without a group, or should the group one be reused?).
If ignoring groups, I would prefer reusing an existing layer of the same name inside a group if there is one (ie, either totally ignoring groups, or having the automapping be done according to groups).

If it is much easier to just ignore groups, I think it would be the best solution.

@MikaelForslind
Copy link

This is also something I'd like.

Not a programmer so I don't know what the best solution would be but in my rules tmx file I think what I'd like is to simply specify something like this:

output_GroupName_LayerName

And it would search inside the GroupName for a layer called LayerName and create or update it.

@eishiya
Copy link
Contributor

eishiya commented Nov 8, 2021

I like MikaelForslind's suggestion to allow targeting group/layer by name, it's fairly intuitive and could be chained to deal with groups within groups.

However, I sometimes use groups as a purely cosmetic container, and thus would like the option to treat the map as essentially flat, so that Group/LayerName is the same as LayerName. Perhaps this could be accomplished with a boolean property on the rule map, e.g. IgnoreGroups, which would cause both the rule map and the destination map to be treated as if all the layers were top-level layers. This would be especially helpful for the rule maps themselves, which can end up with dozens of input or output layers, and it would be convenient to be able to group them without that affecting the function of the rules.

@MikaelForslind
Copy link

Hello!

So I created an example project attached to this post.

The Automap layer is used to draw with two different tiles, one creates some ground tiles (AM-Outside) and one creates an indoor background (AM-Inside). I've manually placed them into Groups called Outside and Inside but when I press Automap it creates two new layers outside of the groups.

Expected behaviour I think would be for Tiled to check all pre-existing layers and update them (even if they're inside groups).

Automap_example.zip

@bjorn bjorn closed this as completed in cd2a4f8 Nov 16, 2021
@bjorn
Copy link
Member

bjorn commented Nov 16, 2021

As noted in the commit message, for now the groups have no special meaning, but also no longer cause problems. For the AutoMapping implementation it is now as if they were not there at all and all relevant layers were at the top level, in both the rule map and the target map.

If there is anybody running into this and wishing for a different behavior, I would suggest opening a new issue. :-)

@bjorn bjorn self-assigned this Feb 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Broken behavior.
Projects
No open projects
Status: Done
Development

No branches or pull requests

6 participants