search

squeek502 / VeganOption

A Minecraft mod that seeks to add vegan alternatives to all Minecraft mob/animal products
The Unlicense
46 stars 10 forks source link

Remaining things for 1.10 port #71

Closed elifoster closed 7 years ago

elifoster commented 7 years ago

Because my memory isn't good enough to just remember. I just went through the readme.

TODO

Issues found

Untested:

squeek502 commented 7 years ago

Thank you for making this list. šŸ‘

elifoster commented 7 years ago

@squeek502 For the JEI descriptions, should we migrate to the system already added by JEI, or should we reimplement it using the old code from the NEI TextHandler class? Using the JEI system would be significantly less code, and would have less room for error since so many people are already using it, but the text processing is significantly less powerful. Basically all formatting stuff would have to be rewritten. For example, from the lang file

example.string=Nested string example
item.VeganOption.exampleItem.name=Example
item.VeganOption.exampleItem.nei.usage=%1$s is nothing like [[minecraft:wheat]] or [[minecraft:golden_apple:1]], but it can do a {example.string}.\n\nLines broken.

would be

item.VeganOption.exampleItem.nei.usage=Example is nothing like Wheat or Golden Apple, but it can do a Nested string example.\n\nLines broken.

Either way, it the valid items would be determined by doing

        for (Item item : Item.REGISTRY)
        {
            if (item.getRegistryName().getResourceDomain().equals(ModInfo.MODID_LOWER))
            {
                if (item.getHasSubtypes())
                {
                    List<ItemStack> subItems = new ArrayList<ItemStack>();
                    item.getSubItems(item, null, subItems);
                    for (ItemStack subItem : subItems)
                    {
                        String descKey = subItem.getUnlocalizedName() + ".nei.usage";
                        if (LangHelper.existsRaw(descKey))
                            // Register the itemstack somehow depending on the approach taken
                    }
                }
                else
                {
                    ItemStack toAdd = new ItemStack(item);
                    String descKey = toAdd.getUnlocalizedName() + ".nei.usage";
                    if (LangHelper.existsRaw(descKey))
                        // Register the itemstack somehow depending on the approach taken
                }
            }
        }

I've done it both ways and that code works well.

squeek502 commented 7 years ago

@elifoster see https://github.com/squeek502/VeganOption/commit/160e12b9e51b3a469df7f872b547f08468a77c80

elifoster commented 7 years ago

I provided a review. :+1:ing the stuff you agree with would be appreciativeā€”I can work on the stuff I brought up no problem.

As for the fluid thing, I'm not sure I quite understand the problem. What is wrong with it being looked up using FluidStacks?

squeek502 commented 7 years ago

Replied to your comments, will commit something that addresses them in a bit. EDIT: https://github.com/squeek502/VeganOption/commit/14706049e097add69d05a09c127611ed95595278

As for the fluid thing, I'm not sure I quite understand the problem. What is wrong with it being looked up using FluidStacks?

The issue is currently that if you press R or U on any of the fluid blocks in JEI (Raw Ender, for example), nothing comes up. Raw Ender should have a Crafting description that shows up, saying that it's the byproduct of an Ender Rift (note: if you do usage of Ender Rift, Raw Ender shows up as a byproduct, so it's not a symmetrical bug).

EDIT: Fixed by https://github.com/squeek502/VeganOption/commit/036f5ab1438a31b8ddf39be1d0f30e84063493f3

elifoster commented 7 years ago

Regarding "Make fluid containers and fluids have the same recipes/usages," it might be better to simply use the fluid containers for the recipes/usages, and then to blacklist the fluid blocks from showing up at all in JEI. What do you think?

squeek502 commented 7 years ago

it might be better to simply use the fluid containers for the recipes/usages, and then to blacklist the fluid blocks from showing up at all in JEI

This is complicated by the fact that certain things will output/use contained fluids, and certain things will output/use fluid blocks from the world. So, if everything shows up as a contained fluid, that divide will be confusing (for example, piston crafting showing input/output of bottled fluid, when that is not how it works).

It might be possible to get around this, though, by making what shows up in JEI independent of what gets set as the input/output in the Wrapper implementations (e.g. IRecipeWrapper.getIngredients sets the bottled fluid as the input, but the category implementation sets the slot to the fluid block--this is kind of what https://github.com/squeek502/VeganOption/commit/036f5ab1438a31b8ddf39be1d0f30e84063493f3 did). I'll look into this more and see what's possible.

squeek502 commented 7 years ago

Putting this here (copied from IRC) just so I don't forget about it:

<squeek> @SatanicSanta, looks like there's still some stuff left that needs to be done with fluid containers
<squeek> VO's bottles/buckets dont interact with the basin anymore
<squeek> prob need to either update bottles to the capacity system or put backwards compatibility in the basin code
<squeek> or both

Also, with the description stuff working properly, the "Make fluid containers and fluids have the same recipes/usages" thing is not as important, since I believe all bottled fluids link to their fluid blocks with the byproducts/byproduct of sections. I think we should focus on the few remaining things and hold off on this bit (will make a separate issue for it and remove it from this checklist).

squeek502 commented 7 years ago

Maybe spoke too soon about closing this. Going to re-open and use it to track a few remaining things: