Jump to content

Requesting Feedback on Registration


Tablock_

Recommended Posts

I would like any feedback on what I have done for registering my items and blocks. I used an enum to organize all of the different namespaces or id's. Each namespace then contains the types of objects that are registered to them. Examples: "steelstone" stores an item only while "bismuth_ore" contains a block and an item. You can tell which namespaces contain what by looking at where they are defined in the ID class. I created a sub-class called Register that I use to define my namespaces. The methods inside this subclass do the registering, but only once I pass an instance of the Register class into one of my namespaces. Feel free to understand how it works, but the important thing I would like feedback on is if this is a valid way to approach registration, or that I just overcomplicated it.

 

public enum ID
{
    STEELSTONE(new Register().item()),
    CARBON(new Register().item()),
    STEEL_BLEND(new Register().item()),
    STEEL_INGOT(new Register().item()),
    RAW_BISMUTH(new Register().item()),
    BISMUTH_INGOT(new Register().item()),
    BISMUTH_NUGGET(new Register().item()),
    XYLUTH_INGOT(new Register().item()),
    SPECIAL_PICKAXE(new Register().object(new PickaxeItem(Main.SPECIAL_TIER, 1, -2.8F, newItemProperties()))),
    
    MACHINE_CASING(new Register().block(Blocks.IRON_BLOCK).blockItem()),
    BISMUTH_ORE(new Register().block(Blocks.IRON_ORE).blockItem()),
    DEEPSLATE_BISMUTH_ORE(new Register().block(Blocks.DEEPSLATE_IRON_ORE).blockItem()),
    XYLITE_ORE(new Register().block(Blocks.NETHER_GOLD_ORE).blockItem()),
    KRYPTOPHYTE_ORE(new Register().block(Blocks.OBSIDIAN).blockItem()),
    MACHINE_BLOCK(new Register().block().blockItem()),
    
    BISMUTH_GENERATOR(new Register().block().blockItem().blockEntity());
    

//Feel free to understand how this works below. Otherwise the methods .block() registers a block using the corresponding namespace, .item() registers item, etc. Keep in mind I frequently used method overloading.


    public static final Named<Item> BISMUTH_ORES = ItemTags.bind(Main.MOD_ID + ":bismuth_ores"); 
    public static final Named<Block> NEEDS_SPECIAL_TOOL = BlockTags.bind(Main.MOD_ID + ":needs_special_tool");
    
    private String name = this.toString().toLowerCase();
    private Block block;
    private Item item;
    private BlockEntityType<?> blockEntityType;
    
    private ID(Register registry)
    {
        registry.register(this);;
    }
    
    private static class Register
    {
        private List<Supplier<Object>> suppliers = new ArrayList<Supplier<Object>>();
        private ID id;
        
        private void register(ID id)
        {
            this.id = id;
            
            for(Supplier<Object> supplier : suppliers)
            {
                Object object = supplier.get();
                
                if(object instanceof Item)
                {
                    id.item = (Item) object;
                    
                    Main.ITEMS.register(id.name, () -> id.item);
                }
                
                if(object instanceof Block)
                {
                    id.block = (Block) object;
                    
                    Main.BLOCKS.register(id.name, () -> id.block);
                }
                
                if(object instanceof BlockEntityType<?>)
                {
                    id.blockEntityType = (BlockEntityType<?>) object;
                    
                    Main.BLOCK_ENTITIES.register(id.name, () -> id.blockEntityType);
                }
            }
        }
        
        private Register object(Object object)
        {
            suppliers.add(() -> object);
            
            return this;
        }
        
        private Register item(Item.Properties itemProperties)
        {
            return object(new Item(itemProperties));
        }
        
        private Register item()
        {
            return item(newItemProperties());
        }
        
        private Register block(BlockBehaviour.Properties blockProperties)
        {
            return object(new Block(blockProperties));
        }
        
        private Register block(Block blockToCopy)
        {
            return block(BlockBehaviour.Properties.copy(blockToCopy));
        }
        
        private Register block()
        {
            suppliers.add(() -> createInstance(""));
            
            return this;
        }
        
        private Register blockItem(Item.Properties itemProperties)
        {
            suppliers.add(() -> new BlockItem(id.block, itemProperties));
            
            return this;
        }
        
        private Register blockItem()
        {
            return blockItem(newItemProperties());
        }
        
        private Register blockEntity()
        {
            suppliers.add(() -> BlockEntityType.Builder.of(this::createBlockEntity, id.block).build(null));
            
            return this;
        }
        
        private Object createInstance(String childClassName, Object... initargs)
        {
            String blockClassName = "com.tablock.my_first_mod.block." + WordUtils.capitalize(id.name.replace('_', ' ')).replaceAll(" ", "");
            Class<?>[] parameterTypes = new Class<?>[initargs.length];
            
            for(int index = 0; index < initargs.length; index++)
            {
                parameterTypes[index] = initargs[index].getClass();
            }
            
            try
            {
                return Class.forName(blockClassName + childClassName).getConstructor(parameterTypes).newInstance(initargs);
            }
            catch(Exception exception)
            {
                exception.printStackTrace();
                
                return null;
            }
        }
        
        private BlockEntity createBlockEntity(BlockPos blockPos, BlockState blockState)
        {
            return (BlockEntity) createInstance("$BlockEntityChild", blockPos, blockState);
        }
    }
    
    private static Item.Properties newItemProperties()
    {
        return new Item.Properties().tab(Main.MOD_TAB);
    }
    
    public String getName()
    {
        return name;
    }
    
    public Block getBlock()
    {
        return block;
    }
    
    public Item getItem()
    {
        return item;
    }
    
    public BlockEntityType<?> getBlockEntityType()
    {
        return blockEntityType;
    }
}

Link to comment
Share on other sites

You are creating Item instances during static init. You must create and register registry entries during RegistryEvent.Register. DeferredRegister makes doing this correctly very easy. Simply create your item in the supplier given to DeferredRegister#register and it will do things in the correct order.

Link to comment
Share on other sites

I am using DeferredRegister. I am initializing them in my Main class and calling DefferedRegister$register(IEventBus bus):

public static final String MOD_ID = "my_first_mod";
public static final DeferredRegister<Item> ITEMS = DeferredRegister.create(ForgeRegistries.ITEMS, Main.MOD_ID);
public static final DeferredRegister<Block> BLOCKS = DeferredRegister.create(ForgeRegistries.BLOCKS, Main.MOD_ID);
public static final DeferredRegister<BlockEntityType<?>> BLOCK_ENTITIES = DeferredRegister.create(ForgeRegistries.BLOCK_ENTITIES, Main.MOD_ID);
public static final DeferredRegister<MenuType<?>> CONTAINERS = DeferredRegister.create(ForgeRegistries.CONTAINERS, Main.MOD_ID);
public static final IEventBus BUS = FMLJavaModLoadingContext.get().getModEventBus();

public Main() throws ClassNotFoundException
{        
   Class.forName("com.tablock." + MOD_ID + ".setup.ID"); //loads the ID class from before
        
   ITEMS.register(BUS);
   BLOCKS.register(BUS);
   BLOCK_ENTITIES.register(BUS);
   CONTAINERS.register(BUS);
        
   TierSortingRegistry.registerTier(Main.SPECIAL_TIER, new ResourceLocation("special"), List.of(new ResourceLocation("netherite")), List.of());
        
   DistExecutor.unsafeRunWhenOn(Dist.CLIENT, () -> () -> BUS.addListener(Client::init));
        
   MinecraftForge.EVENT_BUS.register(this);
}

 

Link to comment
Share on other sites

Ok I understand, but how might I pass the Item constructor directly and have an Item variable that refers to the same item? I have tried using the ObjectHolder annotation, but I dislike having to have a variable for each type (i.e. item, block, block entity, etc). Since I am storing all of my registry types inside individual objects that represent a specific namespace, I can't use ObjectHolder because it requires a static initializer. Basically, I want to remove the hassle of manually creating a variable every time I need to register a new type. I don't want to have two variables, one a Block and the other an Item, just to represent the same namespace. If this is impossible then I will just use ObjectHolder and put my register types into corresponding classes, one class holding all my items, another my blocks, etc.

Edited by Tablock_
Link to comment
Share on other sites

3 hours ago, Tablock_ said:

I can't use ObjectHolder because it requires a static initializer

It specifically does not.

3 hours ago, Tablock_ said:

want to remove the hassle of manually creating a variable every time I need to register a new type. I don't want to have two variables, one a Block and the other an Item, just to represent the same namespace.

You don't need to store the RegistryObject you get from DeferredRegister in a variable. A simple example:

private static final DeferredRegister<Item> ITEM_REGISTRY = DeferredRegister.create(ForgeRegistries.ITEMS, MODID);
private static final DeferredRegister<Block> BLOCK_REGISTRY = DeferredRegister.create(ForgeRegistries.BLOCKS, MODID);

private static final RegistryObject<Block> BLOCK = BLOCK_REGISTRY.register("block1", () -> new Block(BlockBehaviour.Properties.of(Material.STONE)));
private static final RegistryObject<Block> BLOCK2 = BLOCK_REGISTRY.register("block2", () -> new Block(BlockBehaviour.Properties.of(Material.STONE)));

static {
    for (RegistryObject<Block> block : BLOCK_REGISTRY.getEntries()) {
        ITEM_REGISTRY.register(block.getId().getPath(), () -> new BlockItem(block.get(), new Item.Properties()));
    }
}

This will automatically register a BlockItem for every Block. You can expand this construction however much you like, creating helper methods that register both at the same time, etc. etc. The only thing you have to keep in mind is that you must create the registry objects inside the Suppliers.

Link to comment
Share on other sites

I see what you are saying, but I may have found a solution that better fits me:

private static final Supplier<Object> supplier = () -> new Item(newItemProperties());

private static final RegistryObject<Item> item = Main.ITEMS.register("test_item", () -> (Item) supplier.get());

Is the above code valid? The Item constructor should not be called until it is already passed into the DeferredRegister. I am not entirely sure if this is valid due to their being an Item cast. If the code above isn't valid, is this?

private static final Supplier<Item> supplier = () -> new Item(newItemProperties());
 
private static final RegistryObject<Item> item = Main.ITEMS.register("test_item", supplier);
Edited by Tablock_
Link to comment
Share on other sites

Here is my revised class. I am using registry objects to access my items and blocks. See the bottom of the class with all of the accessor methods. Also one question, why is it not ok to supply the Item constructor indirectly? I thought maybe because the game should be creating a new item object instead of reusing the same one. However, doesn't blocks and items use the same object anyway? I thought block entities created their own unique objects, that's why they can cause lag.

public enum ID
{
	STEELSTONE(new Register().item()),
	CARBON(new Register().item()),
	STEEL_BLEND(new Register().item()),
	STEEL_INGOT(new Register().item()),
	RAW_BISMUTH(new Register().item()),
	BISMUTH_INGOT(new Register().item()),
	BISMUTH_NUGGET(new Register().item()),
	XYLUTH_INGOT(new Register().item()),
	SPECIAL_PICKAXE(new Register().object(Item.class, () -> new PickaxeItem(Main.SPECIAL_TIER, 1, -2.8F, newItemProperties()))),
	BIOME_FINDER(new Register().object(Item.class, () -> new BiomeFinder(newItemProperties()))),
	
	MACHINE_CASING(new Register().block(Blocks.IRON_BLOCK).blockItem()),
	BISMUTH_ORE(new Register().block(Blocks.IRON_ORE).blockItem()),
	DEEPSLATE_BISMUTH_ORE(new Register().block(Blocks.DEEPSLATE_IRON_ORE).blockItem()),
	XYLITE_ORE(new Register().block(Blocks.NETHER_GOLD_ORE).blockItem()),
	KRYPTOPHYTE_ORE(new Register().block(Blocks.OBSIDIAN).blockItem()),
	MACHINE_BLOCK(new Register().block().blockItem()),
	
	BISMUTH_GENERATOR(new Register().block().blockItem().blockEntity());
	
	public static final Named<Item> BISMUTH_ORES = ItemTags.bind(Main.MOD_ID + ":bismuth_ores"); 
	public static final Named<Block> NEEDS_SPECIAL_TOOL = BlockTags.bind(Main.MOD_ID + ":needs_special_tool");
	
	private String name = this.toString().toLowerCase();
	private RegistryObject<Block> block;
	private RegistryObject<Item> item;
	private RegistryObject<BlockEntityType<?>> blockEntityType;
	
	private ID(Register registry)
	{
		registry.register(this);
	}
	
	private static class Register
	{
		private HashMap<Class<?>, Supplier<Object>> supplierMap = new HashMap<>();
		private ID id;
		
		private void register(ID id)
		{
			this.id = id;
			
			for(Object key : supplierMap.keySet())
			{
				Supplier<Object> supplier = supplierMap.get(key);
				
				if(key.equals(Item.class))
				{
					id.item = Main.ITEMS.register(id.name, () -> (Item) supplier.get());
				}
				
				if(key.equals(Block.class))
				{
					id.block = Main.BLOCKS.register(id.name, () -> (Block) supplier.get());
				}
				
				if(key.equals(BlockEntityType.class))
				{
					id.blockEntityType = Main.BLOCK_ENTITIES.register(id.name, () -> (BlockEntityType<?>) supplier.get());
				}
			}
		}
		
		private Register object(Class<?> type, Supplier<Object> supplier)
		{
			supplierMap.put(type, supplier);
			
			return this;
		}
		
		private Register item(Item.Properties itemProperties)
		{
			return object(Item.class, () -> new Item(itemProperties));
		}
		
		private Register item()
		{
			return item(newItemProperties());
		}
		
		private Register block(BlockBehaviour.Properties blockProperties)
		{
			return object(Block.class, () -> new Block(blockProperties));
		}
		
		private Register block(Block blockToCopy)
		{
			return block(BlockBehaviour.Properties.copy(blockToCopy));
		}
		
		private Register block()
		{
			return object(Block.class, () -> createInstance(""));
		}
		
		private Register blockItem(Item.Properties itemProperties)
		{
			return object(Item.class, () -> new BlockItem(id.block.get(), itemProperties));
		}
		
		private Register blockItem()
		{
			return blockItem(newItemProperties());
		}
		
		private Register blockEntity()
		{
			return object(BlockEntityType.class, () -> BlockEntityType.Builder.of(this::createBlockEntity, id.block.get()).build(null));
		}
		
		private Object createInstance(String childClassName, Object... initargs)
		{
			String blockClassName = "com.tablock.my_first_mod.block." + WordUtils.capitalize(id.name.replace('_', ' ')).replace(" ", "");
			Class<?>[] parameterTypes = new Class<?>[initargs.length];
			
			for(int index = 0; index < initargs.length; index++)
			{
				parameterTypes[index] = initargs[index].getClass();
			}
			
			try
			{
				return Class.forName(blockClassName + childClassName).getConstructor(parameterTypes).newInstance(initargs);
			}
			catch(Exception exception)
			{
				exception.printStackTrace();
				
				return null;
			}
		}
		
		private BlockEntity createBlockEntity(BlockPos blockPos, BlockState blockState)
		{
			return (BlockEntity) createInstance("$BlockEntityChild", blockPos, blockState);
		}
	}
	
	private static Item.Properties newItemProperties()
	{
		return new Item.Properties().tab(Main.MOD_TAB);
	}
	
	public String getName()
	{
		return name;
	}
	
	public Block getBlock()
	{
		return block.get();
	}
	
	public Item getItem()
	{
		return item.get();
	}
	
	public BlockEntityType<?> getBlockEntityType()
	{
		return blockEntityType.get();
	}
}

 

Link to comment
Share on other sites

8 hours ago, Tablock_ said:

I see what you are saying, but I may have found a solution that better fits me:

private static final Supplier<Object> supplier = () -> new Item(newItemProperties());

private static final RegistryObject<Item> item = Main.ITEMS.register("test_item", () -> (Item) supplier.get());

Is the above code valid? The Item constructor should not be called until it is already passed into the DeferredRegister. I am not entirely sure if this is valid due to their being an Item cast.

This is valid, but kind of strange. Why are you storing the supplier in a separate field? You must not use it directly. You must use the RegistryObject exclusively to access your Item.

8 hours ago, Tablock_ said:

If the code above isn't valid, is this?

private static final Supplier<Item> supplier = () -> new Item(newItemProperties());
 
private static final RegistryObject<Item> item = Main.ITEMS.register("test_item", supplier);

This is basically the same thing.

5 hours ago, Tablock_ said:

Here is my revised class.

Good lord why are you doing all this... stuff? This code could be so much shorter. But yes, from what I can tell it should be okay now.

5 hours ago, Tablock_ said:

Also one question, why is it not ok to supply the Item constructor indirectly? I thought maybe because the game should be creating a new item object instead of reusing the same one. However, doesn't blocks and items use the same object anyway? I thought block entities created their own unique objects, that's why they can cause lag.

I am not sure what you mean by this.

Link to comment
Share on other sites

11 hours ago, diesieben07 said:

Good lord why are you doing all this... stuff? This code could be so much shorter. But yes, from what I can tell it should be okay now.

I am doing it this way to save code and reduce repetition. Each value in the ID enum represents a unique namespace and contains variables that stores the RegistryObject of the block, item, block enitity, etc. These variables can be null. Moreover, if I want to add an item under the namespace "test_item", all I have to do is add one line to the ID enum: TEST_ITEM(new Register().item()). At this point, the other registry types are self-explanatory, .block() registers a block, .blockEntity() registers a block entity. There are many variants of these methods that you can see in the Registry class I created. The code is very long, I understand, but I would like to have a variable for each registry type (item, block, etc) so I can refer to them later. Having to manually create these variables especially when each declaration is very similar is tedious and annoying.

11 hours ago, diesieben07 said:
16 hours ago, Tablock_ said:

Also one question, why is it not ok to supply the Item constructor indirectly? I thought maybe because the game should be creating a new item object instead of reusing the same one. However, doesn't blocks and items use the same object anyway? I thought block entities created their own unique objects, that's why they can cause lag.

I am not sure what you mean by this.

I was asking you why it is not ok to do the following:

public static final Item ITEM = new Item(newItemProperties());

public static final RegistryObject<Item> TEST_ITEM = ITEMS.register("test_item", () -> ITEM);

I thought it was because whenever Supplier$get() is called it should be creating a new instance of the Item class, not reusing the same item object. But why would this be a problem? From my understanding, the game uses the same instance from Item class for every item of the same type. BlockEntities, however, have unique instances of the BlockEntity class for every block entity.

Edited by Tablock_
Link to comment
Share on other sites

10 hours ago, Tablock_ said:

Moreover, if I want to add an item under the namespace "test_item", all I have to do is add one line to the ID enum: TEST_ITEM(new Register().item()).

You can achieve the same thing with way less code. But whatever. You do you.

10 hours ago, Tablock_ said:

I was asking you why it is not ok to do the following:

public static final Item ITEM = new Item(newItemProperties());

public static final RegistryObject<Item> TEST_ITEM = ITEMS.register("test_item", () -> ITEM);

I thought it was because whenever Supplier$get() is called it should be creating a new instance of the Item class, not reusing the same item object. But why would this be a problem? From my understanding, the game uses the same instance from Item class for every item of the same type. BlockEntities, however, have unique instances of the BlockEntity class for every block entity.

The problem is the time when the item constructor is called. It must be done during RegistryEvent.Register, not some random time during some static initializer. You have no control over when this static initializer runs and therefore no control over whether it runs at the correct time.

Link to comment
Share on other sites

Please sign in to comment

You will be able to leave a comment after signing in



Sign In Now


×
×
  • Create New...

Important Information

By using this site, you agree to our Terms of Use.