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: Ignore empty outputs per-rule #3523

Closed
eishiya opened this issue Nov 12, 2022 · 1 comment · Fixed by #3894 or #3896
Closed

Automapping: Ignore empty outputs per-rule #3523

eishiya opened this issue Nov 12, 2022 · 1 comment · Fixed by #3894 or #3896
Labels
feature It's a feature, not a bug. priority request Has been requested by a major supporter.

Comments

@eishiya
Copy link
Contributor

eishiya commented Nov 12, 2022

Current behaviour

Currently, when an automap rules map has multiple output indices, all rules from that map will choose a random index from all of the available indices and then output whatever is at that index, even if it's completely empty. This is different from how inputs work, where empty input layers within a rule are effectively ignored.

Impact

This behaviour means that if one wants to have multiple rules in a map with differing numbers of random outputs, one has to duplicate the outputs across the different indices, which has the unfortunate side effect of skewing output probabilities. The consequence is that rules that could be simple and grouped together have to either have way more output indices than they should need (to keep the probabilities appropriate), or have to be split into several rule maps.

Suggested behaviour

I think it would be more intuitive for indices that are completely empty for a given rule to be ignored. In the case that a user wants to output nothing at all at random from a particular rule, they can still do so, by placing an Ignore tile on any of that output index's layers within the rule's region. Since this is fairly unusual and the intended way to randomize outputting anything at all is to use rule_options, it's not likely that changing this behaviour will affect many people's rules. And where it does, it's a simple (though potentially tedious) fix.

@eishiya eishiya added the feature It's a feature, not a bug. label Nov 12, 2022
@bjorn
Copy link
Member

bjorn commented Feb 3, 2023

I think it's a good idea. The problem is that currently the AutoMapper only remembers the list of "output sets" globally, and once a rule matches, then it is going to pick a random one and apply it, taking into account the rule's output region.

So to do this, either the output sets need to be stored per-rule, or at least a per-rule storage of the relevant output sets would need to be added.

@bjorn bjorn added the priority request Has been requested by a major supporter. label Feb 23, 2024
@bjorn bjorn closed this as completed in 7910f9c Feb 26, 2024
bjorn added a commit that referenced this issue Feb 26, 2024
This change introduces a "compileOutputSet" step which is executed as
part of collecting the rules. In this step, each rule only receives as
possible outputs the output indexes that are non-empty for the region of
that rule.

This makes it much easier to add output variation to only some of the
rules. Previously, due to the possibility of empty outputs being chosen,
having rules with various amounts of variation required setting up
separate rule maps.

This new behavior not kick in for legacy rule maps that still define the
regions manually.

Also fixed a failing Automapping test (unrelated to this change).

Closes #3523
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature It's a feature, not a bug. priority request Has been requested by a major supporter.
Projects
None yet
2 participants