Fix inventory renaming error. #6159
Conversation
Moderocky
requested review from
sovdeeth,
AyhamAl-Ali,
APickledWalrus and
UnderscoreTud
Moderocky
added
bug
Co-authored-by: Patrick Miller <apickledwalrus@gmail.com>
Co-authored-by: Patrick Miller <apickledwalrus@gmail.com>
Co-authored-by: Patrick Miller <apickledwalrus@gmail.com>
Co-authored-by: Patrick Miller <apickledwalrus@gmail.com>
Co-authored-by: Patrick Miller <apickledwalrus@gmail.com>
Co-authored-by: Patrick Miller <apickledwalrus@gmail.com>
| return null; | ||
| return Bukkit.createInventory(null, type); | ||
| } | ||
| return object; // Return the same ItemType they passed without applying a name. |
There was a problem hiding this comment.
The other get below handles converting ItemStacks to ItemTypes. Do we need to handle that here too (to avoid an ArrayStoreException)? I'm not actually sure that ItemStacks could even be passed into this method though.
There was a problem hiding this comment.
I don't know, TheLimeGlass didn't do this in the original PR but there was another change made to this class since that added that converter outside the getter, so I merged the changes together but since I don't know what/why that's there I can't say whether it's needed.
| if (Skript.isRunningMinecraft(1, 14) && type == InventoryType.COMPOSTER) | ||
| return false; | ||
| if (Skript.isRunningMinecraft(1, 20) && type == InventoryType.CHISELED_BOOKSHELF) | ||
| return false; |
There was a problem hiding this comment.
caching still maybe a good idea ;)
| @SuppressWarnings("unchecked") | ||
| public boolean init(Expression<?>[] exprs, int matchedPattern, Kleenean isDelayed, ParseResult parseResult) { | ||
| if (exprs[0].getReturnType().equals(ItemStack.class)) { | ||
| setExpr(exprs[0].getConvertedExpression(ItemType.class)); |
There was a problem hiding this comment.
Is this needed? Can you add a comment explaining why it is if so
There was a problem hiding this comment.
I have no idea what it is, what it does or why it's there, I didn't write it.
There was a problem hiding this comment.
Existing code had casting to ensure the type was not an ItemStack. For some reason in the past an ItemType could have been an ItemStack. This enforces that and removes the need for casting in runtime. This was discussed on the skriptlang Discord a long time ago to do it this way.
|
Refactored #5943 for dev/patch if that's the reasoning for this pull |
|
@Moderocky @TheLimeGlass |
|
Closing in favour of #5943 |
Description
Prevents attempts at creating a non-creatable inventory type.
Also (actually) fixes an error when trying to rename inventory types.
A cherry pick of #5943 for patch branch with extra bugs fixed.
Target Minecraft Versions: any
Requirements: none
Related Issues: #5495 and #5923