SkriptLang
Fixed SimpleExpression#getArray() throwing an error when length of values array is 0
Description
I fixed an issue where casting empty array throws an error
Target Minecraft Versions: any Requirements: none Related Issues: https://github.com/SkriptLang/Skript/issues/6817
I think it may be better to check the class of the array. There's no reason to create a new array instance reflectively if the given array is of the correct type
I also question whether this is something we should be doing at all. Really, this is due to ExprXOf violating its contract. When amount is null, it returns an empty Object array (https://github.com/SkriptLang/Skript/blob/72b17b22f399180c99083b4f65eb764b2672583b/src/main/java/ch/njol/skript/expressions/ExprXOf.java#L65). However, its return type is not guaranteed to be Object (which is what is occurring here). I think we are better off ensuring that all syntax are returning arrays based on their return type rather than cleaning it up for them when there is an issue.
I think we are better off ensuring that all syntax are returning arrays based on their return type rather than cleaning it up for them when there is an issue.
I completely agree in principle and I spent a while trying to fix all of these bad returns in a previous PR, but what I worry is that there is no easy way to hold addons, etc. to this contract, so it's likely bad API use will slip through the net, and we either need to catch that at the first opportunity and give a proper error for it (so people know to fix it) or we need to try and accommodate it in a reasonable way, so that it doesn't error where possible.
I think we are better off ensuring that all syntax are returning arrays based on their return type rather than cleaning it up for them when there is an issue.
I completely agree in principle and I spent a while trying to fix all of these bad returns in a previous PR, but what I worry is that there is no easy way to hold addons, etc. to this contract, so it's likely bad API use will slip through the net, and we either need to catch that at the first opportunity and give a proper error for it (so people know to fix it) or we need to try and accommodate it in a reasonable way, so that it doesn't error where possible.
I would be in favor of an error
I would be in favor of an error
That's fine, do you think that should be done in this PR or its own? Also, where do you think is the best place to test for the array being of the wrong (unreported) type?
Closing. We probably don't want to handle this issue in this method, it's the syntax's responsibility to follow the contract.