Minecraftforge: Advanced ore dictionary item matching proposal

[amadornes]

The problem:

I just ran into a situation that a lot of modders have probably ran into. I have an item with a bunch of possible states (infinite states, in fact) and I want to add an oredict entry that maches just a few of them. This is impossible with the current ore dictionary setup unless I register all those (uncountable) possible states, which is not possible and would most likely kill performance. Adding all of them would have the added issue that all the mods that have a cycling display of all the oredict entries with a specific name would start showing this huge amount of items which are in reality the same one, with very minor variants that don't need to be take into account in the recipe.

An example:

Take IC2 batteries, for example. They have a huge amount of possible states. Right now the only two variants that are registered to the ore dictionary are the fully charged and the empty ones. All the other possible states must be handled separately using a custom IRecipe that's kindof hidden. Mods like JEI don't allow you to shift-click semi-charged batteries into the grid because they don't match the oredict entry, but they really do because the recipe just wants _a_ battery, not either a fully charged one or a completely empty one.

The solutions:

The typical solution:

Solution:
Why not just use the wildcard metadata value? That should work, right?
Why it wouldn't always work:
This would work in most cases. However, most cases doesn't equate to all cases.
What if that battery stored its charge in NBT and used metadata for something completely different like the tier, being the battery we want just one of those metadata values? Using a wildcard here would allow any tier battery to be used, so that's not an option. Using normal metadata matching wouldn't work either, because the NBT tags of the reference and checked item wouldn't always be the same, so that's not an option either.

My solution:

The ideal solution in my opinion would be to keep both a list of "reference" items (what it does now) that would be used by mods that want to display the entries with a specific key, like JEI, as well as a list of predicates or checks of some sort that are the ones items get checked against instead of comparing the item direclty against the ones in the refence list. An example of this would be the following (using Java 8 lambdas):

OreDictionary.register("itemBatteryT8", new ItemStack(MyModItems.battery, 1, 7), (entry, reference, stack) -> stack.getItem() == reference.getItem() && stack.getDamage() == reference.getDamage());

A variation of this according to what @diesieben07 said and what I got from it would be as follows:

OreDictionary.registerReference("itemBatteryT8", new ItemStack(MyModItems.battery, 1, 7));
OreDictionary.registerMatcher("itemBatteryT8", (entry, stack) -> stack.getItem() == MyModItems.battery && stack.getDamage() == 7);

Another variation could check for a specific capability, for example, which would be very useful for tools which can be used in crafting recipes.

That would register the stack new ItemStack(MyModItems.battery, 1, 7) to itemBatteryT8's "reference" item list, and a predicate that would only check for the item type and damage to the list of tests that are ran to check if a stack matches an entry name.
Registering a stack without a predicate would automatically register one that performs the check that's currently being performed by default.

I'll PR it if you're interested in it!

If people are interested in this being a feature that's built into Forge I'll be more than happy to make a pull request adding what I mentioned in my solution instead of using custom hidden IRecipes. If you have any others that you consider could work better than mine, be sure to tell me. I'm open to feedback.
Another small detail could be a "tooltip extension" provider of some sort that mods like JEI could use in cases like this one to tell the player that a battery could have any charge and not just the exact amount that the reference item has.

_(Did I go over the top fancy with all the headings and all that stuff? Maybe. I was bored :P)_

[williewillus]

Lol, the headings actually help, as opposed to the usual giant wall of "omfg it sucks"

[Spartan322]

More people need to use headers like this.

[bspkrs]

+1 for headings.

[WayofTime]

I agree, though using Helvetica may have added more punch to the argument.

[diesieben07]

Since everyone seems to be talking about the headings (which are cool indeed)...
Two things:

• There would need to be not just a Predicate but also some kind of Producer or Iterator for mods who actually _want_ to display all the possible states, right? Not a List, just something that gives the variants out one by one.
• Performance testing would have to be done since this turns "is this item part of this oreDict group" from a static call per entry to a possible megamorphic call per entry.

[amadornes]

The first thing you mentioned should indeed be doable and in some cases would come in very handy, @diesieben07. I still think baking them into a list would be a good option and, in fact, the registration methods for predicates and "reference" items could be completely separate, since they don't depend on eachother at all.

The second one, however, I don't see as too big of a deal since you wouldn't need to check against all the possible values, you'd just write a predicate that'd tell you whether or not the item is valid for the specified oredict name, which could be optimized a lot (range checks, direct NBT checks, etc... instead of checking _everything against everything_).
_(See the variant of the original code I just added to the main post)_

[LexManos]

No, Monolithic 1=item-is-everything-in-my-mod! Items are a way of the past, do not do them.
Also most notably if you are making an item that already exists in the ore dictionary you SHOULDN'T BE MAKING THAT ITEM. Just return the oredict item when something in your mod requires that type of time.
We are not looking into making the Oredict shit more complex just to support modders doing it wrong. The more complex we make this shit the more OTHER mods suffer that try to read the dict.

The other problem is most likely the speed of these things, the OreDict is already shitty performance wise and exposing the matcher function to modders would just slow it down more. As most modders would make their code something like: for (String s : myItem.getNBTList("OredictNames"){found = found|s.equals(name);} which is UNGODLY SLOW. But you KNOW modders will do it because they think it's "cool and flexible!"

For now it's a hard no, go yell at @mezz to get off his ass and work on the API he wanted in Forge and MAYBE i'll consider it.

@Everyone Else: Don't get all flipped out over someone using basic formatting. It spams the issue.

[mezz]

Mods like JEI don't allow you to shift-click semi-charged batteries into the grid because they...

To make sure JEI can handle it, IC2 just needs to blacklist the NBT for charge value, or make it an item without subtypes.

public void register(@Nonnull IModRegistry registry) {
    registry.getJeiHelpers().getNbtIgnoreList().ignoreNbtTagNames(ic2BatteryItem, "chargeNBTKey");
}

[amadornes]

I guess I'll say what I'm actually trying to do so you can try to tell me an alternative way of doing it that doesn't involve my solution in any way.
I'm adding a dynamic metal alloying system similar to TFC's that allows players to mix however much they want of each metal and make an alloy with it. This has to use NBT because of obvious reasons. I'd like my bronze alloys (55-80% copper, 10-35% tin), for example, to be usable in other mods' recipes that take bronze ingots. How else could I go about doing this, @LexManos?
I can't think of other viable solutions that don't involve looking through all the recipes (or at least shaped and unshaped, others wouldn't work) after the game has loaded and adding an alternative version that takes my alloyed ingots instead of the oredicted ones.

[cpw]

Perhaps, don't expose the code for comparison- as lex says, that's a complete pig for performance and overloading more function is just gonna double down on the crappiness.

Perhaps, what we need is some sort of item mapping: when the predict asks the itemstackholder what it is, it says 'I'm bronze' or 'I'm tin' or 'I'm trash'.. Perhaps when registering you supply an optional function that performs the translation?