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

Removed some redundant exists checks and formatting stuff #6696

Merged

Conversation

cheeezburga
Copy link

Description

This PR removes some redundant exists checks for BukkitClasses, SimpleEvents and BukkitEventValues.

  • BukkitEventValues: just the AoE cloud handles were removed. The behaviour should (I think) be the same, but reflection is no longer needed.
  • BukkitClasses: removes the getEntity exists check, as it should always exist in 1.13+
  • SimpleEvents: initially didn't plan on including this in this PR, but saw some exists checks that could be removed. Then I saw that the formatting was a bit random at times, so I made it a bit more consistent. Happy to revert this file to what it was before if the formatting stuff is out of the scope of this housekeeping stuff.

Target Minecraft Versions: any
Requirements: none
Related Issues: #6676 #6641

@APickledWalrus
Copy link
Member

I don't think it's safe to remove the AoE cloud handles. They were just added for 1.20.5.

@cheeezburga
Copy link
Author

I don't think it's safe to remove the AoE cloud handles. They were just added for 1.20.5.

Oh true. Sorry, I didn't give it much thought honestly coz it was listed in #6641 under the 1.13 bit. I'll revert those changes in a second, but do you know what else the AOE stuff in the legacy tracker issue could be referring to?

@APickledWalrus
Copy link
Member

It may be a mistake, I'll check with Sovde

@cheeezburga
Copy link
Author

It may be a mistake, I'll check with Sovde

Too easy, appreciate it. If I just put the stuff I removed back in and then push it here, will it like, cancel out and remove the file from this PR?

@sovdeeth
Copy link
Member

It may be a mistake, I'll check with Sovde

yes this was a mistake, I read it backwards, thinking the method was for newer versions rather than older versions, I'll remove it from the issue checklist

@Moderocky Moderocky added the housekeeping Housekeeping-related issue or task. label May 14, 2024
@Moderocky
Copy link
Member

Oh true. Sorry, I didn't give it much thought honestly coz it was listed in #6641 under the 1.13 bit.

Yes don't worry Sovde put some things in the list that shouldn't have been 😔 he got me too

We can go through it all again at the end anyway in case something was missed

@sovdeeth
Copy link
Member

Oh true. Sorry, I didn't give it much thought honestly coz it was listed in #6641 under the 1.13 bit.

Yes don't worry Sovde put some things in the list that shouldn't have been 😔 he got me too

We can go through it all again at the end anyway in case something was missed

the ones you crossed off were just for docs updates, not removing checks

@Moderocky Moderocky merged commit 06d75e0 into SkriptLang:remove-legacy-code May 29, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
housekeeping Housekeeping-related issue or task.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants