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

Properly name PotionEffectType in lang #6854

Merged

Conversation

ShaneBeee
Copy link
Contributor

Description

This PR aims to properly name the PotionEffectType in lang.

A few years back I did a change in the class info, from potion -> potion effect type
I think I forgot to do the lang.

This rename is more precise to the actual name of the class.


Target Minecraft Versions: any
Requirements: none
Related Issues: none

Copy link
Member

@APickledWalrus APickledWalrus left a comment

Choose a reason for hiding this comment

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

This seems okay to me. I think this is only used for toString really?

@ShaneBeee
Copy link
Contributor Author

This seems okay to me. I think this is only used for toString really?

I believe so ya. The usage for the class info already accepts this, I believe when I did the PR years ago I just forgot to change the lang.
I keep seeing people show errors "cannot compare a potion to a potion" and its super confusing.

@sovdeeth
Copy link
Member

sovdeeth commented Jul 3, 2024

I think this could be a patch change

@ShaneBeee
Copy link
Contributor Author

I think this could be a patch change

I put it on the feature branch for a few reasons:

  1. Its not a breaking change (next to no one will notice this change)
  2. Helps understand errors a bit better
  3. Since 2.9 (stable) isn't out yet, I figured now was a good time

But I understand if you guys don't want it in the next update.

@APickledWalrus APickledWalrus added the enhancement Feature request, an issue about something that could be improved, or a PR improving something. label Jul 6, 2024
@sovdeeth sovdeeth added the feature-ready A PR/issue that has been approved, tested and can be merged/closed in the next feature version. label Jul 12, 2024
@sovdeeth
Copy link
Member

I think this could be a patch change

I put it on the feature branch for a few reasons:

  1. Its not a breaking change (next to no one will notice this change)
  2. Helps understand errors a bit better
  3. Since 2.9 (stable) isn't out yet, I figured now was a good time

But I understand if you guys don't want it in the next update.

to clarify i meant patch change as in it could go in 2.9
feature changes could not.

@ShaneBeee
Copy link
Contributor Author

I think this could be a patch change

I put it on the feature branch for a few reasons:

  1. Its not a breaking change (next to no one will notice this change)
  2. Helps understand errors a bit better
  3. Since 2.9 (stable) isn't out yet, I figured now was a good time

But I understand if you guys don't want it in the next update.

to clarify i meant patch change as in it could go in 2.9 feature changes could not.

ohhh gotcha, thanks for the clarification.

@sovdeeth sovdeeth added patch-ready A PR/issue that has been approved and is ready to be merged/closed for the next patch version. 2.9 Targeting a 2.9.X version release and removed feature-ready A PR/issue that has been approved, tested and can be merged/closed in the next feature version. labels Jul 31, 2024
@sovdeeth sovdeeth changed the base branch from dev/feature to dev/patch July 31, 2024 06:08
@APickledWalrus APickledWalrus merged commit 97e4386 into SkriptLang:dev/patch Jul 31, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.9 Targeting a 2.9.X version release enhancement Feature request, an issue about something that could be improved, or a PR improving something. patch-ready A PR/issue that has been approved and is ready to be merged/closed for the next patch version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants