Fixes reloadDataPacks() on Paper 1.21.4 build 63 and above #627
Conversation
...-bukkit-nms/commandapi-bukkit-1.21.4/src/main/java/dev/jorel/commandapi/nms/NMS_1_21_R3.java
Outdated
Show resolved
Hide resolved
Co-authored-by: booky <53302036+booky10@users.noreply.github.com>
There was a problem hiding this comment.
Just static code analysis right now. None of these things are a big deal, functionality looks correct. Just "code style" stuff.
I tried a bit to test this, but weirdly I couldn't reproduce datapacks not working with the old CommandAPI version? I was probably doing something wrong, so I'll try to figure that out again when I have the time.
...-bukkit-nms/commandapi-bukkit-1.21.4/src/main/java/dev/jorel/commandapi/nms/NMS_1_21_R3.java
Outdated
Show resolved
Hide resolved
...-bukkit-nms/commandapi-bukkit-1.21.4/src/main/java/dev/jorel/commandapi/nms/NMS_1_21_R3.java
Outdated
Show resolved
Hide resolved
...-bukkit-nms/commandapi-bukkit-1.21.4/src/main/java/dev/jorel/commandapi/nms/NMS_1_21_R3.java
Outdated
Show resolved
Hide resolved
...-bukkit-nms/commandapi-bukkit-1.21.4/src/main/java/dev/jorel/commandapi/nms/NMS_1_21_R3.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Testing seems good - as far as I can tell, behavior seems the same as on old paper as well as consistent with Spigot. Although, I realized that I don't really know how the reloadDataPacks process is supposed to work :P. I was initially confused since the functions in my test datapack were reloading correctly. It seems that the functions get reloaded before setSelected is called, so even if that part errors, CommandAPI commands in datapacks still work. Fixing the exception is still definitely good since I'm sure the code after setSelected does important stuff. I just don't know what it does, so I don't know how to test it. But calling the method correctly does make sense to get us back to how it was working before, so I'll assume that's correct 👍.
A few more comments, but nothing at all important. I'd be happy to see this merged.
Pointed out by @booky10 in Discord, due to the addition of the datapack lifecycle event in this commit (PaperMC/Paper@feb8756),
PackRepository#setSelectednow takes in an additional boolean. This results in aNoSuchMethodExceptionwhen trying to run the current code.This PR aims to fix that.
Also updates the current version to 9.7.1-SNAPSHOT in case we'd want to release this as a bug fix.