EnigmaticaModpacks / Enigmatica2Expert

The official repository and issue-tracker for the modpack Enigmatica 2: Expert for Minecraft 1.12.2
https://www.curseforge.com/minecraft/modpacks/enigmatica2expert
Other
141 stars 129 forks source link

Shifting Star issues fix #2145

Closed Skip21 closed 1 year ago

Skip21 commented 1 year ago

Changed the Shifting Star recipe to match a handmade one.

Also changed the recipes that required said Shifting Star to match the new output and handmades.

(Also don't know why DNA and build lines have supposedly changed when they haven't)

NielsPilgaard commented 1 year ago

Hi there, thanks for submitting a PR! Have you verified that this fix works for all recipes you altered? :)

@Discomanco what do you think?

Skip21 commented 1 year ago

I went through multiple testing with all recipes I modified and all worked.

I've modified those as of 1.90f after some people and I got the NBT issue with the benitoite craft.

Discomanco commented 1 year ago

The issue is bigger than just NBT. Most of the remaining recipes don't really care about it, and the issue itself is not the lack of NBT.

Its an issue that happens primarily on servers, and is a thing we discovered with the Black Hole Talisman from Botania. Certain items gain hidden data when entering a players inventory (its neither NBT nor Meta), and something like CraftTweaker can't see it, but some recipe handlers can. This is why its primarily an issue on dedicated servers. And so, I could be afraid that these changes could maybe break things on some servers. Which is also why I outright replaced the item in some recipes

Skip21 commented 1 year ago

I religiously went through a testing phase that follows the exact same lines I'm following in my job (Write, test, [debug, retest}[repeat until fixed], deploy) and so far the recipe never broke could it be both on SP or MP.

Just FYI : You are using the exact same notation I used here in your Cosmic Meatballs craft.

I can even give you the steps I went through to get it right ; 1/ Craft shifting star using Starlight Crafting engine 2/ Craft star using Astral Sorcery altar 3/ Compare using CT built-in command the data 4/ Compare results using Integrated Dynamics NBT reading 5/ Drop both stars, pick them up and repeat 3 and 4 => Resulting in both stars getting the same NBT value corresponding to handmade one 6/ Modify script files wherever data were wrong 7/ Restart pack to apply changes 8/ Repeat steps 1 to 4 => Data are the same for both stars 9/ Try each and every craft requiring a Shifting Star => All worked fine and every iterations went smooth

I advise to test it out for yourself before rejecting it since it basically solves a lot of Endgame.

Discomanco commented 1 year ago

Yeah, and Cosmic Meatballs is the most common to break, followed by Benitoite. That's why this issue is so hard to fix

Skip21 commented 1 year ago

Since video proof seems required to make my point valid here you go ; https://youtu.be/oKWEEOCdyK4

It's a video about crafting both Benitoite and Cosmic meatballs. Please be aware that at the time posting this there is still some work youtube-side to make it hd

As you'll be able to see at no point during the entire video I'm interacting with the Star, the only items I "touch" are the missing Celestial Crystal and Mana pearl pearl/Gaia Spirit that got inverted for some reason (here is human problem failed my filters)

ReiDaTecnologia commented 1 year ago

The issue is bigger than just NBT. Most of the remaining recipes don't really care about it, and the issue itself is not the lack of NBT.

Its an issue that happens primarily on servers, and is a thing we discovered with the Black Hole Talisman from Botania. Certain items gain hidden data when entering a players inventory (its neither NBT nor Meta), and something like CraftTweaker can't see it, but some recipe handlers can. This is why its primarily an issue on dedicated servers. And so, I could be afraid that these changes could maybe break things on some servers. Which is also why I outright replaced the item in some recipes

Do u know what data is that?

Discomanco commented 1 year ago

Since video proof seems required to make my point valid here you go

Unfortunately video proof is no good for a thing that doesn't have a 100% failure rate. Even on our official server Benitoite had like a 25% success rate, only some of the crafted stars didn't work. A cheated star worked every time. And it depended a lot on world, player and sp/mp. Cheated items works pretty much every time.

Only Benitoite and Cosmic Meatballs have had this problem, both with and without the NBT data, I have tried all the different things, get people to change the scripts with no luck. It's not as simple as adding the NBT data to everything. I know this is the logical solution, but there's more behind the scenes that I do not understand

Do u know what data is that?

I do not know, and I don't know how to check. Ghatus and lekutree first noticed it with Black Hole Talisman. In AE2, the Talisman will stack, as AE2 does. It can autocraft Black Hole Tanks with that. If you place it into your player inventory, and then back to AE2, it is no longer stackable, and will no longer work with Black Hole Tanks. If you change the recipe to the new Talisman and craft it, the returned Talisman will now stack with the old ones.

ReiDaTecnologia commented 1 year ago

Do u know what data is that?

I do not know, and I don't know how to check. Ghatus and lekutree first noticed it with Black Hole Talisman. In AE2, the Talisman will stack, as AE2 does. It can autocraft Black Hole Tanks with that. If you place it into your player inventory, and then back to AE2, it is no longer stackable, and will no longer work with Black Hole Tanks. If you change the recipe to the new Talisman and craft it, the returned Talisman will now stack with the old ones.

Damn, i think the only hope is using a debugger to check what is called when u drop the unbroken item and pick up the broken item then check each mod's source code to know what they are doing but idk any debugger either

Discomanco commented 1 year ago

And ofc to add, the new and old Talismans has the same Meta data and both have no NBT. I even tried messing around with empty tags .withTag{} in the recipes and retusn, but to no avail

E: And as a plus note: This only happens on dedicated servers. SP is completely fine. Shifting Star seems to have a higher failure rate on MP than SP

Skip21 commented 1 year ago

Well since you don't seem to be even wanting to give a shot at my fix I'll just close this PR with a note :

Remove any NBT based item from your crafts : Celestial Crystals, Shifting Stars and Quantum Singularities

Discomanco commented 1 year ago

You don't seem to understand what the actual issue here is though. It not NBT data. If it was as simple as adding .withTag{astralsorcery:{}}, then I would have done so. And as you saw, I already tried that in the Cosmic Meatballs, and they still failed to craft a lot. Not every time, but a lot!. Celestial Crystals also don't have any NBT associated with it in the recipe, and it doesn't care about that. Shifting Star is just being a dick about it I have tried various fixes on our official server with people helping me out, tried to make lists of Shifting Stars which the combination table doesn't support (/ct syntax wont catch that)