ercanserteli / xercamods

Collection of Minecraft mods: XercaMod, Music Maker Mod and Joy of Painting
GNU General Public License v3.0
19 stars 20 forks source link

[1.16.5] [Enhancement] Adding an API for Instrument Registration for Xerca Music Mod #47

Open FoxMcloud5655 opened 2 years ago

FoxMcloud5655 commented 2 years ago

I would like to request that a method be added for creating new musical instruments to Xerca Music Mod, either via a vanilla datapack or via an API that other mods can hook into. I'm looking to add a variety of instruments to this mod, but the only way to do so currently is to directly edit your code. Your license is "All Rights Reserved" with zero exceptions, so I am unable to publish anything related to adding or removing such features, even pull requests.

If you would be so kind as to add this in, I would also love to get permission from you to create an addon mod that extends these instruments.

ercanserteli commented 2 years ago

Really cool idea! The "All Rights Reserved" license was a placeholder, now I changed it to GPLv3. For now, you can directly add new instruments in your own fork of the mod or create an addon mod if that is possible without an explicit API, as long as you publish the code and keep the license.

As for the API or datapack support, I would like to add this feature but it might take time, and since I made a big update with the 1.17 version, I plan on adding new features to the versions onward from 1.17. As a result, I don't think I will add any new features to the previous versions, including 1.16. Though of course I am open to pull requests regarding this feature, in any version.

FoxMcloud5655 commented 2 years ago

If it's okay with you, then, I might backport your changes to the 1.17.x version down to 1.16.5 before making an add-on mod or otherwise.

ercanserteli commented 2 years ago

That would be great, if you are willing to go through the trouble.

FoxMcloud5655 commented 2 years ago

Alright, I'm now coming back to this, finally. The changes have been ported over from the 1.18 version, but I am having trouble mapping one part of getMusicData() and saveMusicData() in MusicManager.

public static MusicData getMusicData(UUID id, int ver, MinecraftServer server) {
        SavedDataMusic savedDataMusic = server.getWorld(World.OVERWORLD).getSavedData().getOrCreate(null, "music_map");
        Map<UUID, MusicData> musicMap = savedDataMusic.getMusicMap();
        if(musicMap.containsKey(id)){
            MusicData data = musicMap.get(id);
            if(data.version >= ver){
                return musicMap.get(id);
            }
        }
        return null;
    }

At the .getOrCreate(null, "music_map");, the null is there just to get the mod to compile. It needs a Supplier<SavedMusicData>, but for the life of me, I can't figure out where to get this value at. I'll have to tackle this later after I get some more sleep, but any insights would be super helpful.

ercanserteli commented 2 years ago

I haven't used dimension saved data in 1.16 so I don't know how exactly it is different, but that is what you need to look up in order to implement this part. I just checked it quickly and it appears to me that Supplier<SavedMusicData> just needs to return an empty version of the object, so I think you should be able to use SavedMusicData::new in the same way as it is in 1.18. Of course getOrCreate doesn't also take a Function that takes in NBT this time, instead it seems to require a read function in the SavedData class to achieve the same goal. Again, I'm not sure but it is best to check DimensionSavedDataManager and WorldSavedData in 1.16.

Thank you for taking the time to port it.

FoxMcloud5655 commented 2 years ago

You're welcome! I enjoy this mod enough that I don't mind porting it. Yeah, I looked it up in the documentation, but because we're not using the official mappings, many things are different. I know you switched over to the official mappings for 1.18 since that's literally the only mappings that are available, but I realized that way too late into the middle of the port and decided not to undo all of the work I just did to switch over to the official mappings.

I should have this finished by the end of today and I'll submit a pull request when it's ready.

EDIT: Pull request submitted. The only thing left to do is take the functions out and place them into an API.