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

Add WorldEdit integration to AdvancedPortals #307

Merged
merged 3 commits into from
Dec 15, 2021

Conversation

Joo200
Copy link
Contributor

@Joo200 Joo200 commented Dec 12, 2021

As mentioned here, you're fine with a WorldEdit integration.
#44

And I want to add it in this PR.

I really like AdvancedPortals but too many wands are terrible. I added a config option to enable WorldEdit integration.
With that option:

  • AdvancedPortals uses the WorldEdit selections (has to be a cuboid selection, uses the same player world. This is a limitation but imo it's not that big problem.
  • Disable /portal wand and /portal selector when WorldEdit integration is active

When WorldEdit is installed:

  • Add /portal we-selection to select the portal as worldedit selection

WorldEdit is marked as soft-depend, you don't need to install worldedit for AdvancedPortals.

* New config option to enable WorldEdit integration
* New command "/portal we-selection <name>" to select the Portal as WorldEdit selection
* When enabled: Create portals with WorldEdit selections, disable AdvancedPortal's Wand
@sekwah41
Copy link
Owner

sekwah41 commented Dec 13, 2021

I need to do some manual checks as some large chunks of code have been altered as well as the velocity library build version had been also modified as part of the same pr.

I've added it to my calendar for tomorrow afternoon so I should be able to review this after work.

Also big thanks for following the correct commit standards 🥰

@Joo200
Copy link
Contributor Author

Joo200 commented Dec 13, 2021

Yes, I had to update the Velocity version. I was unable to compile AdvancedPortals without that. You can remove this version update but I didn't need any code changes for that.

@@ -88,12 +89,13 @@ dependencies {
implementation "org.spigotmc:spigot-api:1.16.1-R0.1-SNAPSHOT"
implementation "net.md-5:bungeecord-api:1.16-R0.4"

implementation "com.velocitypowered:velocity-api:1.1.9"
annotationProcessor "com.velocitypowered:velocity-api:1.1.9"
implementation "com.velocitypowered:velocity-api:3.1.0"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Compiles just fine so seems to be good.

@sekwah41
Copy link
Owner

sekwah41 commented Dec 14, 2021

Yes, I had to update the Velocity version. I was unable to compile AdvancedPortals without that. You can remove this version update but I didn't need any code changes for that.

I believe they may have removed some old versions because they wanted to force people over, it may also be because the old ones were vulnerable to log4shell attacks.

Good job generally on the PR :) Other than the broken logic on the if statement I believe everything else is good to go. Though once that's fixed ill do a quick basic check with and without to make sure the main logic is all working fine.

You may also want to check the website branch and add some info about the world edit sub commands you've added. Or maybe a extra page on the world edit integration and how to enable it.

Or you can explain the general stuff and I'll try to update the documentation on the weekend.

@sekwah41 sekwah41 added the release Create a release when this pr is merged label Dec 15, 2021
@sekwah41 sekwah41 merged commit e302136 into sekwah41:spigot-1.13-1.16 Dec 15, 2021
@github-actions
Copy link
Contributor

🚀 PR was released in v0.9.0 🚀

@github-actions github-actions bot added the released This issue/pull request has been released. label Dec 15, 2021
Repository owner deleted a comment from github-actions bot Dec 15, 2021
Repository owner deleted a comment from github-actions bot Dec 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release Create a release when this pr is merged released This issue/pull request has been released.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants