RCXcrafter / EmbersRekindled

Port of Embers Rekindled to a minecraft version beyond 1.12.2
MIT License
11 stars 7 forks source link

Add Almost Unified compat #10

Closed rlnt closed 11 months ago

rlnt commented 11 months ago

This pull request adds compat for Almost Unified. The mod's goal is to unify recipes. Since the tag stamping recipe is hardcoded to tag outputs, there is no way for us to unify these recipes to use the preferred item defined in our config. Users would have to keep our mod priority list and yours in sync which is tedious.

The implemented approach resolves the tag with Almost Unified if it's present which makes it an optional dependency. Otherwise, it will use your standard resolving behavior. Approaches like this have also been applied to other mods with their own mod priority lists e.g. Occultism, Cucumber Lib or Titanium Lib.

Let me know if I need to change something. I made also sure to use your cache. I don't think it's needed since the Almost Unified lookup is just a hash map lookup but this fits the code design I think.

RCXcrafter commented 11 months ago

I understand that it's necessary for this to work but I would prefer not to have almostunified as an optional dependency. I'll have to think about this.

62832 commented 11 months ago

If it's only an optional dependency and not a hard dependency, then why would it be a problem?

rlnt commented 11 months ago

The alternative of not having Almost Unified itself as a dependency would require either changing the JSON, which would involve merging the two stamping recipe types, or a separate compat mod, which I could create if we can't agree on a different solution (I would prefer finding a proper native solution tho because this can get really hacky).

This solution would be a bit cumbersome though, which I don't really like. It would mixin into the recipe manager, read the tag stamping recipes, convert the tags to the preferred items, and add them back with the other recipe type.

I don't see any other option. If you're worried about keeping the integration up to date, that's understandable. So far, however, there hasn't been a single breaking change since releasing AU. And I'm also prepared to keep the integration up to date in case we ever break compatibility.

RCXcrafter commented 11 months ago

I agree that hacking into the recipe manager is a bad solution but I'm hesitant to add an optional dependency for such a small thing. Another problem I have with this is that it makes me responsible for updating this code, even though it's a small amount of code and I beleive you when you say that it's unlikely to break.