[1.21] Select Music Event

[Zepalesque]

What this does is adds an event for when Minecraft#getSituationalMusic() gets fired, allowing for modders to change/set the music based on whatever conditions they want (that are available from the client of course)

hopefully I did everything right, this is my first ever pull reqeust for either forge or neoforge, but I did follow the contributing instructions so hopefully it should be good

[CLAassistant]

All committers have signed the CLA.

[neoforged-pr-publishing]

• Publish PR to GitHub Packages

[embeddedt]

We strive to keep the size of the patches themselves as small as possible. Instead of patching Minecraft#getSituationalMusic, you could replace the single call to it in MusicManager with ClientHooks.getSituationalMusic() and have that method call the original vanilla method, fire the event, and return the appropriate value. Then we no longer need a bunch of return -> result = patches.

[Zepalesque]

ah yeah, good idea, hadn’t thought of that

[Zepalesque]

ok, changes made

[embeddedt]

Much better, I just have one comment

[Zepalesque]

ah yeah good idea, I’ll do this when get the chance

[ChampionAsh5357]

Honestly, I have no idea how this will work in correlation to multiple mods. For example, if a mod played situational music based upon being in a given structure while a different mod played situational music based on an item the player was wearing on their head, which one would take precedence?

I'm not against the PR, but I think there should be some ordering that should be either defined or explained with some custom system or using the priority of the event listener (e.g., HIGHEST for helmet, LOW for in a structure, LOWEST for a biome/dimension. Other ideas are welcome.

Also, I think the music that should be present in the event is the current music, not the one that Mojang is going to take over after yours.

[Zepalesque]

hmm yeah I can probably add some documentation stuff for the event priority stuff, good idea

for the other part, I’m not 100% sure what you mean, but if you mean that you should be able to change the currentMusic field, then there are a few reasons I didn’t do that, mainly because letting you set that could break stuff since that’s just used to track the current music, so changing it would likely lead to things like overlapping music tracks, plus you can access it if necessary for some reason or another anyway through Minecraft.getInstance().musicManager.currentMusic

You can also still have the music track instantly start playing as well and stop existing music, if that’s what you meant, you just have to set the Music so that replaceCurrentMusic() returns true (can’t remember the exact field name atm)

The specific use case this was initially meant for was for the Aether mod, which overrides creative music in its biomes so that it plays the biome music instead, plus one of its addons that additionally adds nighttime music tracks for the dimension
These are supposed to not instantly replace the existing music, but instead they are supposed to play once the previous music is done and the delay timer ends (for the nighttime music tracks at least, the others are just made the default for the dimension of that makes sense), similar to vanilla music, just allowing a more dynamic system, while also allowing for there to still be other uses

Previously the Aether had used it’s own entirely new music manager, but I figured out that it wouldn’t really be necessary to have a separate one if there was a way to modify what music gets chosen based on the situation

heck this got long, whoops lol, hopefully it’s not too convoluted 😂

[ChampionAsh5357]

I don't mean you should change the currentMusic field, rather that the information makes more sense in terms of the event context. The situational music from Mojang just indicates what the default music would be playing. If we know the event logic will play its own sound, it might not make sense to execute Minecraft#getSituationalMusic. The current music is more useful as it indicates what the custom music will be overriding if it chooses to do so.

[Zepalesque]

The issue with that is that if you wanted to somehow utilize the original default music track for the situation when choosing the new potential music track, you wouldn’t be able to if it instead gave you the current playing music (for instance, using a map to switch to a different music track if a certain one would’ve been played plus a certain condition, such as it being nighttime)
Not only that, but it could also cause discrepancy between the situation and the music, as if you used the current music for the same purpose instead, that might not be the default one based on the current situation, and it would give the incorrect music
it’s also meant to be able to be chained together, so that if you have the event fire after another mod’s event you are given both the original music and the one that the other mod set it to

pretty much, the reason it calls Minecraft#getSituationalMusic is so that it can be used as a variable for the modder when they are choosing the music they want to have chosen instead
hopefully that makes sense and isn’t just a big jumble of word soup

[TelepathicGrunt]

Note for my Bumblezone mod, I have a structure that should stop all other music when player is in it and plays the structure’s own music. Then there is an arena event in the structure that can be triggered that will stop the structure music and play the arena event music.

Which means my use case is one where I’m going to want to override everyone else’s music. At least to me, I feel like I want high priority. Is there a reason why a helmet music should override my event music?

[Zepalesque]

most likely what you’d want to do is have the structure music be high priority, and then the event music higher priority, and have both replaceCurrentMusic
not sure what you mean by helmet music tho

[ChampionAsh5357]

pretty much, the reason it calls Minecraft#getSituationalMusic is so that it can be used as a variable for the modder when they are choosing the music they want to have chosen instead

I understand, though I feel as though Minecraft#getSituationalMusic also falls into that category as whatever it outputs would be the currently playing music, but eh. If you believe that it doesn't make sense, I'll leave it at that since I don't have a solid enough argument for or against it.

Which means my use case is one where I’m going to want to override everyone else’s music. At least to me, I feel like I want high priority. Is there a reason why a helmet music should override my event music?

My general thought process was that things closer to the player in context would take higher priority, since that would be what the player is focusing on. However, it's up to the modders to determine which priority makes sense for their mod. I was just using the helmet as an example.

[Zepalesque]

ah ok I see, I didn’t see the part about having an item on the players head before lol so I was confused when telepathicgrunt mentioned helmets lol

but yeah music keeps playing even if it’s not the one that would’ve been picked, so if you are in one biome with one music, which is playing, and you go to another with different music then it won’t just instantly stop the music so it won’t necessarily be the same

hmm, semi-unrelated to this specific conversation, but I just realized an issue that changing it so it reassigns the music variable rather than having a separate posted variable would resolve, so that’s a good thing

[Zepalesque]

ok, changed it so it reassigns the music variable along with having more in-depth documentation

[Zepalesque]

Ok, made the changes requested

[ChampionAsh5357]

Looking good. A few changes because you may have misunderstood what I meant in discord.

[ChampionAsh5357]

Just some patch issues, but other than that, looks good.

[Zepalesque]

okay, fixed those as well

[ChampionAsh5357]

Looks fine now.