version: CraftTweaker2-1.12-4.1.8

Issue: if a mod has tooltips on the item already appeneded craft tweaker will not add specified tooltips even though it's not the same tooltip as modpack maker specifies

Steps To Reproduce:
Add a mod which only appends a tooltip list and does nothing to remove anything from the tooltip list
like silkspawners
add this code for your zs:
minecraft:mob_spawner.addTooltip(format.gray("Craft together with a filled morb to change spawner type."));
Observe the tooltip isn't appeneded even though it's never been added

Conclusion:
your returning from the method because you think the list already contains the tooltip. Here is where your wrong the tooltip arraylist is populated every tick and other mods may add tooltips to items as well so you really don't have to check anything.

My mod silkspawners doesn't remove anything from the tooltip array list you can view the src here:
you will observe another arraylist that's for advanced tooltips I have a checker method to add them from that list to another it's not replacing the original list or clearing it.
https://github.com/jredfox/silkspawners_rewrite/blob/master/src/main/java/com/EvilNotch/silkspawners/client/ToolTipEvent.java

Exact testing instance is available here. Tooltip changing seems to work fine with other items, such as things from TE.

I also tested with <minecraft:mob_spawner>.clearTooltip(); before the tooltip change and it didn't seem to affect anything.

test on another item, like sticks

my mod adds tooltip via event since it's not always my item mob spawner so it's added via event clearing the tooltip apparently does nothing.

Works fine with sticks.

because sticks don't have a tooltip by default besides the default type

this is weird I just commented out all of my code via debug your tooltip wasn't appearing

it appears the issue might be different then I thought.

if an item/block has nbt you completely ignore the tooltip?

Works fine in test instance with and iron sword modified by reputed.

Forge event log as provided by Caliper.

CraftTweaker has the lowest priority, so that might have something to do with it.

Here is the nbt of the spawner:
https://pastebin.com/xbtU6emP

@Noobulus no it has nothing to do with this at the moment the order would be wrong that would be the most that would happen.

my code to prove I commented everything out and it's still not working with mob spawner:

Weird thing is I can create world take my mod out and it works even though that's the only tooltip event and it does nothing so it's has to be on your side.
https://pastebin.com/XjpYKz4n

ok thanks

Note: for the vanilla ItemBlock mob spawner I replace for client syncing issues maybe it's causing issues on your end just thought you should know that vanilla items/blocks are no longer immutable and can be replaced.

Yea... so with this script:

which will match a diamond pickaxe with silk touch, I get this result

Same goes with this script:

<minecraft:diamond_pickaxe>.withTag({ench: [{lvl: 1 as short, id: 33 as short}]}).addTooltip(format.gray("Craft together with a filled morb to change spawner type."));

and there is this:
using the same script from above, I added a mod that added an event handler that did this:

@SubscribeEvent
    public void onItemTooltip(ItemTooltipEvent event) {
        if(event.getItemStack().hasTagCompound() && EnchantmentHelper.getEnchantments(event.getItemStack()).containsKey(Enchantments.SILK_TOUCH)) {
            event.getToolTip().add("I'm enchanted with silk touch!");
        }
    }

Keep in mind that the EnchantmentHelper just reads from the stack's nbt, so there is nothing special going on there, I get this result ingame

and while this picture looks the same as the first one, it is different and was tested with the clearTooltip script from above, with this tooltip adding mod

I really don't know what you do with your tooltip stuff, but you mention that you are replacing the mob spawner ItemBlock, so why don't you just use the addInformation method there?

If you look at my comments I don't replace anything I simply get the tooltip list then add them it's not on my side.

It only occurs when silkspawners is loaded what are you doing you should be using Item.getItemFromBlock() your probably caching all the blocks/items too early do this during post init.

The only thing silkspawners is doing that could mess up your mod is replace the item block of the mob spawner

Your mod still doesn't detect that there is an itemblock for mob spawner is what I am getting at. Meaning it doesn't matter when your event is firing it wont' detect it.

Otherwise your tooltip would appear because I don't remove anything only add info I also don't cancel the event so there is no reason why it's not appearing.

I also do it via event for modded spawners that are not default vanilla's as well so I can't simple be static here.

So quick fix is cache your hashmaps in post init rather then init/pre-init

Well actually, if you replaced the item in the correct event (the Register event) then this wouldn't be a problem at all
https://github.com/jredfox/silkspawners_rewrite/blob/master/src/main/java/com/EvilNotch/silkspawners/MainJava.java#L71
you currently do it in init, keep in mind, scripts are loaded in the Register, which is after the Register event.

well all I can say is it's not my issue figure it out please.

It is possible for you to cache your stuff in post init. Simply go through forges registries during post init and look at all the item and blocks. If your caching your stuff during the registry event then your doing it way wrong as mods can load after your event or before the event(preinit) and then you won't detect it whatsoever as well as mods not trying to be overridden using init instead.

Not going to make a pr you can do that yourself but, here is the basic code. my work is done here
ForgeRegistries.ITEMS.getEntries() > iterate through this

dude, we run our scripts during the Register event, at that point, the vanilla ItemStacks are cached into MCItemStacks, that are immutable and will not change. the scripts run and do their thing based on the current cache, I cant update that cache as they would be treated as different objects, and therefore fail List and Map checks.

So would you kindly get your head out of your ass, realize you are in the wrong here and that there is nothing for me to do as I already have enough constraints set by forge to change how my whole mod works just because you don't want to cut and paste a line of code. Also not to mention that your stuff won't work with Forge ObjectHolders https://mcforge.readthedocs.io/en/latest/concepts/registries/#injecting-registry-values-into-fields
once again, showing that I am very much not in the wrong with how I do stuff.

Okay, let me recap what is being discussed here:

• Silkspawner replaces the Vanilla ItemBlock Spawner in init
• Crafttweaker runs during the RecipeEvent which is before that
• This causes CrT to compare against the Vanilla ItemBlock which does no longer exist, so it will never change the tooltip

-> Now Jared insists that jredfox should move his replacing to a prior point in the FML load cycle.
-> Now Jredfox insists that jared should move the loading of CrT scripts to PostInit.

I can't speak for the prior but I can tell a little story on how CrT parses scripts:
During the RecipeEvent, CrT will first rebuild its internal Item/Liquid/Entity/Potion registries.

The RecipeEvent is always fired after the item and block registry event, so we assume that the item and block registries aren't changed much after that.

Now, I'm babbling around the topic again, how does ZS parse?
As you know we use Bracket handlers like <minecraft:spawner> or <ore:ingotIron>in ZS for better readability and to make it easier for novices to add recipes as those brackets are easier to learn than crafttweaker.crafttweaker-mc.brackets.BracketHandlerItem.getItem("minecraft:spawner", 0)
In order for ZS to know what a bracket means we use a resolve method which resolves the Bracker call into an expression which can then be written to the classfile using an objectweb writer.
Now, if we were to move the initialisation of our registries to a latter state, our Brackets wouldn't match any more.
If we were to reinitialise them at a latter state, some matching methods might break. We do our best to prevent such things from happening (like using custom equals and hash functions) but we cannot guarantee that nothing would break especially when mod support comes in.

Many mods that extend CrT functionality require CrT to load during the recipe event as they want to register their recipes then.

If the matched blocks are replaced after then not only might tooltips break but many normal recipe matches as well, since we essentially do the same to check for recipe item matches as we do for tooltip item matches.
Since we store those itemStacks internally, we cannot do much if the item the itemstack belongs to changes and they will no longer match. We could query the Forge registry each time an IItemStack is called but that would do no good but produce lag.

To conclude

• Running CrT during a latter state of the FML loading cycle is not feasible as it would break many things compat-related. Also the recipe event is made for recipes and the like so CrT is done then.
• Changing IItemStack's implementation to support changes to the Forge registry is not feasible as it would produce lag
• Postponing the tooltip actions and registering them later would bear the same result as staging them directly as the action stages the IIngredient/IItemStack against which we compare later.

Well I do it during init so that mods won't override me so you should be doing it during post init because mods are allowed to register things properly all the way up to init plus this is a 1.12 thing it won't work if you backport.

Stop trying to push the issue on my when you can move the cache to post init it's not just my mod that does registry in init

actually, mods are meant to be using the events, given by forge, if you need to do something last, use the correct priority to your event (as done here https://github.com/CraftTweaker/CraftTweaker/blob/1.12/CraftTweaker2-MC1120-Main/src/main/java/crafttweaker/mc1120/events/CommonEventHandler.java#L44)

This isn't my issue, look at RWTema, who replaces the Hopper block perfectly fine with my stuff catching it.
If you look here, it tells you exactly where to do stuff
https://mcforge.readthedocs.io/en/latest/conventions/loadstages/#initialization

and if you think that me moving the "cache" to post init is possible, then please be my guest to make a PR, but I can tell you now that it won't go well, because we don't have a cache that can simply be moved to post init.

@jredfox While what your suggesting would be a possible solution to this issue, it is not the correct one. Minecraft Forge has implemented the registry events, and it is expected that all mods work within those events as much as possible. As mentioned by Jared, there are many systems within Forge itself, such as the @ObjectHolder annotation which will break when changes are made after those events.

The best solution to this issue would be for you to redesign how you replace the item for this block. Events have a priority mechanism which allow events of higher priority to run first, and events of lower priority to run last. By setting the priority of your event to be the absolute lowest it can be, you should maintain compatibility with CraftTweaker and all the other mods your mod will be incompatible with.

@everyone choonster said to run in post init for iterating through blocks and items way before I suggested it and said it's the only proper way to guarantee that you get all blocks from pre init and init.

Your right you should fire slightly after post init and that would be the "proper fix" to iterate through the arrays of blocks. I have done my job here post init is the best you can do

@Darkhax I am not going to use preinit nor the event so other mods can easily override me if a mod wants to do that they can purposely do it not unconsciously do it during pre init or init. Other mods register things in init as well that replace vanilla items to