skyboy / NetherOres

12 stars 14 forks source link

Added EnderIO SAG Mill support #31

Closed Parker8283 closed 9 years ago

skyboy commented 9 years ago

I cannot accept this because findUniqueIdentifierFor will return null in the case of a custom itemstack and the item being registered with the same name (an intentional, extra, and entirely idiotic check is done to return null in this case).

Also, the need for some special extra route to manage XML building for EIO is absurd; ItemStacks can be passed around via IMC, and the mod sending the IMC (or a string on some NBT data) can be used for creating groups in config files.

Maybe when EIO support isn't absurdly complex (and over IMC at that!).

Parker8283 commented 9 years ago

The findUniqueIdentifierFor was working just fine in my testing. Also, yea, the IMC is rather absurd. EIO does provide a way for users to add this themselves in a config, but I figured it would be easier if it was enabled by default. I understand your concerns and reasons for closing this.

skyboy commented 9 years ago

It normally does work fine, but when a custom ItemStack and an Item have conflicting names from the same mod (despite these things never being accessed in a manner that can conflict like that) it will return null due to a superfluous check. It's annoying, and not a bug, so I've just marked the method as worthless and avoid it.

skyboy commented 9 years ago

These methods, and the associated checks:

    static UniqueIdentifier getUniqueName(Block block)
    {
        if (block == null) return null;
        String name = getMain().iBlockRegistry.getNameForObject(block);
        UniqueIdentifier ui = new UniqueIdentifier(name);
        if (customItemStacks.contains(ui.modId, ui.name))
        {
            return null;
        }

        return ui;
    }

    static UniqueIdentifier getUniqueName(Item item)
    {
        if (item == null) return null;
        String name = getMain().iItemRegistry.getNameForObject(item);
        UniqueIdentifier ui = new UniqueIdentifier(name);
        if (customItemStacks.contains(ui.modId, ui.name))
        {
            return null;
        }

        return ui;
    }

edit: Even more excellent is that these methods will crash if given an item that's not yet properly registered (such as when connected to a server with content-adding mods the server does not have; an action intentionally permitted by default, unless the mod author allows it!)

Parker8283 commented 9 years ago

Ah, well I could just use the getNameForObject method instead and just get substrings for where I use the Unique Identifiers. But in the end, you probably still don't like the IMC, so it won't make a difference in this PR being pulled.

skyboy commented 9 years ago

The complexity and config desyncs is a bit off-putting, yes

Parker8283 commented 9 years ago

In looking at it, EIO has support for using OreDict tags over the stack descriptions; would it be any better if I used those?

skyboy commented 9 years ago

It still poses a problem if the EIO config gets desynced from the NO config as a result of the user changing NO's