Bugfixes and improvements

[EnderKill98]

Hi,

I noticed a bunch of Issues and somehow got sucked into fixing most of them.

See commits for individual changes. But in short:

• The new mod option "Disable Async Playback" should fix #16, #17, #18 and #19 . Still no idea why it happened and curious about how it could happen.
• Allow looping songs using the new "loop" sub-command. This should partially address #15
• Added subcommand to use the internal feature to map instruments. This is especially handy if you're early on some server and want to play most songs using a bunch of noteblocks without instruments. The command in this case would now be: /discjockey remapInstruments map all harp.

For the people interested/impacted, here is the jar file built using this pr (1.20.4): disc_jockey-1.3.1-dev.jar.zip @jiajia06403

I also took the time to backport these commits to 1.20.1. Seems to work so far: disc_jockey-1.3.0-backport-from-mc1.20.4-fixes-dev.jar.zip (source) @LucidMoH @Sobsz

[Sobsz]

Works great on 1.20.1, even without messing with the options! The only nitpicks I can think of are:

• when mapping everything to one instrument, it can ask for more than 25 of it
• it would be nice to be able to map an instrument to nothing, e.g. to remove drums without having to turn them into a pitched instrument

[EnderKill98]

Works great on 1.20.1, even without messing with the options! The only nitpicks I can think of are:

If you mean it fixes the songs, it should only ever do it when you toggle the setting "Disable Async Playback" to on. Otherwise it should behave the same as before. If it still seems fixed, that is likely some change in test scenario (mods, song, timing, ...) then.

• when mapping everything to one instrument, it can ask for more than 25 of it

Yeah. That is a known issue. I implemented this internally a while back and didn't add deduplication of note blocks. So it still needs 1 air noteblock for each instrument and they can't share the same noteblock if it matches the same note, yet.

Since it's often way easier to farm a lot of noteblocks early game, I never bothered to fix it.

Also not sure if it would even play the note twice, if you hit the same noteblock twice in the same tick. Might effect the sound then and make it potentially sound even worse.

• it would be nice to be able to map an instrument to nothing, e.g. to remove drums without having to turn them into a pitched instrument

That is a good idea. I should probably add that. Will see if I find time for that tomorrow.

[EnderKill98]

I added the option to map instruments to "nothing" now. So you can make it effectively discard instruments with that.

Adding instrument deduplication (when 2 instruments are mapped to the same) is surely possible, but I don't really wanna add this right now because the tuning logic is getting pretty messy and is hard to understand as is. This is mainly my fault. Before I add deduplication, I wanna seperate out and overhault that code to be more readable. I lack the motivation for that rn though. So maybe in the future, but not in this pr.

1.20.4: disc_jockey-1.3.1-dev.jar.zip 1.20.1 (backport): disc_jockey-1.3.0-backport-from-mc1.20.4-fixes-dev.jar.zip

[Sobsz]

If you mean it fixes the songs, it should only ever do it when you toggle the setting "Disable Async Playback" to on. Otherwise it should behave the same as before. If it still seems fixed, that is likely some change in test scenario (mods, song, timing, ...) then.

Just confirmed with the exact same song (All Star), environment (multiplayer, no remapping), etc. Your version works regardless of settings, whereas upstream gets through tuning and then hangs. Magic touch? :p