FML icon indicating copy to clipboard operation
FML copied to clipboard

Impossible to override vanilla item / block

Open Clashsoft opened this issue 10 years ago • 35 comments

With the current FMLControlledNamespacedRegistry, it is impossible to override a vanilla item or block since it automatically adds the mod id to the mapping name. The swap method is not accessible because it has a default modifier, and calling it via Reflection crashes because the transactionalAvailabilityMap is null.

Clashsoft avatar Feb 07 '14 21:02 Clashsoft

You can pass the modid "minecraft" in as additional argument in GameRegistry.registerBlock. Read the javadoc!

jeffreykog avatar Feb 07 '14 21:02 jeffreykog

With this

GameRegistry.registerItem(brewingStand, "brewing_stand", "minecraft");

the output says "Adding duplicate key 'BrewingAPI:brewing_stand' to registry". The brewing stand doesnt change at all (I override almost everything, TileEntity, Container, GUI, placer item).

Clashsoft avatar Feb 07 '14 22:02 Clashsoft

Also, calling FMLControlledNamespacedRegistry.add without passing the modId from GameRegistry.registerItem(item, name, modId) still uses the current mod id.

Clashsoft avatar Feb 07 '14 22:02 Clashsoft

I think you'll have to remove the old entry; not sure about the new system though.

ArcanoxDragon avatar Feb 07 '14 22:02 ArcanoxDragon

There's no way to remove an entry from the ID map, at least no "hacky" way.

Clashsoft avatar Feb 07 '14 22:02 Clashsoft

Using "minecraft" should work.

cpw avatar Feb 07 '14 22:02 cpw

Reflection?

ArcanoxDragon avatar Feb 07 '14 22:02 ArcanoxDragon

OK. This isn't going to work at the minute. I thought item replacement would work, but it doesn't. I need special code because you want to preserve/detect your custom replacements in save games - the system doesn't support that behaviour at present.

I'll see what I can do over the next couple of days. Don't try and reflective hack it - you'll just make a nicely broken save for anyone using the hackery.

cpw avatar Feb 07 '14 22:02 cpw

Just out of interest, what are you trying to do? Replace a brewing stand with an alternative block implementation? or what?

cpw avatar Feb 07 '14 22:02 cpw

I consider Reflection as a "hacky" way.

brewingStand2Item = new ItemBrewingStand2(); // extends ItemReed; adds a line to the tooltip for testing overrideItem(brewingStand2Item, "brewing_stand");

public static void overrideItem(Item item, String name) { if (!name.startsWith("minecraft:")) { name = "minecraft:" + name; } GameRegistry.registerItem(item, name, "minecraft"); }

Since it doesnt add a line to the tooltip and I am 100% the code for that is correct, I can assume that registerItem doesnt work.

And yes, alternate implementation to store potion data completely in NBT tags.

EDIT: Just read your comment

Clashsoft avatar Feb 07 '14 22:02 Clashsoft

I think the right solution here is aliasing. Allow registering an alias for one thing to another. I had toyed with it before but I wasnt sure I needed it. This proves it is probably needed. That way, your stand is your code, and its in your mod namespace, and you alias the minecraft one to your code.. It also helps with existing world problems..

cpw avatar Feb 07 '14 22:02 cpw

Well, the FMLControlledNamespacedRegistry extends RegistrySimple, which uses a Map to store it's data. And when adding a key-value pair to a map where the key already exists, it would simply override the value. So you could simply put(name, block) again on the <String,Block> map and put(block, id) on the int map

Clashsoft avatar Feb 08 '14 15:02 Clashsoft

Yes, but that means the change becomes invisible to the idtracker side, which isn't what I want at all..

cpw avatar Feb 08 '14 17:02 cpw

Has there been any progress on this? I see the issue is still open, and the addAlias method in the GameRegistry class is still a stub.

I would like to replace vanilla blocks with replacement blocks in a recommended way, such that they still work for custom recipes from other mods dependent on the vanilla block.

The solution suggested here is not yet implemented, or at least doesn't seem to be.

The documentation recommends reflection (or failing that, Access Transformers); I would rather use a simpler approach such as aliasing, though.

zachanima avatar Jun 29 '14 22:06 zachanima

This seems to be mostly working in forge-1.7.10-10.13.1.1210-new After correcting my dumb mistake (was replacing the block but not the item) the only issue I've currently having is with registerBlockIcons not being called.

ShetiPhian avatar Sep 01 '14 19:09 ShetiPhian

Interesting.. I wonder why.

cpw avatar Sep 01 '14 20:09 cpw

Is it possible that you are registering it too late? As far as I know, you have to do it in the preInit state.

Clashsoft avatar Sep 01 '14 20:09 Clashsoft

Well, the substitute shouldn't be normally registered, for identity reasons. It probably needs the register code to change slightly to scan the substitutions..

cpw avatar Sep 01 '14 20:09 cpw

Also, you can take a look at how I do it without the alias system in my Clashsoft Lib: https://github.com/Clashsoft/Clashsoft-Lib/blob/master/src/main/java/clashsoft/cslib/minecraft/block/CSBlocks.java#L135: https://github.com/Clashsoft/Clashsoft-Lib/blob/master/src/main/java/clashsoft/cslib/minecraft/item/CSItems.java#L325 I believe it does the replacement a bit more safely, even though it involves some really critical and hacky ways that I'm sure @cpw doesn't like at all :trollface:

Clashsoft avatar Sep 01 '14 20:09 Clashsoft

addSubstitutionAlias is being used in preInit after I register my other blocks. For now, so I can test, I'm registering the icons with another block. I haven't ran into any other problems yet.

ShetiPhian avatar Sep 01 '14 20:09 ShetiPhian

As for the "normal" way to do it with aliases, I have two questions:

  1. If I register block Dirt2 as "dirt_2" and then alias "dirt" -> "dirt_2", wouldn't that occupy a new block ID when registering "dirt_2"?
  2. To successfully use aliases, one will have to add the alias before Blocks.class and Items.class init. Which state would that be?

Clashsoft avatar Sep 01 '14 20:09 Clashsoft

@clashsoft your replacement crap is by definition less safe than the CORRECT way to do it. The registry will be locked down to this, once I have sufficient testing done, so BE WARNED..

cpw avatar Sep 01 '14 21:09 cpw

I know this is not the 100% correct way to do it, that's probably why these methods change in almost every update. But still, am I correct with my assumption in my second question? Because if it puts any block in Blocks.class into the field without any registered aliases, it uses the vanilla block until someone manually updates the field, right?

Clashsoft avatar Sep 01 '14 22:09 Clashsoft

NO. not at all. Blocks and Items are both handled by FML automatically. If you register a substitution for anything that's in that class, your alternative will be present there, WHEN THE SERVER IS RUNNING.

cpw avatar Sep 01 '14 22:09 cpw

So, how do I actually register an alias now? FMLControlledNamespacedRegistry.addAlias(String, String) is package private, and GameRegistry.addAlias(String, String, GameRegistry.Type) is a no-op.

Clashsoft avatar Sep 03 '14 21:09 Clashsoft

You need to use the "new" branch of forge http://files.minecraftforge.net/minecraftforge/new

ShetiPhian avatar Sep 03 '14 23:09 ShetiPhian

I tried to override the default bow, but they dissappear just from the game. So I cannot give them anymore or see them in creative.

Cybermaxke avatar Nov 07 '14 14:11 Cybermaxke

Code Only.

LexManos avatar Nov 08 '14 02:11 LexManos

Using this method:

GameRegistry.addSubstitutionAlias(String, Type, Item);

I have currently replaced the entries in the registry and fields in the Items class to make it working. But this method removes the items from the game. You can find my code here.

Cybermaxke avatar Nov 09 '14 16:11 Cybermaxke

So we can conclude that the Alias System works syntactically, but not semantically. Here is a list of things that should be improved.

  1. A substitution should not be forced to be a subtype of the value it substitutes. That just leads to a ton of duplicate code and general confusion.
  2. registerIcons is not called on substitutes. What happens if we don't have a 'normal' block or item that calls the method on our substitutes?
  3. Why exactly does GameRegistry.addSubstitutionAlias(String, Type, Item) has a type argument if you could simply add two methods, one for blocks and one for items?
  4. The amount of unnecessary calls in this method is too damn high. Variables are your friend. And the compiler likes them too.
if (getPersistentSubstitutions().containsKey(nameToReplace) || getPersistentSubstitutions().containsValue(toReplace))                                                         
{                                                                                                                                                                             
    FMLLog.severe("The substitution of %s has already occured. You cannot duplicate substitutions", nameToReplace);                                                           
    throw new ExistingSubstitutionException(nameToReplace, toReplace);                                                                                                        
}                                                                                                                                                                             
I replacement = superType.cast(toReplace);                                                                                                                                    
I original = getRaw(nameToReplace);                                                                                                                                           
if (original == null)                                                                                                                                                         
{                                                                                                                                                                             
    throw new NullPointerException("The replacement target is not present. This won't work");                                                                                 
}                                                                                                                                                                             
if (!original.getClass().isAssignableFrom(replacement.getClass()))                                                                                                            
{                                                                                                                                                                             
    FMLLog.severe("The substitute %s for %s (type %s) is type incompatible. This won't work", replacement.getClass().getName(), nameToReplace, original.getClass().getName());
    throw new IncompatibleSubstitutionException(nameToReplace, replacement, original);                                                                                        
}                                                                                                                                                                             
int existingId = getId(replacement);                                                                                                                                          
if (existingId != -1)                                                                                                                                                         
{                                                                                                                                                                             
    FMLLog.severe("The substitute %s for %s is registered into the game independently. This won't work", replacement.getClass().getName(), nameToReplace);                    
    throw new IllegalArgumentException("The object substitution is already registered. This won't work");                                                                     
}                                                                                                                                                                             
getPersistentSubstitutions().put(nameToReplace, replacement);                                                                                                                 

Clashsoft avatar Nov 11 '14 23:11 Clashsoft