From 0f6ff6e17efc46ef1b230ae628f755b2e8073c1f Mon Sep 17 00:00:00 2001 From: Nowko Date: Sun, 11 Oct 2026 21:06:31 +0200 Subject: [PATCH] perf: trim per-tick and per-click studio work - Jukebox playback reads the cheap block type every tick and the record, which copies the block state and item, once a second. Ejecting or breaking the jukebox still stops playback immediately. - The jukebox click handler checks the block type before getState(), so right-clicking chests no longer copies their inventories. - ItemsAdder station lookups resolve their reflective methods once instead of on every right-click and menu refresh. - Saving serializes the project once for both copies. - Startup removes studio-*.tmp files left by an interrupted write. Co-Authored-By: Claude Opus 5.5 --- .../studio/StudioListener.java | 2 ++ .../studio/StudioService.java | 9 +++++-- .../studio/StudioStation.java | 24 +++++++++++++++++-- .../studio/StudioStore.java | 21 ++++++++++++---- .../studio/StudioListenerTest.java | 11 +++++++++ .../studio/StudioServiceTest.java | 16 +++++++++++++ .../studio/StudioStoreTest.java | 15 ++++++++++++ 7 files changed, 90 insertions(+), 8 deletions(-) diff --git a/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioListener.java b/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioListener.java index 3a75756..867c763 100644 --- a/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioListener.java +++ b/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioListener.java @@ -34,6 +34,8 @@ public StudioListener(StudioService studio, StudioMenu menu) { public void interact(PlayerInteractEvent event) { if (event.getAction() != Action.RIGHT_CLICK_BLOCK || event.getClickedBlock() == null || event.useInteractedBlock() == Event.Result.DENY + // Check the type first: getState() copies block entities such as chest inventories. + || event.getClickedBlock().getType() != Material.JUKEBOX || !(event.getClickedBlock().getState() instanceof Jukebox box)) { return; } diff --git a/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioService.java b/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioService.java index ba8cf2c..15a0238 100644 --- a/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioService.java +++ b/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioService.java @@ -597,8 +597,9 @@ private void tick() { } Block block = world.getBlockAt(key.x, key.y, key.z); int elapsed = (int) (clock - playback.start); - if (!(block.getState() instanceof Jukebox box) - || !playback.song.id().equals(discs.songId(box.getRecord())) + // The block type is cheap to read every tick. Reading the record copies the block state and + // item, so it is checked once a second; ejection and breaking stop playback immediately. + if (block.getType() != Material.JUKEBOX || elapsed % 20 == 0 && !holds(block, playback.song) || elapsed >= playback.song.lengthTicks() + settings.tailTicks()) { it.remove(); continue; @@ -619,6 +620,10 @@ private void tick() { } } + private boolean holds(Block block, Song song) { + return block.getState() instanceof Jukebox box && song.id().equals(discs.songId(box.getRecord())); + } + private void emit(Player player, List due) { for (Timeline.Trigger trigger : due) { privateSounds.computeIfAbsent(player.getUniqueId(), key -> new HashSet<>()).add(trigger.note().sound()); diff --git a/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioStation.java b/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioStation.java index 34b062d..ac9d23d 100644 --- a/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioStation.java +++ b/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioStation.java @@ -9,7 +9,10 @@ import org.bukkit.plugin.Plugin; import org.bukkit.util.BoundingBox; +import java.lang.reflect.Method; import java.util.Locale; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; import java.util.logging.Logger; /** @@ -20,6 +23,7 @@ public final class StudioStation { /** Squared reach within which a player can keep using the station's controls. */ private static final double REACH_SQUARED = 36; + private static final Map METHODS = new ConcurrentHashMap<>(); private final Material material; private final String itemsAdderId; @@ -135,13 +139,29 @@ public Block require(Player player, Location location) { // CustomBlock.byAlreadyPlaced(block) / CustomFurniture.byAlreadySpawned(entity), then getNamespacedID(). private static String namespacedId(String type, String lookup, Class parameter, Object target) { try { - Object custom = Class.forName(type).getMethod(lookup, parameter).invoke(null, target); - return custom == null ? null : (String) custom.getClass().getMethod("getNamespacedID").invoke(custom); + Object custom = method(type + "#" + lookup + "#" + parameter.getName(), + () -> Class.forName(type).getMethod(lookup, parameter)).invoke(null, target); + return custom == null ? null : (String) method(custom.getClass().getName() + "#getNamespacedID", + () -> custom.getClass().getMethod("getNamespacedID")).invoke(custom); } catch (ReflectiveOperationException | RuntimeException ex) { return null; } } + @FunctionalInterface + private interface Lookup { Method find() throws ReflectiveOperationException; } + + // matches(Block) runs on every right-click on a block and every studio menu refresh, so each + // reflective lookup is resolved once. Failures are not cached: ItemsAdder may enable later. + private static Method method(String key, Lookup lookup) throws ReflectiveOperationException { + Method method = METHODS.get(key); + if (method == null) { + method = lookup.find(); + METHODS.put(key, method); + } + return method; + } + private static boolean pluginEnabled(String name) { Plugin plugin = Bukkit.getPluginManager().getPlugin(name); return plugin != null && plugin.isEnabled(); diff --git a/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioStore.java b/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioStore.java index ae4a69d..6bc7cb9 100644 --- a/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioStore.java +++ b/src/main/java/net/tfminecraft/musicalinstruments/studio/StudioStore.java @@ -38,6 +38,15 @@ public StudioStore(Path root) throws IOException { this.root = root; Files.createDirectories(root.resolve("projects")); Files.createDirectories(root.resolve("songs")); + // A crash between writing and moving leaves a temporary file; it never holds the only copy. + try (var files = Files.walk(root, 3)) { + for (Path file : files.filter(StudioStore::temporary).toList()) Files.deleteIfExists(file); + } + } + + private static boolean temporary(Path file) { + String name = file.getFileName().toString(); + return name.startsWith("studio-") && name.endsWith(".tmp"); } public Optional project(UUID owner) throws IOException { @@ -214,10 +223,11 @@ public void save(Project project) throws IOException { Path previousFile = projectFile(owner, previous.song().id()); if (!Files.exists(previousFile)) write(previousFile, encodeProject(previous), true); } - YamlConfiguration yaml = encodeProject(project); + // Both copies hold the same bytes, so the project is serialized once. + byte[] contents = encodeProject(project).saveToString().getBytes(StandardCharsets.UTF_8); Path file = projectFile(owner, project.song().id()); - write(file, yaml, true); - write(selectedFile, yaml, true); + write(file, contents, true); + write(selectedFile, contents, true); archived.add(owner); remember(project, file, selectedFile); } @@ -269,10 +279,13 @@ private YamlConfiguration read(Path file) throws IOException { } private void write(Path file, YamlConfiguration yaml, boolean replace) throws IOException { + write(file, yaml.saveToString().getBytes(StandardCharsets.UTF_8), replace); + } + + private void write(Path file, byte[] contents, boolean replace) throws IOException { if (!replace && Files.exists(file)) { throw new IOException("Published editions cannot be overwritten"); } - byte[] contents = yaml.saveToString().getBytes(StandardCharsets.UTF_8); if (contents.length > MAX_FILE_BYTES) { throw new IOException("Studio file exceeds the size limit: " + file.getFileName()); } diff --git a/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioListenerTest.java b/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioListenerTest.java index bf3ac36..6fde912 100644 --- a/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioListenerTest.java +++ b/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioListenerTest.java @@ -47,6 +47,7 @@ void setup() { when(studio.discs()).thenReturn(discs); when(studio.station()).thenReturn(StudioStation.none()); when(block.getState()).thenReturn(box); + when(block.getType()).thenReturn(Material.JUKEBOX); when(player.getInventory()).thenReturn(inventory); when(player.hasPermission("instruments.play")).thenReturn(true); when(discs.custom(disc)).thenReturn(true); @@ -164,6 +165,16 @@ void rightClickingTheStationOpensTheStudioOnceAndKeepsTheKeyboardClosed() throws assertEquals(Event.Result.DENY, off.useItemInHand()); } + @Test + void clicksOnOtherBlocksDoNotCopyTheirBlockState() throws Exception { + when(block.getType()).thenReturn(Material.CHEST); + PlayerInteractEvent click = event(EquipmentSlot.HAND); + listener.interact(click); + verify(block, never()).getState(); + verify(studio, never()).play(any(), any()); + assertNotEquals(Event.Result.DENY, click.useItemInHand()); + } + @Test void otherBlocksClicksAndSneakPlacementKeepVanillaBehaviour() throws Exception { when(studio.station()).thenReturn(StudioStation.vanilla(Material.LOOM)); diff --git a/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioServiceTest.java b/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioServiceTest.java index 4401d92..b77c5d2 100644 --- a/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioServiceTest.java +++ b/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioServiceTest.java @@ -668,6 +668,22 @@ private Block jukebox(Song song, ItemStack disc) throws IOException { return block; } + @Test + void jukeboxRecordIsCheckedOnceASecondAndARemovedJukeboxStopsAtOnce() throws Exception { + Song song = song(new Track(1, 60, 1, false, List.of(new Note(0, "lute", "first", 4, 1), + new Note(5, "lute", "second", 4, 1), new Note(25, "lute", "third", 4, 1)))); + ItemStack disc = mock(ItemStack.class); + Block block = jukebox(song, disc); + studio.play(block, disc); + clearInvocations(block); + for (int elapsed = 0; elapsed <= 5; elapsed++) tick.run(); + verify(block, times(1)).getState(); + verify(player).playSound(any(Location.class), eq("second"), eq(SoundCategory.RECORDS), anyFloat(), anyFloat()); + when(block.getType()).thenReturn(Material.AIR); + for (int elapsed = 6; elapsed <= 25; elapsed++) tick.run(); + verify(player, never()).playSound(any(Location.class), eq("third"), any(SoundCategory.class), anyFloat(), anyFloat()); + } + @Test void jukeboxAppliesItsSnapshotAndPlaysEveryTrackOnlyToNearbyListeners() throws Exception { Song song = song(track(1, "lute"), track(2, "flute")); diff --git a/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioStoreTest.java b/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioStoreTest.java index 6c13709..4c6f3d3 100644 --- a/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioStoreTest.java +++ b/src/test/java/net/tfminecraft/musicalinstruments/studio/StudioStoreTest.java @@ -37,6 +37,21 @@ private Project project() { return new Project(new Song(UUID.randomUUID(), UUID.randomUUID(), "Musician", "Música", List.of(old)), pending, 120, false); } + @Test + void startupRemovesTemporaryFilesLeftByAnInterruptedWrite() throws Exception { + Project project = project(); + new StudioStore(directory).save(project); + Path folder = directory.resolve("projects").resolve(project.song().owner().toString()); + Path leftover = Files.writeString(folder.resolve("studio-123.tmp"), "partial"); + Path unrelated = Files.writeString(folder.resolve("notes.tmp"), "kept"); + Path similar = Files.writeString(directory.resolve("songs").resolve("studio-readme.txt"), "kept"); + StudioStore reopened = new StudioStore(directory); + assertFalse(Files.exists(leftover)); + assertTrue(Files.exists(unrelated)); + assertTrue(Files.exists(similar)); + assertEquals(project, reopened.project(project.song().owner()).orElseThrow()); + } + @Test void projectsAndPendingTakesSurviveANewStoreInstance() throws Exception { Project project = project();