Astral Sorcery

Astral Sorcery

63M Downloads

"astralsorcery" NBT tags interfere with TConstruct casting

codewarrior0 opened this issue ยท 3 comments

commented

AstralSorcery built from today's master at commit d06a67a

Tool parts from Tinker's Construct cannot be used to create casts after Astral Sorcery has added its "astralsorcery":{} NBT tag to them. This is due to the casting table recipe matcher requiring an exact NBT match for recipe inputs, on account of TConstruct items and tool parts using NBT tags to indicate the shape of a cast and the material of a tool part.

This tag is being added as a result of the ASM hack which adds extra enchantment levels according to an amulet-enchantment mechanic, which was introduced in f3f8c39.

commented

Congratulations, you found a bug in an unreleased build.
Guess what: That's one of the reasons it's not released. It's not finished and not tested. Things are broken and things don't work properly. That's why you don't build from source and report bugs about it. It is already long fixed on my local dev instance, i didn't bother to push it yet - many people like to build mods from unreleased things from github nowadays for whatever reason and think everything that's pushed is stable and finished.

tl;dr: Don't report bugs on unfinished things.

On the other hand, strict NBT matching is horrible as it is and shouldn't be done anymore anyway. There are a multitude of possible approaches to matching an NBT tag that are different than .equals and serve a better and more dynamic approach to figuring out if the stack has the value the mod in question is looking for. Expecting an NBT-Tag to not be modified by other mods is not more than wishful thinking.

On the topic of "you might need testers": Yes, i do and i give out testbuilds to people i trust and that i know are familiar with the mod and its mechanics. Those are specific builds with notes as to what's still needed to be adressed and what's still in-work.

commented

I'm sorry if I've upset you. I chose to report this particular issue early because it will have lasting effects on the game world that persist even after Astral Sorcery is rolled back or removed.

Looking forward to your next push.

commented

While i do highly discourage building mods from unfinished things from a repository, i am quite more upset about not even TiC handling NBT-matching properly and doing a hard-check instead. While i do not doubt them knowing what they're doing, i do thus think they were aware of the problems a hard-check can lead to, especially in terms of incompatibility; more so if you're looking at something as generic in functionality as the enchantment amulet. While there are now things in place that prevent some of this from happening, i do highly encourage that they should drop their lazy act and implement NBT-checking properly on the tags they require/check. "Inspiration" could be drawn from the over-engineered checks i've written up for ModularMachinery that allow for much more than just "is this tag the same as the other". https://github.com/HellFirePvP/ModularMachinery/blob/master/src/main/java/hellfirepvp/modularmachinery/common/util/nbt/NBTMatchingHelper.java#L25