Skip to content
This repository has been archived by the owner on May 26, 2018. It is now read-only.

Impossible to override vanilla item / block #370

Open
Clashsoft opened this issue Feb 7, 2014 · 35 comments
Open

Impossible to override vanilla item / block #370

Clashsoft opened this issue Feb 7, 2014 · 35 comments

Comments

@Clashsoft
Copy link

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.

@jeffreykog
Copy link
Contributor

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

@Clashsoft
Copy link
Author

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
Copy link
Author

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

@ArcanoxDragon
Copy link

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

@Clashsoft
Copy link
Author

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

@cpw
Copy link
Contributor

cpw commented Feb 7, 2014

Using "minecraft" should work.

@ArcanoxDragon
Copy link

Reflection?

@cpw
Copy link
Contributor

cpw commented Feb 7, 2014

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
Copy link
Contributor

cpw commented Feb 7, 2014

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

@Clashsoft
Copy link
Author

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

@cpw
Copy link
Contributor

cpw commented Feb 7, 2014

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..

@Clashsoft
Copy link
Author

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

@cpw
Copy link
Contributor

cpw commented Feb 8, 2014

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

@zachanima
Copy link

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.

@ShetiPhian
Copy link

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.

@cpw
Copy link
Contributor

cpw commented Sep 1, 2014

Interesting.. I wonder why.

@Clashsoft
Copy link
Author

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

@cpw
Copy link
Contributor

cpw commented Sep 1, 2014

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

@Clashsoft
Copy link
Author

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:

@ShetiPhian
Copy link

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.

@Clashsoft
Copy link
Author

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?

@cpw
Copy link
Contributor

cpw commented Sep 1, 2014

@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..

@Clashsoft
Copy link
Author

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?

@cpw
Copy link
Contributor

cpw commented Sep 1, 2014

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.

@Clashsoft
Copy link
Author

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.

@ShetiPhian
Copy link

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

@Cybermaxke
Copy link

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.

@LexManos
Copy link
Member

LexManos commented Nov 8, 2014

Code Only.

@Cybermaxke
Copy link

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.

@Clashsoft
Copy link
Author

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);                                                                                                                 

@donington
Copy link
Contributor

I know this hasn't been looked at in a while, but I would like to add that as of 1.8 substituting fire is now impossible with the system as-is.

The issue is that in order to substitute a Block properly, the corresponding Item needs to be overridden as well. As of 1.8 minecraft:fire no longer has an associated Item to override, rendering the alias impossible to complete. Only substituting the fire block alone causes fire to not spawn properly.

Please see https://github.com/donington/finitefire/releases/tag/1.8-1.0 for the code I used to come to this conclusion.

@hea3ven
Copy link

hea3ven commented Jun 17, 2015

Currently, addSubstitutionAlias doesn't seem to be completely working even for basic blocks. I'm using this code, and the block can't be placed in the world because the block state to id mapping still has the original block in it. From my debugging I believe that the problem is that GameData.injectSnapshot is not taking the substitution aliases into account, but I don't know enough of the registries to be sure.

@usafphoenix
Copy link

is FMLControlledNamespacedRegistry what's responsible for block and item ids now?
I appear to be getting item ID collisions with several mods installed, and i'm not sure how to fix this in 1.7.10. in 1.6.4 i could just edit block and item ids in config files.

@LexManos
Copy link
Member

LexManos commented Jul 7, 2015

ID Collisions should not happen, For basic debugging use the forums and post logs. Do not hijack github issues that have NOTHING to do with your issue.

@usafphoenix
Copy link

not trying to hijack anything, just scowering the internet to find a solution to a problem, and asking simple questions. But which is now resolved. Got some quality help from a stranger who was both knowledgable and gracious, and it's all fixed now. I'm going to go slaughter some endermen. peace.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests