Automapping: Always apply output sets with empty index (#3909)

Output sets with an empty index (output_foo, rather than output1_foo, for example) are now considered unconditional outputs and no longer participate in the random output index selection process. Effectively, when a rule matches that has output sets with both an empty index and output sets with an index, first its unconditional output set will apply and then a randomly selected output set from its output sets with non-empty index. For compatibility reasons, this behavior does not affect rule maps in "legacy" mode (when the user manually defined the rule regions).
mapeditor · Mar 18, 2024 · 88e4525 · 88e4525
1 parent d29ef08
commit 88e4525
Show file tree

Hide file tree

Showing 10 changed files with 149 additions and 39 deletions.
diff --git a/NEWS.md b/NEWS.md
@@ -17,6 +17,7 @@
 * tmxrasterizer: Fixed --hide/show-layer to work on group layers (#3899)
 * tmxviewer: Added support for viewing JSON maps (#3866)
 * AutoMapping: Ignore empty outputs per-rule (#3523)
+* AutoMapping: Always apply output sets with empty index
 * Windows: Fixed the support for WebP images (updated to Qt 6.6.1, #3661)
 * Fixed the option to resolve properties on export to also resolve class members (#3411, #3315)
 * Fixed terrain tool behavior and terrain overlays after changing terrain set type (#3204, #3260)

diff --git a/docs/manual/automapping.md b/docs/manual/automapping.md
@@ -111,7 +111,9 @@ output[index]_name
 
 Everything after the first underscore is the **name**, which determines which layer in the working map the tiles or objects will be placed on. If the working map includes multiple layers by this name, the bottom-most one will be used. If the rule matches and the working map does not already contain the named output layer, Automapping will create the layer.
 
-The **index** is optional, and is not related to the input indices. Instead, output indices are used to randomize the output: every time the rule finds a match, a random output index is chosen and only the output layers with that index will have their contents placed into the working map. If an output index is completely empty for a given rule, it will never be chosen for that rule (since Tiled 1.10.3).
+The **index** is optional, and is not related to the input indices. Instead, output indices are used to randomize the output: every time the rule finds a match, a random output index is chosen and only the output layers with that index will have their contents placed into the working map.
+
+For convenience, Tiled 1.10.3 introduced two changes to the behavior related to indexes. If an output index is completely empty for a given rule, it will never be chosen for that rule. This is useful when some rules have more random options than others. Also, when no index is specified, that part of the rule's output will always apply when the rule matches. This can be used to combine an unconditional part of a rule's output with a random part.
 
 #### Random Output Example
 
@@ -406,6 +408,8 @@ If you'd like to instead update your rules to not rely on any legacy behavior, t
 
 * If you have rules that rely on some output indices being empty to randomly not make any changes, you will need to place [**Ignore** special tiles](#specialtiles) in at least one layer of each empty output index so that those indices aren't ignored. Alternatively, you can use [`rule_options`](#object-properties) to give those rules a chance to not run at all.
 
+* If you had rules with random output, but did not specify an index for one of the outputs, this part of the rule's output is now excluded from the options and applied unconditionally instead. If all outputs should be random options, make sure they all have an index.
+
 ## Credits
 
 The [Sidescroller Details](#sidescroller-details) example uses art from [A platformer in the forest](https://opengameart.org/content/a-platformer-in-the-forest) by Buch.
diff --git a/src/tiled/automapper.cpp b/src/tiled/automapper.cpp
@@ -437,18 +437,18 @@ bool AutoMapper::setupRuleMapLayers()
             continue;
         }
 
-        const int layerNameStartPosition = ruleMapLayerName.indexOf(QLatin1Char('_')) + 1;
+        const int layerNameStartPosition = ruleMapLayerName.indexOf(QLatin1Char('_'));
 
         // both 'rule' and 'output' layers will require and underscore and
         // rely on the correct position detected of the underscore
-        if (layerNameStartPosition == 0) {
+        if (layerNameStartPosition == -1) {
             error += tr("Did you forget an underscore in layer '%1'?").arg(ruleMapLayerName);
             error += QLatin1Char('\n');
             continue;
         }
 
-        const QString layerName = ruleMapLayerName.mid(layerNameStartPosition);  // all characters behind the underscore (excluded)
-        QString setName = ruleMapLayerName.left(layerNameStartPosition);         // all before the underscore (included)
+        const QString layerName = ruleMapLayerName.mid(layerNameStartPosition + 1); // all characters after the underscore
+        QString setName = ruleMapLayerName.left(layerNameStartPosition);            // all before the underscore
 
         if (setName.startsWith(QLatin1String("output"), Qt::CaseInsensitive))
             setName.remove(0, 6);
@@ -472,11 +472,11 @@ bool AutoMapper::setupRuleMapLayers()
             inputLayer.tileLayer = tileLayer;
             setupInputLayerProperties(inputLayer);
 
-            InputSet &inputSet = find_or_emplace<InputSet>(setup.mInputSets, [&setName] (const InputSet &set) {
+            auto &inputSet = find_or_emplace<InputSet>(setup.mInputSets, [&setName] (const InputSet &set) {
                 return set.name == setName;
             }, setName);
 
-            InputConditions &conditions = find_or_emplace<InputConditions>(inputSet.layers, [&layerName] (const InputConditions &conditions) {
+            auto &conditions = find_or_emplace<InputConditions>(inputSet.layers, [&layerName] (const InputConditions &conditions) {
                 return conditions.layerName == layerName;
             }, layerName);
 
@@ -495,7 +495,7 @@ bool AutoMapper::setupRuleMapLayers()
             else if (layer->isObjectGroup())
                 setup.mOutputObjectGroupNames.insert(layerName);
 
-            OutputSet &outputSet = find_or_emplace<OutputSet>(setup.mOutputSets, [&setName] (const OutputSet &set) {
+            auto &outputSet = find_or_emplace<OutputSet>(setup.mOutputSets, [&setName] (const OutputSet &set) {
                 return set.name == setName;
             }, setName);
 
@@ -577,8 +577,8 @@ void AutoMapper::setupRules()
     if (setup.mLayerOutputRegions)
         regionOutput |= setup.mLayerOutputRegions->region();
 
-    const bool ignoreEmptyOutputs = !(mRuleMapSetup.mLayerRegions ||
-                                      mRuleMapSetup.mLayerInputRegions);
+    const bool legacyMode = (mRuleMapSetup.mLayerRegions ||
+                             mRuleMapSetup.mLayerInputRegions);
 
     // When no input regions have been defined at all, derive them from the
     // "input" and "inputnot" layers.
@@ -638,8 +638,12 @@ void AutoMapper::setupRules()
 
         for (const OutputSet &outputSet : std::as_const(mRuleMapSetup.mOutputSets)) {
             RuleOutputSet index;
-            if (compileOutputSet(index, outputSet, rule.outputRegion) || !ignoreEmptyOutputs)
-                rule.outputSets.add(index, outputSet.probability);
+            if (compileOutputSet(index, outputSet, rule.outputRegion) || legacyMode) {
+                if (outputSet.name.isEmpty() && !legacyMode)
+                    rule.outputSet = std::move(index);
+                else
+                    rule.outputSets.add(index, outputSet.probability);
+            }
         }
     }
 
@@ -990,6 +994,10 @@ bool AutoMapper::compileInputSet(RuleInputSet &index,
     return true;
 }
 
+/**
+ * Processes the given \a outputSet, adding the output layers to the given
+ * \a index. Returns whether the output set is non-empty.
+ */
 bool AutoMapper::compileOutputSet(RuleOutputSet &index,
                                   const OutputSet &outputSet,
                                   const QRegion &outputRegion) const
@@ -1179,7 +1187,7 @@ void AutoMapper::matchRule(const Rule &rule,
                            const std::function<void(QPoint pos)> &matched,
                            const AutoMappingContext &context) const
 {
-    if (rule.outputSets.isEmpty())
+    if (!rule.outputSet && rule.outputSets.isEmpty())
         return;
 
     QVector<RuleInputSet> inputSets;
@@ -1229,33 +1237,22 @@ void AutoMapper::applyRule(const Rule &rule, QPoint pos,
                            ApplyContext &applyContext,
                            AutoMappingContext &context) const
 {
-    Q_ASSERT(!rule.outputSets.isEmpty());
-
     // Translate the position to adjust to the location of the rule.
     pos -= rule.inputRegion.boundingRect().topLeft();
 
-    // choose by chance which group of rule_layers should be used:
-    const RuleOutputSet &outputSet = rule.outputSets.pick();
+    // If named output sets are given, choose one of them by chance
+    const RuleOutputSet *randomOutputSet = nullptr;
+    if (!rule.outputSets.isEmpty())
+        randomOutputSet = &rule.outputSets.pick();
 
     if (rule.options.noOverlappingOutput) {
-        // check if there are no overlaps within this rule.
         QHash<const Layer*, QRegion> ruleRegionInLayer;
 
-        // TODO: Very slow to re-calculate the entire region for
-        // each rule output layer here, each time a rule has a match.
-
-        for (const auto &tileOutput : outputSet.tileOutputs) {
-            const Layer *targetLayer = context.outputTileLayers.value(tileOutput.name);
-            QRegion &outputLayerRegion = ruleRegionInLayer[targetLayer];
-            outputLayerRegion = tileOutput.tileLayer->region() & rule.outputRegion;
-        }
+        if (rule.outputSet)
+            collectLayerOutputRegions(rule, *rule.outputSet, context, ruleRegionInLayer);
 
-        for (const auto &objectOutput : outputSet.objectOutputs) {
-            const Layer *targetLayer = context.outputTileLayers.value(objectOutput.name);
-            QRegion &outputLayerRegion = ruleRegionInLayer[targetLayer];
-            for (const MapObject *mapObject : objectOutput.objects)
-                outputLayerRegion |= objectTileRect(*mRulesMapRenderer, *mapObject);
-        }
+        if (randomOutputSet)
+            collectLayerOutputRegions(rule, *randomOutputSet, context, ruleRegionInLayer);
 
         // Translate the regions to the position of the rule and check for overlap.
         for (auto it = ruleRegionInLayer.keyValueBegin(), it_end = ruleRegionInLayer.keyValueEnd();
@@ -1283,12 +1280,46 @@ void AutoMapper::applyRule(const Rule &rule, QPoint pos,
         }
     }
 
-    copyMapRegion(rule, pos, outputSet, context);
+    if (rule.outputSet)
+        copyMapRegion(rule, pos, *rule.outputSet, context);
+
+    if (randomOutputSet)
+        copyMapRegion(rule, pos, *randomOutputSet, context);
 
     if (applyContext.appliedRegion)
         *applyContext.appliedRegion |= rule.outputRegion.translated(pos.x(), pos.y());
 }
 
+/**
+ * Collects the per-layer output region of the given \a rule, when using the
+ * given \a outputSet.
+ *
+ * The \a ruleRegionInLayer parameter tells us for each target output layer,
+ * which region will be touched by applying this output.
+ */
+void AutoMapper::collectLayerOutputRegions(const Rule &rule,
+                                           const RuleOutputSet &outputSet,
+                                           AutoMappingContext &context,
+                                           QHash<const Layer*, QRegion> &ruleRegionInLayer) const
+{
+    // TODO: Very slow to re-calculate the entire region for each rule output
+    // layer here, each time a rule has a match. These regions are also
+    // calculated in AutoMapper::setupRules, when no region layers are defined.
+
+    for (const auto &tileOutput : outputSet.tileOutputs) {
+        const Layer *targetLayer = context.outputTileLayers.value(tileOutput.name);
+        QRegion &outputLayerRegion = ruleRegionInLayer[targetLayer];
+        outputLayerRegion |= tileOutput.tileLayer->region() & rule.outputRegion;
+    }
+
+    for (const auto &objectOutput : outputSet.objectOutputs) {
+        const Layer *targetLayer = context.outputTileLayers.value(objectOutput.name);
+        QRegion &outputLayerRegion = ruleRegionInLayer[targetLayer];
+        for (const MapObject *mapObject : objectOutput.objects)
+            outputLayerRegion |= objectTileRect(*mRulesMapRenderer, *mapObject);
+    }
+}
+
 void AutoMapper::copyMapRegion(const Rule &rule, QPoint offset,
                                const RuleOutputSet &outputSet,
                                AutoMappingContext &context) const

diff --git a/src/tiled/automapper.h b/src/tiled/automapper.h
@@ -362,6 +362,7 @@ class TILED_EDITOR_EXPORT AutoMapper : public QObject
         QRegion inputRegion;
         QRegion outputRegion;
         RuleOptions options;
+        std::optional<RuleOutputSet> outputSet;
         RandomPicker<RuleOutputSet> outputSets;
     };
 
@@ -431,14 +432,16 @@ class TILED_EDITOR_EXPORT AutoMapper : public QObject
                    const AutoMappingContext &context) const;
 
     /**
-     * Applies the given \a rule at each of the given \a positions.
-     *
-     * Might skip some of the positions to satisfy the NoOverlappingRules
-     * option.
+     * Applies the given \a rule at the given \a pos.
      */
     void applyRule(const Rule &rule, QPoint pos, ApplyContext &applyContext,
                    AutoMappingContext &context) const;
 
+    void collectLayerOutputRegions(const Rule &rule,
+                                   const RuleOutputSet &outputSet,
+                                   AutoMappingContext &context,
+                                   QHash<const Layer *, QRegion> &ruleRegionInLayer) const;
+
     void addWarning(const QString &text,
                     std::function<void()> callback = std::function<void()>());
 

diff --git a/tests/automapping/output-probability/map.tmx b/tests/automapping/output-probability/map.tmx
@@ -0,0 +1,31 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<map version="1.10" tiledversion="1.10.2" orientation="orthogonal" renderorder="right-down" width="5" height="5" tilewidth="16" tileheight="16" infinite="0" nextlayerid="10" nextobjectid="1">
+ <tileset firstgid="1" source="../spr_test_tileset.tsx"/>
+ <layer id="1" name="set" width="5" height="5">
+  <data encoding="csv">
+2,2,2,2,2,
+2,2,2,2,2,
+2,2,2,2,2,
+2,2,2,2,2,
+2,2,2,2,2
+</data>
+ </layer>
+ <layer id="9" name="auto_base" width="5" height="5">
+  <data encoding="csv">
+0,0,0,0,0,
+0,0,0,0,0,
+0,0,0,0,0,
+0,0,0,0,0,
+0,0,0,0,0
+</data>
+ </layer>
+ <layer id="8" name="auto" width="5" height="5">
+  <data encoding="csv">
+0,0,0,0,0,
+0,0,0,0,0,
+0,0,0,0,0,
+0,0,0,0,0,
+0,0,0,0,0
+</data>
+ </layer>
+</map>
diff --git a/tests/automapping/output-probability/rules.tmx b/tests/automapping/output-probability/rules.tmx
@@ -0,0 +1,39 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<map version="1.10" tiledversion="1.10.2" orientation="orthogonal" renderorder="right-down" width="4" height="4" tilewidth="16" tileheight="16" infinite="0" nextlayerid="7" nextobjectid="5">
+ <tileset firstgid="1" source="../spr_test_tileset.tsx"/>
+ <layer id="1" name="input_set" width="4" height="4">
+  <data encoding="csv">
+0,0,0,0,
+0,2,0,0,
+0,0,0,0,
+0,0,0,0
+</data>
+ </layer>
+ <layer id="5" name="outputB_auto" width="4" height="4">
+  <data encoding="csv">
+0,0,0,0,
+0,7,0,0,
+0,0,0,0,
+0,0,0,0
+</data>
+ </layer>
+ <layer id="6" name="outputA_auto" width="4" height="4">
+  <properties>
+   <property name="Probability" type="float" value="3"/>
+  </properties>
+  <data encoding="csv">
+0,0,0,0,
+0,6,0,0,
+0,0,0,0,
+0,0,0,0
+</data>
+ </layer>
+ <layer id="2" name="output_auto_base" width="4" height="4">
+  <data encoding="csv">
+0,0,0,0,
+0,5,0,0,
+0,0,0,0,
+0,0,0,0
+</data>
+ </layer>
+</map>
diff --git a/tests/automapping/probability/rules.txt → .../automapping/output-probability/rules.txt b/tests/automapping/probability/rules.txt → .../automapping/output-probability/rules.txt
diff --git a/tests/automapping/probability/map.tmx → tests/automapping/rule-probability/map.tmx b/tests/automapping/probability/map.tmx → tests/automapping/rule-probability/map.tmx
@@ -1,5 +1,5 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<map version="1.9" tiledversion="1.9.1" orientation="orthogonal" renderorder="right-down" width="5" height="5" tilewidth="16" tileheight="16" infinite="0" nextlayerid="9" nextobjectid="1">
+<map version="1.10" tiledversion="1.10.2" orientation="orthogonal" renderorder="right-down" width="5" height="5" tilewidth="16" tileheight="16" infinite="0" nextlayerid="9" nextobjectid="1">
  <tileset firstgid="1" source="../spr_test_tileset.tsx"/>
  <layer id="1" name="set" width="5" height="5">
   <data encoding="csv">

diff --git a/tests/automapping/probability/rules.tmx → tests/automapping/rule-probability/rules.tmx b/tests/automapping/probability/rules.tmx → tests/automapping/rule-probability/rules.tmx
@@ -1,5 +1,5 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<map version="1.9" tiledversion="1.9.1" orientation="orthogonal" renderorder="right-down" width="7" height="4" tilewidth="16" tileheight="16" infinite="0" nextlayerid="5" nextobjectid="5">
+<map version="1.10" tiledversion="1.10.2" orientation="orthogonal" renderorder="right-down" width="7" height="4" tilewidth="16" tileheight="16" infinite="0" nextlayerid="5" nextobjectid="5">
  <tileset firstgid="1" source="../spr_test_tileset.tsx"/>
  <layer id="1" name="input_set" width="7" height="4">
   <data encoding="csv">

diff --git a/tests/automapping/rule-probability/rules.txt b/tests/automapping/rule-probability/rules.txt
@@ -0,0 +1 @@
+./rules.tmx