Refined Relocation

Refined Relocation

3M Downloads

Oredict lookup only checks against the first match

squeek502 opened this issue ยท 8 comments

commented

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).

commented

Completely untested fix here: squeek502@b93f78f

Consider it an example fix for now.

commented

Turns out getOreIDs was added extremely recently by Forge: MinecraftForge/MinecraftForge@25ef7e3

commented

(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/

commented

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.

commented

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.

commented

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.

commented

This was fixed in c0cc933, should probably be closed.

commented

Closing this as 1.6.4 is no longer supported and it's been fixed in the 1.7 version.