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

Make Backtank preserve NBT on placement. #6662

Merged

Conversation

VoidLeech
Copy link
Collaborator

Issues Fixed

Fixes #6225

Implemented Solution

This PR saves the item's full nbt data to the backtank block entity, which is then the first thing merged into the root stack.getOrCreateTag() (either via right-click equipping, or the loot table), before the currently saved data overwrites parts of it. Of that data, only Air seems truly relevant still, and the rest could probably be adjusted to just write/read to/from the full nbt, while still making sure already placed backtanks don't lose their enchantments/names. (But that's left out of this PR for now)

Concerns

  • I had hoped this would also apply to Backtank custom mod data isn't being preserved when it's placed in block form #6186, but Cold Sweat appears to use Forge's capability system (which I'm not too familiar with) to store some of its insulation data and so still throws out data.
    Any capabilities on the item could probably be saved inside the block entity and restored when picking up the backtank via right-click equipping, but I'm not too sure how they'd be properly preserved from the backtank's loot table. Any ideas on that front would be appreciated.

VoidLeech added 4 commits July 5, 2024 00:16
ForgeCaps don't save yet either though
This reverts commit a6f5c51.

Would duplicate the enchants on the backtank
@IThundxr IThundxr added the pr type: fix PR fixes a bug label Jul 5, 2024
@simibubi simibubi merged commit cbfbfb9 into Creators-of-Create:mc1.18/dev Jul 15, 2024
@simibubi
Copy link
Collaborator

Thanks for the PR.
For capabilities to persist, the "ForgeCaps" compound has to be saved on to the tileentity as well. I'll look into this briefly

@VoidLeech VoidLeech deleted the mc1.18/backtank-preserves-nbt branch July 15, 2024 13:27
@VoidLeech
Copy link
Collaborator Author

Hey @simibubi,
Nice to see the ForgeCaps saving to the block entity as well.
There is a regression in 15f426d though: when upgrading a world from the current patch (where "VanillaTag" does not exist, i.e. will return an empty compound) to the current main, the custom name and enchantments of placed backtanks are voided, because when reading, "Enchantments" isn't read at all anymore, and "CustomName" isn't being put into vanillaTag.

@simibubi
Copy link
Collaborator

Hi, that's a valid concern.
I was just going to put a quick notice about it in the changelog.
Name and enchants on placed backtanks didn't feel critical enough to leave a migration, but I understand if you disagree

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr type: fix PR fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Placing down backtanks will remove Quark's Runic Etching
3 participants