Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hide Tools in JEI - or not #1505

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Conversation

warjort
Copy link
Contributor

@warjort warjort commented Feb 25, 2021

What:
Allow tools to behave like other items in JEI (and make GTCE's JEI look more like Tinker's :-)

How solved:
Introduced a new configuration option that by default keeps the existing behaviour:

    # Whether to hide tools in JEI and creative search menu showing only Darmstadtium. Default: true
    B:hideToolsInJEI=true

When this flag is changed to false, a new predicate is checked on each ToolMetaItem.MetaToolValueItem (i.e. each tool type)

Any materials matching the predicate will be added as "subitems" to minecraft's creative tab, which is the default mechanism to have them appear in the JEI item grid.

In addition, the JEI subtype identification for those items is changed to be metadata;materialname so that JEI can correctly distinguish them

This end result is that you can search for a tool, e.g. "wrought iron wire cutters" click on it and go directly to just its recipe(s), rather than a list of every wire cutter recipe for every material.

Outcome:
Closes #1503

Additional info:
I have tried to make this as easy as possible to use, here are some examples from gregtech.common.items.MetaTool

+    public static final Predicate<SolidMaterial> isToolMaterialFlint = (material) -> 
        material.toolDurability > 0 || material == Materials.Flint;

        SWORD = addItem(0, "tool.sword").setToolStats(new ToolSword())
            .setFullRepairCost(2)
+            .canGenerate(isToolMaterialFlint)
            .addOreDict(ToolDictNames.craftingToolSword);

        SOFT_HAMMER = addItem(7, "tool.soft_hammer").setToolStats(new ToolSoftHammer())
            .setFullRepairCost(6)
+            .canGenerate((material) ->
+                material == Materials.Wood ||
+                material == Materials.Rubber ||
+                material == Materials.Plastic ||
+                material == Materials.Polytetrafluoroethylene
+            )
            .addOreDict(ToolDictNames.craftingToolSoftHammer)
            .addComponents(new SoftMalletItemStat());

Possible compatibility issue:
This mechanism allows tools that do support this feature, to live alongside those that don't.

Even with the flag set to false and the new behaviour active, tools from the addon projects will continue to behave in the old way until their definition has been updated to define a predicate for what materials to show in JEI.

@warjort
Copy link
Contributor Author

warjort commented Feb 25, 2021

As usual here is a list of issues;

  1. Semantics: The name "canGenerate" is not very descriptive but I didn't want to call it "showInJEI"
    Additionally some of the predicates I have defined in MetaTool have some "stupid" names that are basically just mimicking its subpredicates.

  2. CreativeTabs: To get the items into the creative mode tabs I just overrode the getSubItems() method. However this did not work as expected. The items are being added to 2 separate tabs.
    This appears to be a deliberate consequence of some implicit behaviour which obviously didn't anticipate this new processing.
    Perhaps the easiest way is to fix it is to have all the tools in a separate tab?

  3. Ore Dictionary and material NBT: One consequence of this change is that you no longer get a complete list of all tools when you press in "r" in JEI.
    This sounds ok, except JEI is only showing the Darmstadtium tool in recipes instead of cycling through the ore dictionary.
    This is the existing behaviour.
    As part of the ore dictionary processing there is a tool.getStackForm(1) which means it has Darmstadtium in the material tag.
    I am not sure how to fix it or how much of a problem it is. The ore dictionary still works, its just not displayed properly.

  4. Predicates: The predicates I have written in MetaTool are based on the recipes. I have tried to err on the side displaying an item when I wasn't sure if it should be, but there appears to be some inconsistencies in the logic.
    e.g. you can create a diamond electric wrench but not a normal one
    I also think there is duplicated processing for SCREWDRIVER_LV in ToolRecipeHandler.process(Long)Stick

My original prototype for this, actually had the recipes loaders register what items to display in JEI as they create the recipes.
That would mean it is easier to keep in step, but it is also a lot more work to change, harder to read and makes the partial backwards compatibility impossible.

@warjort
Copy link
Contributor Author

warjort commented Feb 25, 2021

By the way, do you know how the existing code knows not to create Coal pickaxes?
It is far from obvious, it took me a couple of hours to figure it out.
I must have seen the code that does it a couple of times and not recognised what it was doing. :-)

@Archengius
Copy link
Member

The implementation is flawed by design, even the predicate itself shows that: you hard-coded flint tools there.
I don't see any benefits from having this feature, except maybe making flint tools more obvious, but then you end up with cluttered JEI.
I don't like the JEI item handler change either. When I click on the darmstandium axe, I want to see recipes of ALL axes and see their characteristics, not just darmstandium axe.

@warjort
Copy link
Contributor Author

warjort commented Feb 25, 2021

The implementation is flawed by design, even the predicate itself shows that: you hard-coded flint tools there.

I would love to be able to write

        SWORD = addItem(0, "tool.sword").setToolStats(new ToolSword())
            .setFullRepairCost(2)
            .canGenerate((material) -> OrePrefix.toolSword.doGenerateItem(material))
            .addOreDict(ToolDictNames.craftingToolSword);

But that ore prefix doesn't exist and even if it did, it would probably still exclude flint?
All I am doing is following the existing recipe rules.
The flint tool recipes are already hardcoded in this same class.

When I click on the darmstandium axe, I want to see recipes of ALL axes

And that is what will still happen. The default behaviour doesn't change anything.
My new subtype handler doesn't get used and nothing is added to the creative mode tabs or JEI.

@warjort
Copy link
Contributor Author

warjort commented Feb 25, 2021

Personally, I also find clutter in JEI horrible. But in this case, it actually makes it more usable.

Currently if you want to make a tool, you have to click on the darmstadtium version and scroll through 10+ pages of unordered recipes to find the one want.
Newbies don't know to do this unless some quest book or wiki page explains it to them.
In practice you just bookmark the tools you use the first time and never search again.

With the tools expanded in JEI it behaves like all other recipes, removing any learning curve.
A search shows all the tools on one page and you can refine it with the material name or some other tool criteria such as the mining level for pickaxes.

@LAGIdiot
Copy link
Member

I like idea behind this PR. I also feel that scrolling through ton of tools to find one from material I want is tedious and hard to figure out for starting players. And when it is configurable then it is even better.

I am bit concerned about mentioned issues so I will have to take a deeper look at this.

@LAGIdiot LAGIdiot added integration: JEI type: feature New feature or request rsr: revision Release size requirements: Revision labels Mar 19, 2021
@warjort
Copy link
Contributor Author

warjort commented Apr 8, 2021

To resolve point (2) above I went ahead and added a creative tab for tools.

The original "darmstadtium" tools are still also displayed in the main GTCE creative tab.
I haven't changed that logic.

This new tab appears unconditionally, so it will be empty when hideToolsInJei=true.

I did this for 2 reasons;

  • The initialisation of the creative tabs is in a static initializer of a commonly used class which means it is hard to reason about when the initialisation occurs versus when the config is read. And even if it works now, it might break in future as classloading changes?
  • Although the hideToolsInJei is specified as "requiresRestart", this is only for the JEI part. For the creative tabs you can swap the configuration in game and it will work.

I chose a "hard hammer" as the icon of the creative tab.
I was going to choose a wrench, but then I noticed forestry has a tab with a wrench on it. :-)

@warjort
Copy link
Contributor Author

warjort commented Apr 8, 2021

I believe the remaining issues now are:

  • The display of darmstadtium tools for the ore dictionary (visual glitch)
  • Hard coded recipes for additional tools in MetaTool being hard coded in the predicates

@warjort
Copy link
Contributor Author

warjort commented Oct 29, 2021

The display of darmstadtium tools for the ore dictionary (visual glitch)

As per the discussion on #1727 this visual glitch would be impossible to fix.
To make the ore dictionary cycle through all the tools with differing materials would require multiple registrations in the ore dictionary with different toolstat/material NBT.
But forge does not allow an item to be registered more than once in an ore dictionary with only differing NBT.

So we are stuck with just showing one tool material for the ore dictionary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integration: JEI rsr: revision Release size requirements: Revision type: feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Flint Tools not showing in JEI
3 participants