From 0e5248e5e64d2871736a8428b6ecf3878adee4ab Mon Sep 17 00:00:00 2001
From: Jonathan Coates <git@squiddev.cc>
Date: Sun, 24 Mar 2024 12:53:57 +0000
Subject: [PATCH] Prevent playing music discs with speaker.playSound

I have mixed feelings about speaker.playSound. On one hand, it's pretty
useful to be able to play any sound. On the other, it sometimes feels
... maybe a little too magic?

One particular thing I don't like is that it allows you to play
arbitrary records, which sidesteps both a vanilla mechanic (finding
record discs) and existing CC functionality (disk.playAudio). We now
prevent playing record tracks from the speaker.
---
 .../peripheral/speaker/SpeakerPeripheral.java |  18 ++-
 .../computercraft/gametest/Speaker_Test.kt    |  11 ++
 .../speaker_test.will_not_play_record.snbt    | 138 ++++++++++++++++++
 3 files changed, 159 insertions(+), 8 deletions(-)
 create mode 100644 projects/common/src/testMod/resources/data/cctest/structures/speaker_test.will_not_play_record.snbt

diff --git a/projects/common/src/main/java/dan200/computercraft/shared/peripheral/speaker/SpeakerPeripheral.java b/projects/common/src/main/java/dan200/computercraft/shared/peripheral/speaker/SpeakerPeripheral.java
index df995fb6d..8a081deb6 100644
--- a/projects/common/src/main/java/dan200/computercraft/shared/peripheral/speaker/SpeakerPeripheral.java
+++ b/projects/common/src/main/java/dan200/computercraft/shared/peripheral/speaker/SpeakerPeripheral.java
@@ -18,16 +18,18 @@
 import dan200.computercraft.shared.network.client.SpeakerPlayClientMessage;
 import dan200.computercraft.shared.network.client.SpeakerStopClientMessage;
 import dan200.computercraft.shared.network.server.ServerNetworking;
+import dan200.computercraft.shared.platform.PlatformHelper;
 import dan200.computercraft.shared.util.PauseAwareTimer;
-import net.minecraft.ResourceLocationException;
 import net.minecraft.core.BlockPos;
 import net.minecraft.core.Holder;
+import net.minecraft.core.registries.Registries;
 import net.minecraft.network.protocol.game.ClientboundSoundPacket;
 import net.minecraft.resources.ResourceLocation;
 import net.minecraft.server.level.ServerLevel;
 import net.minecraft.sounds.SoundEvent;
 import net.minecraft.sounds.SoundSource;
 import net.minecraft.util.Mth;
+import net.minecraft.world.item.RecordItem;
 import net.minecraft.world.level.block.state.properties.NoteBlockInstrument;
 
 import javax.annotation.Nullable;
@@ -252,15 +254,15 @@ public final boolean playSound(ILuaContext context, String name, Optional<Double
         var volume = (float) clampVolume(checkFinite(1, volumeA.orElse(1.0)));
         var pitch = (float) checkFinite(2, pitchA.orElse(1.0));
 
-        ResourceLocation identifier;
-        try {
-            identifier = new ResourceLocation(name);
-        } catch (ResourceLocationException e) {
-            throw new LuaException("Malformed sound name '" + name + "' ");
-        }
+        var identifier = ResourceLocation.tryParse(name);
+        if (identifier == null) throw new LuaException("Malformed sound name '" + name + "' ");
+
+        // Prevent playing music discs.
+        var soundEvent = PlatformHelper.get().tryGetRegistryObject(Registries.SOUND_EVENT, identifier);
+        if (soundEvent != null && RecordItem.getBySound(soundEvent) != null) return false;
 
         synchronized (lock) {
-            if (pendingSound != null | (dfpwmState != null && dfpwmState.isPlaying())) return false;
+            if (pendingSound != null || (dfpwmState != null && dfpwmState.isPlaying())) return false;
             dfpwmState = null;
             pendingSound = new PendingSound<>(identifier, volume, pitch);
             return true;
diff --git a/projects/common/src/testMod/kotlin/dan200/computercraft/gametest/Speaker_Test.kt b/projects/common/src/testMod/kotlin/dan200/computercraft/gametest/Speaker_Test.kt
index be8cab748..8a2d9572f 100644
--- a/projects/common/src/testMod/kotlin/dan200/computercraft/gametest/Speaker_Test.kt
+++ b/projects/common/src/testMod/kotlin/dan200/computercraft/gametest/Speaker_Test.kt
@@ -29,4 +29,15 @@ fun Fails_to_play_multiple_sounds(helper: GameTestHelper) = helper.sequence {
             }
         }
     }
+
+    /**
+     * [SpeakerPeripheral.playSound] will not play records.
+     */
+    @GameTest
+    fun Will_not_play_record(helper: GameTestHelper) = helper.sequence {
+        thenOnComputer {
+            callPeripheral("right", "playSound", SoundEvents.MUSIC_DISC_PIGSTEP.location.toString())
+                .assertArrayEquals(false)
+        }
+    }
 }
diff --git a/projects/common/src/testMod/resources/data/cctest/structures/speaker_test.will_not_play_record.snbt b/projects/common/src/testMod/resources/data/cctest/structures/speaker_test.will_not_play_record.snbt
new file mode 100644
index 000000000..eb2a11850
--- /dev/null
+++ b/projects/common/src/testMod/resources/data/cctest/structures/speaker_test.will_not_play_record.snbt
@@ -0,0 +1,138 @@
+{
+    DataVersion: 3465,
+    size: [5, 5, 5],
+    data: [
+        {pos: [0, 0, 0], state: "minecraft:polished_andesite"},
+        {pos: [0, 0, 1], state: "minecraft:polished_andesite"},
+        {pos: [0, 0, 2], state: "minecraft:polished_andesite"},
+        {pos: [0, 0, 3], state: "minecraft:polished_andesite"},
+        {pos: [0, 0, 4], state: "minecraft:polished_andesite"},
+        {pos: [1, 0, 0], state: "minecraft:polished_andesite"},
+        {pos: [1, 0, 1], state: "minecraft:polished_andesite"},
+        {pos: [1, 0, 2], state: "minecraft:polished_andesite"},
+        {pos: [1, 0, 3], state: "minecraft:polished_andesite"},
+        {pos: [1, 0, 4], state: "minecraft:polished_andesite"},
+        {pos: [2, 0, 0], state: "minecraft:polished_andesite"},
+        {pos: [2, 0, 1], state: "minecraft:polished_andesite"},
+        {pos: [2, 0, 2], state: "minecraft:polished_andesite"},
+        {pos: [2, 0, 3], state: "minecraft:polished_andesite"},
+        {pos: [2, 0, 4], state: "minecraft:polished_andesite"},
+        {pos: [3, 0, 0], state: "minecraft:polished_andesite"},
+        {pos: [3, 0, 1], state: "minecraft:polished_andesite"},
+        {pos: [3, 0, 2], state: "minecraft:polished_andesite"},
+        {pos: [3, 0, 3], state: "minecraft:polished_andesite"},
+        {pos: [3, 0, 4], state: "minecraft:polished_andesite"},
+        {pos: [4, 0, 0], state: "minecraft:polished_andesite"},
+        {pos: [4, 0, 1], state: "minecraft:polished_andesite"},
+        {pos: [4, 0, 2], state: "minecraft:polished_andesite"},
+        {pos: [4, 0, 3], state: "minecraft:polished_andesite"},
+        {pos: [4, 0, 4], state: "minecraft:polished_andesite"},
+        {pos: [0, 1, 0], state: "minecraft:air"},
+        {pos: [0, 1, 1], state: "minecraft:air"},
+        {pos: [0, 1, 2], state: "minecraft:air"},
+        {pos: [0, 1, 3], state: "minecraft:air"},
+        {pos: [0, 1, 4], state: "minecraft:air"},
+        {pos: [1, 1, 0], state: "minecraft:air"},
+        {pos: [1, 1, 1], state: "minecraft:air"},
+        {pos: [1, 1, 2], state: "computercraft:speaker{facing:north}", nbt: {id: "computercraft:speaker"}},
+        {pos: [1, 1, 3], state: "minecraft:air"},
+        {pos: [1, 1, 4], state: "minecraft:air"},
+        {pos: [2, 1, 0], state: "minecraft:air"},
+        {pos: [2, 1, 1], state: "minecraft:air"},
+        {pos: [2, 1, 2], state: "computercraft:computer_normal{facing:north,state:on}", nbt: {ComputerId: 1, Label: "speaker_test.will_not_play_record", On: 1b, id: "computercraft:computer_normal"}},
+        {pos: [2, 1, 3], state: "minecraft:air"},
+        {pos: [2, 1, 4], state: "minecraft:air"},
+        {pos: [3, 1, 0], state: "minecraft:air"},
+        {pos: [3, 1, 1], state: "minecraft:air"},
+        {pos: [3, 1, 2], state: "minecraft:air"},
+        {pos: [3, 1, 3], state: "minecraft:air"},
+        {pos: [3, 1, 4], state: "minecraft:air"},
+        {pos: [4, 1, 0], state: "minecraft:air"},
+        {pos: [4, 1, 1], state: "minecraft:air"},
+        {pos: [4, 1, 2], state: "minecraft:air"},
+        {pos: [4, 1, 3], state: "minecraft:air"},
+        {pos: [4, 1, 4], state: "minecraft:air"},
+        {pos: [0, 2, 0], state: "minecraft:air"},
+        {pos: [0, 2, 1], state: "minecraft:air"},
+        {pos: [0, 2, 2], state: "minecraft:air"},
+        {pos: [0, 2, 3], state: "minecraft:air"},
+        {pos: [0, 2, 4], state: "minecraft:air"},
+        {pos: [1, 2, 0], state: "minecraft:air"},
+        {pos: [1, 2, 1], state: "minecraft:air"},
+        {pos: [1, 2, 2], state: "minecraft:air"},
+        {pos: [1, 2, 3], state: "minecraft:air"},
+        {pos: [1, 2, 4], state: "minecraft:air"},
+        {pos: [2, 2, 0], state: "minecraft:air"},
+        {pos: [2, 2, 1], state: "minecraft:air"},
+        {pos: [2, 2, 2], state: "minecraft:air"},
+        {pos: [2, 2, 3], state: "minecraft:air"},
+        {pos: [2, 2, 4], state: "minecraft:air"},
+        {pos: [3, 2, 0], state: "minecraft:air"},
+        {pos: [3, 2, 1], state: "minecraft:air"},
+        {pos: [3, 2, 2], state: "minecraft:air"},
+        {pos: [3, 2, 3], state: "minecraft:air"},
+        {pos: [3, 2, 4], state: "minecraft:air"},
+        {pos: [4, 2, 0], state: "minecraft:air"},
+        {pos: [4, 2, 1], state: "minecraft:air"},
+        {pos: [4, 2, 2], state: "minecraft:air"},
+        {pos: [4, 2, 3], state: "minecraft:air"},
+        {pos: [4, 2, 4], state: "minecraft:air"},
+        {pos: [0, 3, 0], state: "minecraft:air"},
+        {pos: [0, 3, 1], state: "minecraft:air"},
+        {pos: [0, 3, 2], state: "minecraft:air"},
+        {pos: [0, 3, 3], state: "minecraft:air"},
+        {pos: [0, 3, 4], state: "minecraft:air"},
+        {pos: [1, 3, 0], state: "minecraft:air"},
+        {pos: [1, 3, 1], state: "minecraft:air"},
+        {pos: [1, 3, 2], state: "minecraft:air"},
+        {pos: [1, 3, 3], state: "minecraft:air"},
+        {pos: [1, 3, 4], state: "minecraft:air"},
+        {pos: [2, 3, 0], state: "minecraft:air"},
+        {pos: [2, 3, 1], state: "minecraft:air"},
+        {pos: [2, 3, 2], state: "minecraft:air"},
+        {pos: [2, 3, 3], state: "minecraft:air"},
+        {pos: [2, 3, 4], state: "minecraft:air"},
+        {pos: [3, 3, 0], state: "minecraft:air"},
+        {pos: [3, 3, 1], state: "minecraft:air"},
+        {pos: [3, 3, 2], state: "minecraft:air"},
+        {pos: [3, 3, 3], state: "minecraft:air"},
+        {pos: [3, 3, 4], state: "minecraft:air"},
+        {pos: [4, 3, 0], state: "minecraft:air"},
+        {pos: [4, 3, 1], state: "minecraft:air"},
+        {pos: [4, 3, 2], state: "minecraft:air"},
+        {pos: [4, 3, 3], state: "minecraft:air"},
+        {pos: [4, 3, 4], state: "minecraft:air"},
+        {pos: [0, 4, 0], state: "minecraft:air"},
+        {pos: [0, 4, 1], state: "minecraft:air"},
+        {pos: [0, 4, 2], state: "minecraft:air"},
+        {pos: [0, 4, 3], state: "minecraft:air"},
+        {pos: [0, 4, 4], state: "minecraft:air"},
+        {pos: [1, 4, 0], state: "minecraft:air"},
+        {pos: [1, 4, 1], state: "minecraft:air"},
+        {pos: [1, 4, 2], state: "minecraft:air"},
+        {pos: [1, 4, 3], state: "minecraft:air"},
+        {pos: [1, 4, 4], state: "minecraft:air"},
+        {pos: [2, 4, 0], state: "minecraft:air"},
+        {pos: [2, 4, 1], state: "minecraft:air"},
+        {pos: [2, 4, 2], state: "minecraft:air"},
+        {pos: [2, 4, 3], state: "minecraft:air"},
+        {pos: [2, 4, 4], state: "minecraft:air"},
+        {pos: [3, 4, 0], state: "minecraft:air"},
+        {pos: [3, 4, 1], state: "minecraft:air"},
+        {pos: [3, 4, 2], state: "minecraft:air"},
+        {pos: [3, 4, 3], state: "minecraft:air"},
+        {pos: [3, 4, 4], state: "minecraft:air"},
+        {pos: [4, 4, 0], state: "minecraft:air"},
+        {pos: [4, 4, 1], state: "minecraft:air"},
+        {pos: [4, 4, 2], state: "minecraft:air"},
+        {pos: [4, 4, 3], state: "minecraft:air"},
+        {pos: [4, 4, 4], state: "minecraft:air"}
+    ],
+    entities: [],
+    palette: [
+        "minecraft:polished_andesite",
+        "minecraft:air",
+        "computercraft:speaker{facing:north}",
+        "computercraft:computer_normal{facing:north,state:on}"
+    ]
+}