Tinker's Construct + Foundry crash

[joselitosn]

Ignition: Foundry mod uses tinker's recipes to build internally it's compatible recipes. After the latest TC update Foundry is crashing in the following line, which made me think at first Foundry was the culprit.

After a few tests with different mod combinations I found Modern Metals with TC and Foundry to be the cause, specifically the Galvanized Steel alloy. If I disable it in the config the game doesn't crash.

After a few more tests I also realized there is no molten version of any of the modern metals. Am I correct to assume TC compatibility is currently broken?

Minecraft 1.12.2 Forge 14.23.2.2618 Tinker's Construct 2.9.1.65 Ignition: Foundry 3.1.1.1 Modern Metals 2.5.0-beta4.91

Note that the game doesn't crash with just TC and Modern Metals.

[dshadowwolf]

Foundry has an internal registry of liquid metals that it fills in based on mods it knows how to integrate with. (see, for example, the botania integration where it specifically adds Botanias molten metals)

As it doesn't seem, from a quick search, to actually add anything outside those mods I am not surprised by the crash at all. And... you're quite wrong about there being "no molten version" - every material in BaseMetals and ModernMetals has a molten version and there are several added for vanilla materials in case another mod hasn't already added them.

The issue, in this case, appears to be with Foundry referencing its own internal registry of "molten metals" and not the FluidRegistry provided by Forge where all "fluids" in the game are registered.

[joselitosn]

Sorry about not being more specific, maybe I should open another bug report about the molten version problem. With only Modern Metals and TC installed I can only melt Aluminum and Aluminum Brass as seen on the screenshot below.

As for Foundry mod integration, since I'm not a modder I can't say that the way it's being done is wrong or prone to errors. My guess was that something changed in either the newest TC or Modern Metals version, because it did work before and because disabling just one metal (Galvanized Steel) does keep the bug from happening.

Edit: Aluminum and Aluminum Brass only works because they are also added by TC. If for example I add Thermal Foundation, Iridium also starts working. So that's what I meant by TC compatibility not working.

Also sorry if anything I said sounded aggressive or critical, english is not my main language. 🙇

[dshadowwolf]

Blargh - seems you've run into the perpetually re-appearing bug. We seem to run into this issue with every update of either *Metals or Tinkers' and usually don't catch it until after the release, even with heavy testing ahead of time.

[dshadowwolf]

And I have to say "Wow!" - turns out that some of the API updates and changes seem to have had code disappear or never have existed in the first place. Seems that when we added the option of turning off specific liquid forms of materials we missed implementing it as options in the configuration holder for ModernMetals, causing it to never actually create or register any molten metals.

Beyond that there were some bugs in the Tinkers Construct interaction code that have necessitated a change in the API provided by BaseMetals... but this issue actually helped to point out the full depth of the problem we had in the TiC interface code.

Thank you!

[joselitosn]

Edit: Coudn't test with the latest BaseMetals, for now there's an error somewhere... 😣 So I tested again but with the version from curseforge, but ModernMetals and OreSpawn are ok. So I'm guessing ignore it for now... 😔

Should I test the latest commit? Well, I already did anyways. 😄

I can say it did fix the Ignition: Foundry crash. Hurray! 🎉

But it also gave me a few questions...

1) Are all metals supposed to be working in the smeltery? If yes, the following ores couldn't be smelted:

• Boron Ore
• Cadmium Ore
• Rutile Ore
• Tantalum Ore
• Thorium Ore
• Uranium Ore
• Zirconium Ore

The four metals alloys can be smelted, but can't be extracted from the smeltery. Actually Aluminum Brass works but that's probably because TC already added it. Looking in JEI, the Modern Metals version doesn't have a recipe.

2) I went ahead and tried Mekanism integration and I can see that some ores aren't supported even if they have Shards, but I didn't do a full review. Should I do it?

If there's anything else I can help with, just ask. 🙂

[dshadowwolf]

hrm... I've only just gotten a lot of this touched up yesterday and earlier tonight - there was a lot that couldn't get done before I got around to fixing things to match the API's and there is more work yet to go...

[dshadowwolf]

Fixed that crash you linked @joselitosn - should be on the Maven, (BaseMetals) beta4.170, I believe.

I did take a run through ModernMetals over the last couple of days and its reported (by automated code quality tools) as being free of easy to spot issues and I have tested in game and not found issues with the smelter (in fact, the smelter was one of the tests, the other was for the modifiers we add)

If you can repeat your testing and let me know what still isn't working, I'd be grateful

[joselitosn]

Minecraft crashes before it finishes loading Crash Pastebin

[dshadowwolf]

mmm, yummy, yummy crash - let me finish fixing up BaseMinerals and I'll chew on this one - I have a feeling I know exactly what it is :)

[dshadowwolf]

Fixed locally - doing some testing of mods now to check for proper behavior

[dshadowwolf]

okay, its fixed locally - I'm holding off pushing it as I've managed to get some of the remaining IC2 issues fixed as well

[dshadowwolf]

BaseMetals PR #292 fixes the latest crash and implements the last bits of the planned interactions for IC2. This has not (yet) propagated to ModernMetals, but I'll be on that after I get some sleep.

[joselitosn]

Damn, now I feel bad for asking for this fix. I know you've spent the weekend trying to fix the remaining bugs. Get some deserved rest and don't push yourself. 😥

[dshadowwolf]

and there - MMe PR #91 spreads the love, MMe now how functioning IC2 integration... all thats left is BaseMinerals, EndMetals and NetherMetals

[dshadowwolf]

EndMetals and NetherMetals just got PR's fired off that update and fix their existing interactions and add some more. Just BaseMinerals left to handle.

[dshadowwolf]

and... last of the known major errors (save missing recipes) appear to have been handled. Thank you for the patience and for the report that led to finding several major bugs and getting them fixed.