fix: spawner type in pattern (#3042)

* fix: try parsing special block data before blindly applying NBT

* Fix legacy block wrappers with new NBT API (#2361)

(cherry picked from commit 5b674745f17d8ede505857242e5240c6dbeebfd6)

---------

Co-authored-by: Maddy Miller <mnmiller1@me.com>

worldedit-core/src/legacy/java/com/sk89q/worldedit/blocks/LegacyBaseBlockWrapper.java

@@ -0,0 +1,48 @@
+/*
+ * WorldEdit, a Minecraft world manipulation toolkit
+ * Copyright (C) sk89q <http://www.sk89q.com>
+ * Copyright (C) WorldEdit team and contributors
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <https://www.gnu.org/licenses/>.
+ */
+
+package com.sk89q.worldedit.blocks;
+
+import com.sk89q.jnbt.CompoundTag;
+import com.sk89q.worldedit.util.concurrency.LazyReference;
+import com.sk89q.worldedit.world.block.BaseBlock;
+import com.sk89q.worldedit.world.block.BlockState;
+import org.enginehub.linbus.tree.LinCompoundTag;
+
+import javax.annotation.Nullable;
+
+@Deprecated
+public class LegacyBaseBlockWrapper extends BaseBlock {
+    protected LegacyBaseBlockWrapper(BlockState blockState) {
+        super(blockState);
+    }
+
+    // These two methods force the legacy blocks to use the old NBT methods.
+    @Nullable
+    @Override
+    public LazyReference<LinCompoundTag> getNbtReference() {
+        CompoundTag nbtData = getNbtData();
+        return nbtData == null ? null : LazyReference.from(nbtData::toLinTag);
+    }
+
+    @Override
+    public void setNbtReference(@Nullable LazyReference<LinCompoundTag> nbtData) {
+        setNbtData(nbtData == null ? null : new CompoundTag(nbtData.getValue()));
+    }
+}

worldedit-core/src/legacy/java/com/sk89q/worldedit/blocks/MobSpawnerBlock.java

@@ -28,7 +28,6 @@ import com.sk89q.jnbt.NBTUtils;
 import com.sk89q.jnbt.ShortTag;
 import com.sk89q.jnbt.StringTag;
 import com.sk89q.jnbt.Tag;
-import com.sk89q.worldedit.world.block.BaseBlock;
 import com.sk89q.worldedit.world.block.BlockState;
 import com.sk89q.worldedit.world.storage.InvalidFormatException;
 
@@ -42,7 +41,7 @@ import java.util.Map;
  *         deprecated for removal without replacement
  */
 @Deprecated(forRemoval = true)
-public class MobSpawnerBlock extends BaseBlock {
+public class MobSpawnerBlock extends LegacyBaseBlockWrapper {
 
     private String mobType;
     private short delay = -1;
@@ -114,6 +113,7 @@ public class MobSpawnerBlock extends BaseBlock {
     }
 
     @Override
+    @Deprecated
     public boolean hasNbtData() {
         return true;
     }
@@ -124,6 +124,7 @@ public class MobSpawnerBlock extends BaseBlock {
     }
 
     @Override
+    @Deprecated
     public CompoundTag getNbtData() {
         Map<String, Tag<?, ?>> values = new HashMap<>();
         values.put("Delay", new ShortTag(delay));
@@ -165,6 +166,7 @@ public class MobSpawnerBlock extends BaseBlock {
     }
 
     @Override
+    @Deprecated
     public void setNbtData(CompoundTag rootTag) {
         if (rootTag == null) {
             return;
@@ -184,7 +186,7 @@ public class MobSpawnerBlock extends BaseBlock {
         try {
             spawnDataTag = NBTUtils.getChildTag(values, "SpawnData", CompoundTag.class);
             mobType = spawnDataTag.getString("id");
-            if (mobType.equals("")) {
+            if (mobType.isEmpty()) {
                 throw new InvalidFormatException("No spawn id.");
             }
             this.mobType = mobType;

worldedit-core/src/legacy/java/com/sk89q/worldedit/blocks/SignBlock.java

@@ -37,8 +37,12 @@ import java.util.stream.Collectors;
 
 /**
  * Represents a sign block.
+ *
+ * @deprecated WorldEdit does not handle interpreting NBT,
+ *     deprecated for removal without replacement
  */
-public class SignBlock extends BaseBlock {
+@Deprecated
+public class SignBlock extends LegacyBaseBlockWrapper {
 
     private String[] text;
 
@@ -93,6 +97,7 @@ public class SignBlock extends BaseBlock {
     }
 
     @Override
+    @Deprecated
     public boolean hasNbtData() {
         return true;
     }
@@ -103,6 +108,7 @@ public class SignBlock extends BaseBlock {
     }
 
     @Override
+    @Deprecated
     public CompoundTag getNbtData() {
         Map<String, Tag<?, ?>> values = new HashMap<>();
         if (isLegacy()) {
@@ -111,7 +117,7 @@ public class SignBlock extends BaseBlock {
             values.put("Text3", new StringTag(text[2]));
             values.put("Text4", new StringTag(text[3]));
         } else {
-            ListTag messages = new ListTag(StringTag.class, Arrays.stream(text).map(StringTag::new).collect(Collectors.toList()));
+            ListTag<?, ?> messages = new ListTag<>(StringTag.class, Arrays.stream(text).map(StringTag::new).collect(Collectors.toList()));
             Map<String, Tag<?, ?>> frontTextTag = new HashMap<>();
             frontTextTag.put("messages", messages);
             values.put("front_text", new CompoundTag(frontTextTag));
@@ -120,6 +126,7 @@ public class SignBlock extends BaseBlock {
     }
 
     @Override
+    @Deprecated
     public void setNbtData(CompoundTag rootTag) {
         if (rootTag == null) {
             return;
@@ -158,7 +165,7 @@ public class SignBlock extends BaseBlock {
             }
         } else {
             CompoundTag frontTextTag = (CompoundTag) values.get("front_text");
-            ListTag messagesTag = frontTextTag.getListTag("messages");
+            ListTag<?, ?> messagesTag = frontTextTag.getListTag("messages");
             for (int i = 0; i < messagesTag.getValue().size(); i++) {
                 StringTag tag = (StringTag) messagesTag.getValue().get(i);
                 text[i] = tag.getValue();

worldedit-core/src/legacy/java/com/sk89q/worldedit/blocks/SkullBlock.java

@@ -23,7 +23,6 @@ import com.sk89q.jnbt.CompoundTag;
 import com.sk89q.jnbt.StringTag;
 import com.sk89q.jnbt.Tag;
 import com.sk89q.worldedit.internal.util.DeprecationUtil;
-import com.sk89q.worldedit.world.block.BaseBlock;
 import com.sk89q.worldedit.world.block.BlockState;
 
 import java.util.HashMap;
@@ -36,7 +35,7 @@ import java.util.Map;
  *         deprecated for removal without replacement
  */
 @Deprecated(forRemoval = true)
-public class SkullBlock extends BaseBlock {
+public class SkullBlock extends LegacyBaseBlockWrapper {
 
     private String owner = ""; // notchian
 
@@ -89,6 +88,7 @@ public class SkullBlock extends BaseBlock {
     }
 
     @Override
+    @Deprecated
     public boolean hasNbtData() {
         return true;
     }
@@ -99,6 +99,7 @@ public class SkullBlock extends BaseBlock {
     }
 
     @Override
+    @Deprecated
     public CompoundTag getNbtData() {
         Map<String, Tag<?, ?>> values = new HashMap<>();
         Map<String, Tag<?, ?>> inner = new HashMap<>();
@@ -108,6 +109,7 @@ public class SkullBlock extends BaseBlock {
     }
 
     @Override
+    @Deprecated
     public void setNbtData(CompoundTag rootTag) {
         if (rootTag == null) {
             return;

worldedit-core/src/main/java/com/sk89q/worldedit/extension/factory/parser/DefaultBlockParser.java

@@ -48,13 +48,13 @@ import com.sk89q.worldedit.extension.platform.Actor;
 import com.sk89q.worldedit.extension.platform.Capability;
 import com.sk89q.worldedit.extent.inventory.BlockBag;
 import com.sk89q.worldedit.internal.registry.InputParser;
+import com.sk89q.worldedit.internal.util.DeprecationUtil;
 import com.sk89q.worldedit.math.BlockVector3;
 import com.sk89q.worldedit.registry.state.Property;
 import com.sk89q.worldedit.util.HandSide;
 import com.sk89q.worldedit.util.formatting.text.TextComponent;
 import com.sk89q.worldedit.world.World;
 import com.sk89q.worldedit.world.block.BaseBlock;
-import com.sk89q.worldedit.world.block.BlockCategories;
 import com.sk89q.worldedit.world.block.BlockState;
 import com.sk89q.worldedit.world.block.BlockStateHolder;
 import com.sk89q.worldedit.world.block.BlockType;
@@ -538,13 +538,7 @@ public class DefaultBlockParser extends InputParser<BaseBlock> {
             //FAWE end
         }
 
-        if (nbt != null) {
+        if (DeprecationUtil.isSign(blockType)) {
-            BaseBlock result = blockStates.size() > 0 ? state.toBaseBlock(nbt) : new BlanketBaseBlock(state, nbt);
-            return validate(context, result);
-        }
-
-        if (blockType == BlockTypes.SIGN || blockType == BlockTypes.WALL_SIGN
-                || BlockCategories.SIGNS.contains(blockType)) {
             // Allow special sign text syntax
             String[] text = new String[4];
             text[0] = blockAndExtraData.length > 1 ? blockAndExtraData[1] : "";
@@ -552,10 +546,11 @@ public class DefaultBlockParser extends InputParser<BaseBlock> {
             text[2] = blockAndExtraData.length > 3 ? blockAndExtraData[3] : "";
             text[3] = blockAndExtraData.length > 4 ? blockAndExtraData[4] : "";
             return validate(context, new SignBlock(state, text));
-        } else if (blockType == BlockTypes.SPAWNER) {
+        } else if (blockType == BlockTypes.SPAWNER && (blockAndExtraData.length > 1 || nbt != null)) {
             // Allow setting mob spawn type
+            String mobName;
             if (blockAndExtraData.length > 1) {
-                String mobName = blockAndExtraData[1];
+                mobName = blockAndExtraData[1];
                 EntityType ent = EntityTypes.get(mobName.toLowerCase(Locale.ROOT));
                 if (ent == null) {
                     throw new NoMatchException(Caption.of("worldedit.error.unknown-entity", TextComponent.of(mobName)));
@@ -564,14 +559,13 @@ public class DefaultBlockParser extends InputParser<BaseBlock> {
                 if (!worldEdit.getPlatformManager().queryCapability(Capability.USER_COMMANDS).isValidMobType(mobName)) {
                     throw new NoMatchException(Caption.of("worldedit.error.unknown-mob", TextComponent.of(mobName)));
                 }
-                return validate(context, new MobSpawnerBlock(state, mobName));
             } else {
-                //noinspection ConstantConditions
+                mobName = EntityTypes.PIG.id();
-                return validate(context, new MobSpawnerBlock(state, EntityTypes.PIG.id()));
             }
-        } else if (blockType == BlockTypes.PLAYER_HEAD || blockType == BlockTypes.PLAYER_WALL_HEAD) {
+            return validate(context, new MobSpawnerBlock(state, mobName));
+        } else if ((blockType == BlockTypes.PLAYER_HEAD || blockType == BlockTypes.PLAYER_WALL_HEAD) && (blockAndExtraData.length > 1 || nbt != null)) {
             // allow setting type/player/rotation
-            if (blockAndExtraData.length <= 1) {
+            if (blockAndExtraData.length == 1) {
                 return validate(context, new SkullBlock(state));
             }
 
@@ -580,12 +574,14 @@ public class DefaultBlockParser extends InputParser<BaseBlock> {
             return validate(context, new SkullBlock(state, type.replace(" ", "_"))); // valid MC usernames
         } else {
             //FAWE start
-            nbt = state.getNbtData();
+            if (nbt == null) {
+                nbt = state.getNbtData();
+            }
             BaseBlock result;
             if (nbt != null) {
-                result = blockStates.size() > 0 ? state.toBaseBlock(nbt) : new BlanketBaseBlock(state, nbt);
+                result = !blockStates.isEmpty() ? state.toBaseBlock(nbt) : new BlanketBaseBlock(state, nbt);
             } else {
-                result = blockStates.size() > 0 ? new BaseBlock(state) : state.toBaseBlock();
+                result = !blockStates.isEmpty() ? new BaseBlock(state) : state.toBaseBlock();
             }
             return validate(context, result);
             //FAWE end