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

Update passengers and vehicles #5312

Open
wants to merge 30 commits into
base: dev/feature
Choose a base branch
from

Conversation

TheLimeGlass
Copy link
Collaborator

@TheLimeGlass TheLimeGlass commented Jan 2, 2023

Description

Update passengers and vehicles. The version control was missed when we removed everything under 1.13. Multiple passenger support is 1.9 and 1.12 in Spigot for some event.

  • Updates time states to be proper and updated number references to EventValues.TIME_PAST etc.
  • Optimizations.
  • Code update and proper formatting.
  • Adds CondHasPassengers for checking passengers.
  • Adds entity support to isEmpty because Spigot has Entity#isEmpty to check if the entity is empty like a minecart.
  • Makes vehicle and passenger[s] default expressions so you can use them in vehicle events.
  • Removes all older than 1.12 things in favor of multiple passengers.
  • Added support for last spawned entity in the make ride effect because if it's an entity data, that entity data will be spawned.
  • Renamed ExprPassenger to ExprPassengers.
  • If any of the passengers or vehicles contained null values, Skript could have errored. No clue how this never got reported.
  • You cannot set the vehicle of multiple entities, they should be using the passengers of expression. That's like stuffing 10 villagers in a minecart, it makes no sense.
  • Added many tests to test against the newly added/updated syntaxes. Note we can't check if the entity is riding the forced make ride entity as that takes another tick. Something we need the JUnit test pull request for to be able to delay.

Target Minecraft Versions: any
Requirements: none
Related Issues: non closing isEmpty #4185

@TheLimeGlass TheLimeGlass added enhancement Feature request, an issue about something that could be improved, or a PR improving something. feature Pull request adding a new feature. labels Jan 2, 2023
Co-authored-by: Kiip <25848425+kiip1@users.noreply.github.com>
Copy link
Member

@Moderocky Moderocky left a comment

Choose a reason for hiding this comment

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

Looks good.

Copy link
Member

@AyhamAl-Ali AyhamAl-Ali left a comment

Choose a reason for hiding this comment

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

Nice PR ⚡

TheLimeGlass and others added 2 commits May 5, 2023 21:18
Co-authored-by: Ayham Al Ali <20037329+AyhamAl-Ali@users.noreply.github.com>
@TheLimeGlass TheLimeGlass requested review from AyhamAl-Ali, kiip1 and APickledWalrus and removed request for kiip1 August 30, 2023 20:24
@sovdeeth sovdeeth added 2.9 Targeting a 2.9.X version release and removed 2.8 Targeting a 2.8.X version release labels Jan 1, 2024
@sovdeeth
Copy link
Member

Tests failing

@sovdeeth sovdeeth removed the 2.9 Targeting a 2.9.X version release label Jun 28, 2024
@sovdeeth sovdeeth removed their request for review September 13, 2024 15:01
@sovdeeth
Copy link
Member

closing due to inactivity

@TheLimeGlass
Copy link
Collaborator Author

TheLimeGlass commented Sep 16, 2024

Conflicts were rough. Can we please get this merged soon to avoid more conflicts?

The current class present on master was importing the org.bukkit.event.entity.EntityMountEvent which was throwing an exception for not existing on older versions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature request, an issue about something that could be improved, or a PR improving something. feature Pull request adding a new feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants