
Oredict lookup only checks against the first match
squeek502 opened this issue ยท 8 comments
Just a heads up: OreDictionary.getOreID(itemStack) only returns the first ore ID found, even if an itemStack is registered under multiple ore dictionary entries.
I'm not sure how to get all ore dictionary IDs that an itemStack is registered under without reflection or something (OreDictionary.java doesn't seem to provide a method), but that's what needs to be done. Then just loop through each, get their names, and compare them all to the custom filter(s).
Relevant source code: https://github.com/Dynious/RefinedRelocation/blob/master/src/main/java/com/dynious/refinedrelocation/grid/FilterStandard.java#L60 (and other getOreID() calls).
Completely untested fix here: squeek502@b93f78f
Consider it an example fix for now.
Turns out getOreIDs was added extremely recently by Forge: MinecraftForge/MinecraftForge@25ef7e3
(Hey, you're back :P)
Thanks for the heads up, I didn't even know things could have multiple registries! Performance is very important in a filter that is used often, I'm not sure if there's a way to do this without hurting performance too much.
Are you experiencing issues with multiple registries? The current sorting system is a bit derpy (which is probably causing most problems), but that should be fixed in 1.0.5 and the builds here: http://www.creeperrepo.net/ci/RefinedRelocation/
It was just something I came across while working with ore dictionary lookups in another mod. For example, Pam's HarvestCraft registers all its base food as both crop<Name>
and something like listAllberry
. This makes it so that getOreID on something like a cranberry may return the ore ID of cropCranberry while something like a strawberry may return the ore ID of listAllberry, which will make certain checks fail when they should actually pass.
I'm not really sure there's a way around it in terms of performance; getting and checking all ore IDs is a necessity for ensuring that you don't get false negatives.
I heard Forge would change some things in the OreDictionary so a waited with fixing this. Turns out it improves performance which is always nice. This will be fixed in the 1.7 build, but not in the 1.6 one, as only 1.7 has real support for multiple id's per stack.
Fair enough. Just be aware that without this fix in 1.6.4, the possibility for false negatives when filtering using the ore dictionary will always exist.
EDIT: Figured it shouldn't be closed until the fix is actually implemented in the 1.7.2 version.
This was fixed in c0cc933, should probably be closed.