[🐞]: Incompatibility crash with Numismatic Overhaul

mosharky commented 8 months ago

Before Continuing:

[X] Always check you are using the latest version of the mods and its dependencies
[X] Remove known problematic rendering mods such as Optifine, Sodium and others. Do NOT report issues related to said mods
[X] If you are unsure which mod is the culprit. Disable half your mods at time until you isolate the culprit
[X] If you have a crash ALWAYS post LATEST.LOG below
[X] If the issue is about behaviors be sure to consult the WIKI first

Version-Loader

1.20.1-fabric

Supplementaries Version

1.20-2.7.30

Moonlight Lib Version

1.20-2.9.2

Issue Detail

Both Supplementaries and Numismatic Overhaul use a custom data driven system for villager trades, so they directly clash when trying to add their own trades. The crash happens when loading or creating a world.

Optional Attachment

latest.log crash

To Produce

Use:

Moonlight Lib
Supplementaries
Numismatic Overhaul
owo lib

Try creating or loading into a singleplayer world to reproduce.

MehVahdJukaar commented 8 months ago

did we just use the same exact frealing name for data. lmao. I was aware that not using my own namespaced system could have resulted in clashes but thoguht that the chance of that happening with somebody elee using the same exact folder structure and substructure would have been slim

MehVahdJukaar commented 8 months ago

try new moonlight version

MehVahdJukaar commented 8 months ago

also crash comes from their mod not mine

mosharky commented 8 months ago

same crash i think https://mclo.gs/DHhjps8

I think you or them will have to do more fixing beyond this crash too since numismatic overhaul tries to patch modded trades to use their currency. I have a feeling that it wont be able to patch the trades now added by moonlight/supplementaries

mosharky commented 8 months ago

maybe you or numismatic could add a mod_loaded condition for a compat version of supplementaries trades?

MehVahdJukaar commented 8 months ago

true. can be overridden via same data system tho. Either way direct integration would be needed

mosharky commented 8 months ago

also crash comes from their mod not mine

should I report it to them or do you have a fix planned?

MehVahdJukaar commented 8 months ago

i cant fix the crash. its not coming from my mod. You should report to them

mosharky commented 8 months ago

alright

Noaaan commented 8 months ago

did we just use the same exact frealing name for data. ...

Yes, this is the case. Numismatic Overhaul has used this namespace for cross-mod trades since its inception back in January 2022. As shown in the following screenshot using Supplementaries 2.7.21, Numismatic works just fine with it.

Looking at commit history the functionality to add Villager trades via Moonlight Lib was added 4 days ago? Not sure if that is the first instance of it. The best solution in regards to compatibility would be for Moonlight Lib to use a different namespace. I am unsure how many mods have compatibility with Numismatic Overhaul's format, and this will keep breaking for every other mod which uses it. The crash itself is simply that our parser fails since the trade files have different structures.

MehVahdJukaar commented 8 months ago

well thing is i think they technically do use 2 diff namespaces since i dont look in that folder but only its subfolders

Noaaan commented 8 months ago

I don't understand what you mean, this is not true? Similar to recipes: all JSON files within the villager_trades folder are searched, which does include subfolders. This is due to how an IdentifiableResourceLoader works within Fabric API. Then for each trade we look up the name of the profession in the villager trade map based on the field in the file, e.g. profession:armorer.

MehVahdJukaar commented 8 months ago

thats true. what i meant is that i filted to only look into subfolders

MehVahdJukaar / Supplementaries