Allow sound instances to play custom audio streams

[SquidDev]

There's a couple of mods out there which play dynamically generated sounds, rather than reading from an ogg file. For instance, Etched allows streaming audio from an arbitrary URL and CC: Restitched allows players to generate music data and stream it to the client.

Minecraft's sound engine supports custom audio streams. However, as there is no API exposed for this, mods must use mixins. Unfortunately the sound code is fairly complex and, due to its multi-threaded and callback-based nature, awkward to mixin to. For instance (note, most of these examples are using MojMap):

• Etched uses @Redirect. This is the most "correct" method (and is what this PR also does), but is incompatible with any other mod.
• CC: Restitched @Injects into the play method and a callback lambda. This code is incorrect, due to the multi-threaded nature of the sound code (though works well enough in practice).
• Some mods reimplement some (or all) of the play method (i.e. Infinidisc, music-box).

Given several mods want to provide custom AudioStreams, but doing so in a compatible way is tricky to get right, it makes sense for Fabric to provide an API everyone can hook into.

This PR adds a interface FabricSoundInstance, injected into SoundInstance. This provides a new method getAudioStream, which can be overridden to return a custom audio stream. The default implementation implements the current behaviour of just reading an .ogg file from resource packs.

Further notes

• The obvious alternative here would be to add an event, instead of providing a new interface. The current approach felt more natural, as whenever you want this functionality, you're going to be sub-classing SoundInstance anyway. Happy to go for either.

• This does still require SoundInstance.getSound to resolve to an actual file, even if that file isn't read by the audio stream. This is definitely a little clunky; I'm just not sure if "fixing" this is worth the additional implementation cost. Thoughts welcome!

• I've added a new module fabric-sound-api-v1 as nowhere else felt natural. Obviously let me know if I've got anything wrong with adding the new module :).

• It appears that injected interfaces only work when registered in the main source set. I'm not sure if this is intentional or not?

[modmuss50]

It appears that injected interfaces only work when registered in the main source set.

I dont think thats intentional, most likely something I can fix in loom. Ideally client only modules should have everything in the client sourceset as this will prevent the need to split the jar in a mod dev env. I will take a look into this.

Had a quick look at the PR, it looks great at first glance. I will take a deeper look soon. 👍

[modmuss50]

This should be done by default, something I can fix in a future loom version, for now you should be able to add the following to the sub projects build.gradle:

loom {
	interfaceInjection {
		interfaceInjectionSourceSets.add sourceSets.client
	}
}

[SquidDev]

Also why are you shipping the empty ogg farm as part of the API (and not testmod)?

As every mod that provides a custom audio stream needs to reference a dummy sound it made sense to provide it in the API, rather than every mod having to bundle their own - much like fabric-gametest-api-v1's empty structure.

[apple502j]

@SquidDev Then the existence should be documented somewhere, along with Identifier of the empty sound exposed as a field.

[SquidDev]

Then the existence should be documented somewhere, along with Identifier of the empty sound exposed as a field.

:+1: Is it acceptable just to add it as a constant to FabricSoundInstance? Not sure there's a more sensible location until #2503 is resolved.

[modmuss50]

Is it acceptable just to add it as a constant to FabricSoundInstance?

Yes, I think that would be a good idea 👍

[SquidDev]

I felt it was probably worth while adding an example, as the need for sounds.json is a little awkward and thus worth being explicit in. Happy to revert if it's Too Much :).

[SquidDev]

Hah, I should know better than to commit without running checkstyle!

I don't know if there's any interest in adding a pre-commit hook which runs it automatically? Or is that just going to get too irritatingly slow for people?

[Technici4n]

More documentation is always welcome. :)

Not sure about the pre-commit hook as I've never used them myself, but it looks like it might end up being rather annoying ? We always wait for CI to pass before merging anything.

[Juuxel]

IMO it's too slow since running gradlew checkstyleMain checkstyleTestmod (or even just the main one) builds all modules and that takes a while even with --parallel.

[modmuss50]

I agree, spotless would also need to be ran making commits very slow. I would reccomend using the checkstyle plugin in your IDE to make it clear when you have an issue.

Added to last call, thanks 👍

[SquidDev]

Thank you everyone :).