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

Request: probability setting for output layers on automapping. #3179

Closed
cosmicr opened this issue Nov 16, 2021 · 11 comments · Fixed by #3602
Closed

Request: probability setting for output layers on automapping. #3179

cosmicr opened this issue Nov 16, 2021 · 11 comments · Fixed by #3602
Assignees

Comments

@cosmicr
Copy link

cosmicr commented Nov 16, 2021

If I have 5 grass tiles, but 3 of them are grass with flowers, I was wondering if it could be made so that I could set the probability of those output layers being less than the rest?

Eg:

output5_grass_0.5
output4_grass_0.5
output3_grass_0.5
output2_grass
output1_grass
input_grass

where output3 to output5 are the flower tiles.

This way even though I have a bigger selection of flowers, I'll still see more grass.

Apologies if this is already a thing, I couldn't find it in the documentation.

@eishiya
Copy link
Contributor

eishiya commented Nov 16, 2021

Nope, not currently possible AFAIK. I've been accomplishing varied probability by duplicating outputs that need to be more common, but that gets annoying and hard to maintain very quickly. I'd love some better way to have Tiled favour certain outputs over others.

Unfortunately I don't think your proposed solution would quite work as you expect, because Tiled works by output index, not output layer. How should Tiled weigh output index 5 in the case that you have output5_grass_0.5 and output5_grassDetails_0.1? It is the indices that need to be weighed, but Automapping currently has no way to set properties for a given index, only for a given layer. Perhaps the last found specified probability for a given index would be used?

Also, I think it would be better to specify the probabilities as custom properties rather than as part of the layer name. If the probability is in the layer name, then targeting specific layer names can be ambiguous. What if I want to target a layer called grass_0.5? That seems like a terribly contrived scenario, but layer names can have all sorts of funkiness in them as an aid to importing into game engines.

Edit: A more far-fetched suggestion: If we ever get a dedicated rule editor that lets us specify all the candidate tiles for a given cell all together instead of or in addition to via output indices, the tiles' probability properties could be used to decide which tile to place.

@cosmicr
Copy link
Author

cosmicr commented Nov 16, 2021

Ah that's a shame. Oh well, hopefully one day it will be possible in one way or another. Thanks for the reply. I'll leave the issue open for a bit in case someone else wants to chime in.

@eishiya
Copy link
Contributor

eishiya commented Nov 16, 2021

It's a good issue that should be left open. Even if your specific implementation suggestion wouldn't work, the basic idea is a very good one and perhaps bjorn or someone else can think of a way to make it work.

@bjorn bjorn self-assigned this Mar 28, 2022
bjorn added a commit to bjorn/tiled-dev that referenced this issue Mar 6, 2023
Rather than each output set always having equal probability, it is now
possible to specify a probability for each output set, using a custom
"Probability" property on any of its layers (when multiple layers in an
output set have a "Probability" property, the last / top-most one wins).

The probability between output sets is relative and default to 1.0. So
for example, when setting it to 2.0 for some output set, it will be
twice as likely to be picked relative to any others.

Closes mapeditor#3179
@bjorn
Copy link
Member

bjorn commented Mar 6, 2023

@cosmicr I've implemented this feature in PR #3602. You're welcome to test it out using the builds that should become available at https://github.com/bjorn/tiled-dev/actions/runs/4345422448.

@eishiya
Copy link
Contributor

eishiya commented Mar 6, 2023

I had a look at that build. It consistently crashes when I try to Undo Automapping that invoved a probability. All other Undo is fine, including the same rules without probability set. Explicitly setting probability to 1.0 on an output still crashes.

It would be nice if the probability property were displayed on output layers to ease setting it, similar to how AutoEmpty is displayed on input layers.

Aside from that rather serious bug and that minor invoncenice, it works :D Being able to control the probabilities this way is very nice!

Edit: Here's my draft documentation for probability, in case you want to use it for the current docs:

This float layer property can be added to output layers to control the probability that a given output index will be chosen. The probabilities for each output index are relative to one another, and default to 1.0. For example, if you have outputA with probability 2 and outputB with probability 0.5, A will be chosen four times as often as B. If multiple output layers with the same index have their Probability set, the last (top-most) layer's probability will be used.

@bjorn
Copy link
Member

bjorn commented Mar 7, 2023

It would be nice if the probability property were displayed on output layers to ease setting it, similar to how AutoEmpty is displayed on input layers.

I think there are some problems with displaying this Probability property...

  • It's entirely irrelevant when you only have one output index, which is usually the case. And showing it only when multiple output indices are defined gets a little tricky / annoying.
  • Even when you have multiple output indices, showing it grayed out on each layer as "1.0" may give the wrong impression, since another layer might have set a different value. Displaying the value of the other layer gets a little tricky / annoying.
  • Due to the second issue, it could easily lead people to set it on multiple layers for the same output index.

So I'm not sure if we're really helping by showing this property. :-(

On the bright side, the crash should be fixed now.

@bjorn
Copy link
Member

bjorn commented Mar 7, 2023

Well, I've just decided to show that property for now, hopefully it helps more people than it confuses. :-)

bjorn added a commit that referenced this issue Mar 7, 2023
* AutoMapping: Added support for output set probability (#3179)
* AutoMapping: Fixed crash on undo when output layers have properties
@eishiya
Copy link
Contributor

eishiya commented Mar 7, 2023

TIL that apparently custom properties on output layers get copied o: I had no idea. Do they get copied only when the layer is created by automapping, or do they get copied even if the layer already existed?

Regarding displaying the "Probability" property, maybe it should be called "IndexProbability"? Since it doesn't need to be typed, a longer name that's clearer is better than a shorter name that's vague. I'm not sure if my suggested name is necessarily clearer, though xP Ideally it should only be displayed on layers of an index that hasn't had a probability assigned, but I imagine that's difficult to do.

Thank you for implementing this!

@bjorn
Copy link
Member

bjorn commented Mar 7, 2023

TIL that apparently custom properties on output layers get copied o: I had no idea. Do they get copied only when the layer is created by automapping, or do they get copied even if the layer already existed?

They get copied as part of applying a rule, so they won't get copied when no rule has matched. It doesn't matter whether the layer previously existed or not.

I'm also not sure if "IndexProbability" is clearer. :S Maybe "OutputIndexProbability"...

@eishiya
Copy link
Contributor

eishiya commented Mar 7, 2023

They get copied as part of applying a rule, so they won't get copied when no rule has matched. It doesn't matter whether the layer previously existed or not.

I see. I will make a note of this in my docs draft.

I'm also not sure if "IndexProbability" is clearer. :S Maybe "OutputIndexProbability"...

This property only appears on output layers, so I don't think specifying "Output" is necessary. I guess we can leave it as "Probability" and remove the automatic visibility if people get confused about it?
Although, I do wonder if a more specific name might not be a good idea just to avoid collisions with properties people might want to add to their layers. I imagine using automapping to add layer properties is very rare, though.

@bjorn
Copy link
Member

bjorn commented Mar 7, 2023

I imagine using automapping to add layer properties is very rare, though.

Copying the properties was implemented in f19f97b in response to a request posted on the forum, but I reckon it is rare indeed.

I guess I'll just keep the name "Probability".

dogboydog pushed a commit to dogboydog/tiled that referenced this issue Mar 8, 2023
Rather than each output set always having equal probability, it is now
possible to specify a probability for each output set, using a custom
"Probability" property on any of its layers (when multiple layers in an
output set have a "Probability" property, the last / top-most one wins).

The probability between output sets is relative and default to 1.0. So
for example, when setting it to 2.0 for some output set, it will be
twice as likely to be picked relative to any others.

Closes mapeditor#3179
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 a pull request may close this issue.

3 participants