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

More(!?) Converter and Comparator Improvements #5815

Merged

Conversation

APickledWalrus
Copy link
Member

Description

  • improves string representations of some comparator types
  • blocks circular chained converters (I had an issue where Skript created a Component -> String -> Component chained converter, this exists for other types such as ItemType -> ItemStack -> ItemType)
  • implements a new resolution method for comparators. Skript now attempts to convert Object A to the type of Object B (and vice-versa) to perform a same-type comparison.

I also considered implementing a resolution where Skript may find converters for A -> C and B -> C and then perform a same-type comparison for C. Not sure if this is worth it though. Thoughts welcome.


Target Minecraft Versions: any
Requirements: none
Related Issues:
This fixes an issue reported on Discord where Skript could not perform the following comparison:
if region at player is "<text>" - this comparison should work because Skript has a String -> Region Converter. Skript can now perform this comparison.

This enables Skript to compare to objects by converting them to the same type. Currently, this only supports converting Object A to the type of Object B and vice-versa. It is not currently possible for Skript to do Object A to some type C and Object B to some type C. That could be a heavier operation and potentially not work the performance degradation.
@APickledWalrus APickledWalrus added the enhancement Feature request, an issue about something that could be improved, or a PR improving something. label Jul 12, 2023
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.

We believe in you pickle, don't break Skript now 😄

...and a test!

this is to address SkriptLang#5848
essentially, this forces ExpressionList and LiteralList to ONLY use super types based on Skript ClassInfos (e.g. Object instead of something like Serializable) - probably needs more testing!
@APickledWalrus APickledWalrus changed the base branch from master to dev/feature September 16, 2023 18:09
@TheLimeGlass TheLimeGlass removed their request for review October 5, 2023 22:24
@APickledWalrus APickledWalrus added the bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. label Nov 8, 2023
This will provide better toString context
Adds "exact exist" methods for usage during registration (e.g. checking if another addon has already registered a comparator/converter of some types)
@APickledWalrus
Copy link
Member Author

@TheLimeGlass Feel free to review this again as I know you had interest in the toString formatting for these classes

@APickledWalrus APickledWalrus added the 2.8 Targeting a 2.8.X version release label Nov 20, 2023
@APickledWalrus APickledWalrus merged commit 18736a9 into SkriptLang:dev/feature Dec 18, 2023
4 checks passed
@APickledWalrus APickledWalrus deleted the fix/converters-comparators branch December 18, 2023 20:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.8 Targeting a 2.8.X version release bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. enhancement Feature request, an issue about something that could be improved, or a PR improving something.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants