-
-
Notifications
You must be signed in to change notification settings - Fork 382
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
PotionEffectType overhaul #7364
PotionEffectType overhaul #7364
Conversation
Do you know if there is any way of removing the getId entirely? It's deprecated for removal (though that's hidden by the suppression). |
could use keys/strings(from keys) instead. I also considered doing an overhaul for this and using a RegistryClassInfo when the registry is available. |
@Moderocky ok I re-wrote the whole PR.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks for fixing this.
…ing at least 1" This reverts commit dea1d22.
@APickledWalrus will wait for you before merging because you have a stale potions rework PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few minor things. One thing I wanted to ask about is the serialization changes. Is it possible that the serialization/deserialization process would change on the same version (that is, using the registry vs using PotionEffectType.getByName). Nice work!
- how'd you get up there silly?
Description
This PR aims to overhaul potion effect type in Skript.
It started with a bug:
![Screenshot 2025-01-02 at 12 00 56 PM](https://private-user-images.githubusercontent.com/20008482/399795828-7c2172f3-cb91-4246-bd28-2ec5746dd98d.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MzkwODE0MDcsIm5iZiI6MTczOTA4MTEwNywicGF0aCI6Ii8yMDAwODQ4Mi8zOTk3OTU4MjgtN2MyMTcyZjMtY2I5MS00MjQ2LWJkMjgtMmVjNTc0NmRkOThkLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNTAyMDklMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjUwMjA5VDA2MDUwN1omWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTFjYzg2NTM5MWE4OWVkZDVhNWJjZGM4MDdlNDUzYjc3NTg5NTk1MGZiMGMzYzQ1YmUwNjA5YzBiOGI0NTdiNTQmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.rByAWG__dRs-mNR_TbW5uQjEKQLWYg_MSHwMEdGw0Jw)
"null" showing up in the docs for PotionEffectType:
It has since changed into re-working a lot:
Target Minecraft Versions: any
Requirements: none
Related Issues: none