Conversation
967849d to
a1c1c8c
Compare
willkroboth
left a comment
There was a problem hiding this comment.
The GitHub Actions build is currently failing due to a javadocs error (see 4a101ab).
GitHub didn't let me comment this on the appropriate lines, but I don't think the generic parameter of CommandAPIPaperConfig does anything.
- public class CommandAPIPaperConfig<T extends LifecycleEventOwner> extends CommandAPIBukkitConfig<CommandAPIPaperConfig<T>> {
+ public class CommandAPIPaperConfig extends CommandAPIBukkitConfig<CommandAPIPaperConfig> {IMO the javadocs on the constructor should also probably not use fully qualified names, since they appear fully qualified in the pop up if they aren't imported in the class.
See b4747b5#diff-07f13e9808547824ce44db9baab74a0a21b74a38d710c7c0deefbb486d655afe for the changes I suggested in CommandAPIPaperConfig.
...spigot/commandapi-spigot-vh/src/main/java/dev/jorel/commandapi/CommandAPIVersionHandler.java
Outdated
Show resolved
Hide resolved
...mmandapi-paper/commandapi-paper-core/src/main/java/dev/jorel/commandapi/CommandAPIPaper.java
Outdated
Show resolved
Hide resolved
...mmandapi-paper/commandapi-paper-core/src/main/java/dev/jorel/commandapi/CommandAPIPaper.java
Outdated
Show resolved
Hide resolved
commandapi-core/src/main/java/dev/jorel/commandapi/CommandAPI.java
Outdated
Show resolved
Hide resolved
...bukkit/commandapi-bukkit-core/src/main/java/dev/jorel/commandapi/CommandAPIBukkitConfig.java
Show resolved
Hide resolved
|
|
||
| public APITypeProvider(PaperNMS<?> paperNMS) { | ||
| this.paperNMS = paperNMS; | ||
| this.paperNMS = (PaperNMS<CommandSourceStack>) paperNMS; |
There was a problem hiding this comment.
Every single time paperNMS is referenced (except once in createCommandRegistrationStrategy), it is just used to call paperNMS.bukkitNMS(). It seems like this class could use direct access to bukkitNMS rather than calling the method every single time.
| this.paperNMS = (PaperNMS<CommandSourceStack>) paperNMS; | |
| this.bukkitNMS = paperNMS.bukkitNMS(); | |
| this.commandRegistrationStrategy = paperNMS.createCommandRegistrationStrategy(); |
...i-paper-nms/commandapi-paper-api/src/main/java/dev/jorel/commandapi/nms/APITypeProvider.java
Outdated
Show resolved
Hide resolved
...i-paper/commandapi-paper-vh/src/main/java/dev/jorel/commandapi/CommandAPIVersionHandler.java
Outdated
Show resolved
Hide resolved
...i-paper/commandapi-paper-vh/src/main/java/dev/jorel/commandapi/CommandAPIVersionHandler.java
Outdated
Show resolved
Hide resolved
...pi-paper/commandapi-paper-core/src/main/java/dev/jorel/commandapi/CommandAPIPaperConfig.java
Show resolved
Hide resolved
willkroboth
left a comment
There was a problem hiding this comment.
I noticed that there are files in commandapi-spigot-documentation-code and commandapi-paper-documentation-code. Don't know if you want to remove those now, or later when doing the docs PR.
...mmandapi-paper/commandapi-paper-core/src/main/java/dev/jorel/commandapi/CommandAPIPaper.java
Outdated
Show resolved
Hide resolved
...mmandapi-paper/commandapi-paper-core/src/main/java/dev/jorel/commandapi/CommandAPIPaper.java
Show resolved
Hide resolved
...ommandapi-paper/commandapi-paper-core/src/main/java/dev/jorel/commandapi/VersionContext.java
Outdated
Show resolved
Hide resolved
...i-paper-nms/commandapi-paper-api/src/main/java/dev/jorel/commandapi/nms/APITypeProvider.java
Outdated
Show resolved
Hide resolved
Yeah, left those in so I know what I need for the documentation. They are only temporary which is also why they are not being compiled. |
Resolves #651
This is a small follow-up for #517 and addresses a few issues that were not crucial to get #517 to work but still have to be addressed before releasing. There are a few things that still need to be done here:
CommandAPIBukkitis mentioned in a message when the proper platform should be mentioned insteadCommandAPIPaper > CommandAPIBukkit, however I think it would be useful (mainly for us when people ask for support) to at least log the PaperNMS class. This should, if added, also be applied to the Spigot implementation if applicable.