-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bukkit 1.10.x - 1.12.x and 1.13.2 and performance / readability improvements #12
base: bukkit-api
Are you sure you want to change the base?
Conversation
The reason the Travis build is not working properly is due to the local dependency on CraftBukkit and Spigot, though this should be excused for the sake of complying with Minecraft's EULA of not distributing the server jar (which is being done by the mirror repository). I would advise finding an alternative such as a private repository for Travis, but the server jar should not, under any circumstance, be distributed by a Maven repository. |
This is a very comprehensive PR and I really appreciate the time you put into it - I am not around as much as I used to be so I really appreciate the contribution. The changes mostly look good to me and you're definitely right about the pom.xml dependencies. I am also happy that Spigot finally appears to have set up a proper API for the love mode, saving us the trouble of adding updates without new features just for new version matches. Have you had a chance to test this with any of the earlier versions? It definitely will compile but I'm not knowledgeable as to what happens when the plugin depends on an API level at compile time that is not available at runtime. If there are any issues regarding this it can be fixed by putting this futureproof version as a subproject like all of the NMS ones. Before we merge this PR though, I would like to take a couple of days to try and find time to integrate BuildTools into the travis flow so that it can build the plugin without needing to find the server jar on a maven repository. This should be fairly straightforward but it's important for this to function right since automatic releases on GitHub are set up using travis. Also, I wanted to point out that if you are interested in becoming the plugin's official maintainer (or at least a contributor), I would be happy to add you as a contributor onto the repo and the plugin page. |
It's something I pull requested to Bukkit for this very reason. Someone on the SpigotMC forums was looking to update this plugin themselves to 1.13 so I figured I'd take the liberty to do it for them and have it be future proof so others wouldn't have similar issues
If for some reason the plugin is run on a version of Bukkit that do not have this API, (say 1.13.0, 1.13.1 or early versions of 1.13.2) because these people will inevitably exist, the plugin will throw a NoSuchMethodException. While not ideal, Bukkit doesn't really provide a friendly way of checking API version by commits. Yes, there is That being said, server owners are expected to update their servers regularly (hence the reason there is a server startup delay for server builds older than a few days).
In truth, I'm not sure the project will require a maintainer if this PR were to be merged. Due to this project's simplicity and the fact that there is now a proper API for this, it's not likely it will break in the future. Bukkit strives to be as forwards compatible as possible. |
Good job, Choco. I am not around as often as I used to be to help Sultan also. Many people request this plugin from me still on Bukkit and Spigot and I'm sure they will be ingratiated to see updates. |
Sorry, I realized I dropped the ball on this again, I have some time this weekend that I intend to use towards this end. I believe I know how to leverage the current setup to use the Bukkit API for newer versions and our makeshift API for older ones. Will try that, and once that is done we should hopefully have full backward and forward compatibility. |
This PR makes a variety of changes to the plugin in order to have it function for 1.13.2 and onward without requiring any further updates.
<type>jar</type>
declarations../target
build directory as it is deprecated by Maven and unsupported by EclipsegroupId
in children pomsThe following are changes made exclusively to specific classes
lastMateAnimals
as a Set rather than a Set for memory leak reasons and clear it on disablewildSexTask
be a BukkitTask rather than an int. These scheduler methods are deprecated, therefore BukkitRunnable instances should be used insteadonEnable()
methodELIGIBLE_MATES_BUFFER
List constant to buffer entities rather than creating new ArrayList instances for every iteration.Math#max()
instead of ternary in#getReproductionProbability()
lastMateAnimals
and use proper methods insteadfather
andmother
WildAnimal.java:
LOVEMODE_TICK_DURATION
constant for readability sake in version-specific implementationsThis PR also removes the
dependency-reduced-pom.xml
which was left in the repository by mistake. This is an IDE-specific file and should not be present.