Implement GemKnife - #36
Draft
cpaca wants to merge 34 commits into
Draft
Conversation
This isn't working (and shulkerStateMeta.setBlockState(box); isn't fixing it)
Turns out shulkerForEach was written right the first time, it's just that Library wasn't.
I am 100% confident this thing is now broken.
This primarily affects the lore.
cpaca
marked this pull request as draft
July 6, 2024 20:43
Contributor
Author
|
I just remembered - when I was testing this with other colors of shulkerbox, it wasn't working. Can either continue working on that, or scrap library of memories, or scrap the entire branch (and decide gem knife won't get to take emerald blocks from your inventory) |
Collaborator
|
Everything you did to BlockAbility is wrong. You're supposed to do that in an Override on LibraryShulkerBox. Save the box contents, call the super method, then set the box contents. |
Bugged: It's not consuming my emeralds.
WARNING: Untested.
This doesn't work.
dlee13
reviewed
Jul 9, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Util.shulkerForEach(box, action) does these 3 steps at once:
Why?
Honestly? I wanted to have a shulker-box of momentos. However, I do vaguely remember something about wanting "holoitems with inventories" (gem knife, quartz blossom, corrupted wheat seeds, watering can, hourglass, rift walker) to not have their own inventory, and instead take stuff from the player's inventory.
Therefore, shulkerForEach is meant to make it easier for these items to also search shulkers inside the player's inventory. Notably, gem knife (with this) won't require you to take the emeralds out of shulkerboxes.
Did you say "shulker-box of momentos" earlier?
My test subject of choice was to make a holoitem that's the same as a shulkerbox, but when you die it'll trigger any momento abilities that are also inside of the shulker-box.
To be clear, I'd been theorycrafting this for a while, though today's ARH session is what pissed me off enough to implement this immediately even though it's a new holoitem. (Additionally, I'd forgotten Gem Knife exists, and Momento is already implemented so in theory it shouldn't be too much ontop of that.)
What are these changes in BlockAbility?
Well, to make a shulker box custom item, I couldn't figure out how to get it work as an enchantment. I swapped it to a custom item, but it kept deleting the contents, so I wrote some stuff in BlockAbility so it'd stop doing that.
Balance problems/too weak/strong/don't like it/will not add to main server/etc.
The main purpose of this commit was for Util.shulkerForEach. LibraryShulkerBox doesn't have a recipe, and I sprung this all on you without any warning. I don't mind that much if LibraryShulkerBox gets deleted, although I will be a little sad since it's useful for reducing the number of inventory slots I'm using.
That said, the changes I made to BlockAbility might be helpful for future shulker holoitems, so those might be worth a keep as well.