Skip to content
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

Implements NMS for pasting and supports older versions of server #2110

Merged
merged 5 commits into from
Mar 26, 2023

Conversation

tastybento
Copy link
Member

This PR adds back in the use of NMS to paste blocks fast when pasting islands. It also implements the code required to support older versions of Minecraft properly with NMS. Right now, it supports 1.19.3 and 1.19.4 and if the server is not those versions then it falls back to Bukkit API. To support newer versions in the future is simple:

  1. Add the dependency to the POM (see the approach used in the POM file to access multiple versions of the same artifact)
  2. Make a new folder in the nms package with the correct name for the server
  3. Extend the PasteHandlerImpl.java class in the new folder, or create a new one if the NMS methods have changed.

The reason why I only supported one version of the server via NMS in BentoBox (with ASkyBlock I had support for many, but it depended on local jar copies of the server) was because although Spigot provides a repo with the all the server versions, Maven only allows one version to be referenced. However, I found a workaround from StackExchange and this enables multiple versions to be dependencies. Therefore, the approach of selecting the correct class (or a fallback) can be used at last.

@HSGamer
Copy link
Contributor

HSGamer commented Mar 22, 2023

Personally, I'd prefer to make a separated addon to handle these NMS-specific code in a multi-module project, and let BentoBox handle the common one.
Because the fallback handler already supports multiple versions although it's slow & laggy. We have a setting to limit how many blocks to be removed per ticks.

@BONNe
Copy link
Member

BONNe commented Mar 22, 2023

I would agree with @HSGamer

Even adding more versions would be annoying as you would need to add more and more dependencies.

A separate addon with proper spigot-nms or reflections would be much better.

@tastybento
Copy link
Member Author

That's a noble goal, and I eagerly await PR's to support for that. :-)

BentoBox has fast deletion by NMS now, and this PR just adds fast pasting of blueprints, plus as a bonus 1.19.3 and 1.19.4 support to bridge the gap. I don't see any harm in merging it. We can adopt the plan to move to a separate addon in the future for NMS in general, and I'm interested in suggestions (or PR's) on how to do that.

@HSGamer
Copy link
Contributor

HSGamer commented Mar 23, 2023

I think adding NMS for PasteHandler would be enough.

Supporting old versions should be moved to another repository where it's properly handled, instead of using a hacky dependency.
Even though it's a temporary solution, I don't think it's worth doing in the main plugin as it will eventually be deleted / moved.

@tastybento
Copy link
Member Author

Okay, I'll remove support for 1.19.3 and keep the status quo.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

22.9% 22.9% Coverage
0.0% 0.0% Duplication

@tastybento tastybento merged commit 1a293a6 into develop Mar 26, 2023
@tastybento tastybento deleted the multiversion_nms branch March 26, 2023 17:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants