diff --git a/src/main/java/electroblob/wizardry/client/audio/SoundLoop.java b/src/main/java/electroblob/wizardry/client/audio/SoundLoop.java index 651fb7fb..3bce8e3e 100644 --- a/src/main/java/electroblob/wizardry/client/audio/SoundLoop.java +++ b/src/main/java/electroblob/wizardry/client/audio/SoundLoop.java @@ -27,6 +27,16 @@ public abstract class SoundLoop implements ITickable { private static final Set activeLoops = new HashSet<>(); + // The issue we had here was that there is no way of hooking into the client tick loop between the sound manager + // update and the world tick (other than via the sounds themselves, but playing other sounds from there results in + // CMEs, which is why we made this class in the first place). This meant there was always a gap between consecutive + // sounds, which could be heard in-game as a weird audio artefact. The solution is simply to overlap the sounds by + // 1 tick, which is fairly straightforward for the end sound, but because the start sound has to finish for us to + // know when to play the others, this doubling-up solution is required (bit of a waste of resources but meh). + /** A dummy sound with effectively zero volume, played one tick in advance of the actual start sound so the loop + * sound can start one tick before the actual start sound ends. */ + private final ISound dummyStart; // It's the stupidest system ever but it works! + private final ISound start; private final ISound loop; private final ISound end; @@ -34,29 +44,28 @@ public abstract class SoundLoop implements ITickable { private boolean looping = false; private boolean needsRemoving = false; - public SoundLoop(SoundEvent start, SoundEvent loop, SoundEvent end, SoundCategory category, ISoundFactory factory){ + public SoundLoop(SoundEvent start, SoundEvent loop, SoundEvent end, SoundCategory category, float volume, ISoundFactory factory){ // The reason I've gone to the effort of having a factory for these is that we need SoundLoop to have control // over which sounds are repeated and which aren't whilst keeping them private. - this.start = factory.create(start, category, false); - this.loop = factory.create(loop, category, true); - this.end = factory.create(end, category, false); + this.dummyStart = factory.create(start, category, 0.00001f, false); // Inaudible but not actually 0 + this.start = factory.create(start, category, volume, false); + this.loop = factory.create(loop, category, volume, true); + this.end = factory.create(end, category, volume, false); } @Override public void update(){ // Check every tick if the start sound is done playing and if so, start the loop sound - if(!looping && !Minecraft.getMinecraft().getSoundHandler().isSoundPlaying(start)){ + if(!looping && !Minecraft.getMinecraft().getSoundHandler().isSoundPlaying(dummyStart)){ Minecraft.getMinecraft().getSoundHandler().playSound(loop); looping = true; } } - /** Stops the loop part of the sound immediately and starts playing the end part. This may be called from subclasses - * or externally depending on the implementation. */ + /** Starts playing the end part immediately and marks the loop part to stop next tick. This may be called from + * subclasses or externally depending on the implementation. */ // For continuous spell sounds it's internally public void endLoop(){ - Minecraft.getMinecraft().getSoundHandler().stopSound(start); - Minecraft.getMinecraft().getSoundHandler().stopSound(loop); Minecraft.getMinecraft().getSoundHandler().playSound(end); // Can't modify activeLoops directly since we'll probably be calling this method from update(), which is // during iteration of activeLoops so it could cause a ConcurrentModificationException @@ -68,12 +77,20 @@ public abstract class SoundLoop implements ITickable { this.needsRemoving = true; } + /** Stops the start and loop sounds immediately. */ + protected void stopStartAndLoop(){ + Minecraft.getMinecraft().getSoundHandler().stopSound(dummyStart); + Minecraft.getMinecraft().getSoundHandler().stopSound(start); + Minecraft.getMinecraft().getSoundHandler().stopSound(loop); + } + // Static methods public static void addLoop(SoundLoop loop){ activeLoops.add(loop); // Do this here rather than in the constructor in case someone wants to play the loop later or reuse it - Minecraft.getMinecraft().getSoundHandler().playSound(loop.start); + Minecraft.getMinecraft().getSoundHandler().playSound(loop.dummyStart); + Minecraft.getMinecraft().getSoundHandler().playDelayedSound(loop.start, 2); // 1 seems to be insufficient } @SubscribeEvent @@ -82,13 +99,14 @@ public abstract class SoundLoop implements ITickable { // meaning we don't get the 'cut' between the start and loop sounds // FIXME: Apparently this only works for dispensers. What the heck is the difference?! if(event.phase == TickEvent.Phase.END){ - activeLoops.forEach(SoundLoop::update); + activeLoops.stream().filter(s -> s.needsRemoving).forEach(SoundLoop::stopStartAndLoop); activeLoops.removeIf(s -> s.needsRemoving); + activeLoops.forEach(SoundLoop::update); // Do this last to allow a 1-tick overlap } } @FunctionalInterface public interface ISoundFactory { - ISound create(SoundEvent sound, SoundCategory category, boolean repeat); + ISound create(SoundEvent sound, SoundCategory category, float volume, boolean repeat); } } diff --git a/src/main/java/electroblob/wizardry/client/audio/SoundLoopSpell.java b/src/main/java/electroblob/wizardry/client/audio/SoundLoopSpell.java index 7937c86f..86a6c6aa 100644 --- a/src/main/java/electroblob/wizardry/client/audio/SoundLoopSpell.java +++ b/src/main/java/electroblob/wizardry/client/audio/SoundLoopSpell.java @@ -17,8 +17,8 @@ public abstract class SoundLoopSpell extends SoundLoop { private final Spell spell; - public SoundLoopSpell(SoundEvent start, SoundEvent loop, SoundEvent end, ISoundFactory factory, Spell spell){ - super(start, loop, end, WizardrySounds.SPELLS, factory); + public SoundLoopSpell(SoundEvent start, SoundEvent loop, SoundEvent end, float volume, ISoundFactory factory, Spell spell){ + super(start, loop, end, WizardrySounds.SPELLS, volume, factory); this.spell = spell; } @@ -43,7 +43,7 @@ public abstract class SoundLoopSpell extends SoundLoop { private final EntityLivingBase source; public SoundLoopSpellEntity(SoundEvent start, SoundEvent loop, SoundEvent end, Spell spell, EntityLivingBase source, float volume, float pitch){ - super(start, loop, end, (sound, category, repeat) -> new MovingSoundEntity<>(source, sound, category, volume, pitch, repeat), spell); + super(start, loop, end, volume, (sound, category, v, repeat) -> new MovingSoundEntity<>(source, sound, category, v, pitch, repeat), spell); this.source = source; } @@ -58,13 +58,13 @@ public abstract class SoundLoopSpell extends SoundLoop { public SoundLoopSpellPosition (SoundEvent start, SoundEvent loop, SoundEvent end, Spell spell, double x, double y, double z, float sndVolume, float sndPitch){ // Huh, I actually found a use for a non-static initialiser block - hence the double curly brackets... - super(start, loop, end, (sound, category, r) -> new PositionedSound(sound, category){{ + super(start, loop, end, sndVolume, (sound, category, v, r) -> new PositionedSound(sound, category){{ // ...et voila, we can just set protected fields as we please using external variables this.xPosF = (float)x; this.yPosF = (float)y; this.zPosF = (float)z; this.repeat = r; - this.volume = sndVolume; + this.volume = v; this.pitch = sndPitch; }}, spell); } diff --git a/src/main/java/electroblob/wizardry/tileentity/TileEntityMagicLight.java b/src/main/java/electroblob/wizardry/tileentity/TileEntityMagicLight.java index e7543b5d..e0b12580 100644 --- a/src/main/java/electroblob/wizardry/tileentity/TileEntityMagicLight.java +++ b/src/main/java/electroblob/wizardry/tileentity/TileEntityMagicLight.java @@ -2,7 +2,7 @@ package electroblob.wizardry.tileentity; import net.minecraft.nbt.NBTTagCompound; -// TODO: Remove this class entirely, it really doesn't need to be a tile entity +// TODO: Remove this class entirely, it can just be a TileEntityTimer (see RenderImbuementAltar for a better solution) @Deprecated public class TileEntityMagicLight extends TileEntityTimer {