How to improve my codding style for registry

Posted August 5, 20187 yr

Asking for help in this forum, some people have told me that I'm using a common wrong coding style that I should avoid:

So, I want to ask a few questions:

Currently, I have a common, a client and a server (which I don't use) proxy. That topic said everything from common proxy should be moved to the main file. So my question is if I should do something like this:

Basically:

Move everything from CommonProxy to main file (ExtraCraft).
Delete CommonProxy.
Change inheritance of ClientProxy from CommonProxy to ExtraCraft.
Change inheritance of ServerProxy from CommonProxy to ExtraCraft.

CommonProxy: (Delete it)

//@Mod.EventBusSubscriber(modid = ExtraCraft.MOD_ID)
public class CommonProxy {
	
	// Config instance
    /*public static Configuration config;
    
	public void preInitializationEvent(FMLPreInitializationEvent event) {
		File directory = event.getModConfigurationDirectory();
        config = new Configuration(new File(directory.getPath(), "modtut.cfg"));
        Config.readConfig();
	}
	
	public void initializationEvent(FMLInitializationEvent event) {
	}
	
	public void postInitializationEvent(FMLPostInitializationEvent event) {
        if (config.hasChanged()) {
            config.save();
        }
	}
	
	@SubscribeEvent
	public static void registerItems(final RegistryEvent.Register<Item> event) {		
		ModItems.registerItems(event);
		ModBlocks.registerItemBlocks(event);
	}
	
	@SubscribeEvent
	public static void registerBlocks(final RegistryEvent.Register<Block> event) {
		ModBlocks.registerBlocks(event);
	}
	
	public void registerRenderers() {
		
	}*/
}

ClientProxy: (Change inheritance)

@Mod.EventBusSubscriber(modid = ExtraCraft.MOD_ID, value = Side.CLIENT)
public class ClientProxy extends ExtraCraft /*CommonProxy*/{
	
	@Override
	public void preInitializationEvent(FMLPreInitializationEvent event) {
		super.preInitializationEvent(event);
	}
	
	@Override
	public void initializationEvent(FMLInitializationEvent event) {
		super.initializationEvent(event);
	}
	
	@Override
	public void postInitializationEvent(FMLPostInitializationEvent event) {
		super.postInitializationEvent(event);		
	}
	
	@SubscribeEvent
	public static void registerModels(ModelRegistryEvent event) {		
		ModItems.initializeItemsModels();
		ModBlocks.initializeBlockModels();
	}

	@Override
	public void registerRenderers() {
		
	}
}

ServerProxy: (Change inheritance)

@Mod.EventBusSubscriber(modid = ExtraCraft.MOD_ID, value = Side.SERVER)
public class ServerProxy extends ExtraCraft /*CommonProxy*/ {
	
	@Override
	public void preInitializationEvent(FMLPreInitializationEvent event) {
		super.preInitializationEvent(event);
	}
	
	@Override
	public void initializationEvent(FMLInitializationEvent event) {
		super.initializationEvent(event);
	}
	
	@Override
	public void postInitializationEvent(FMLPostInitializationEvent event) {
		super.postInitializationEvent(event);
	}
}

ExtraCraft: (Get all the code from CommonProxy)

@Mod(modid = ExtraCraft.MOD_ID, name = ExtraCraft.NAME, version = ExtraCraft.VERSION)
/*ADD*/ @Mod.EventBusSubscriber(modid = ExtraCraft.MOD_ID)
public class ExtraCraft {
	
	public static final String MOD_ID = "extracraft";
	public static final String NAME = "ExtraCraft Mod";
	public static final String VERSION = "0.1.0.0";
	
	/*ADD*/ public static Configuration config;

	static Logger logger; // private
	
	@SidedProxy(clientSide = "net.enderlook.extracraft.proxy.ClientProxy", serverSide = "net.enderlook.extracraft.proxy.ServerProxy")
	//private static CommonProxy proxy;
	
	@Mod.Instance
	private static ExtraCraft instance;
	
	@Mod.EventHandler
	public void preInitializationEvent(FMLPreInitializationEvent event) {
		logger = event.getModLog();
		//proxy.preInitializationEvent(event);		
		/*ADD*/ File directory = event.getModConfigurationDirectory();
		/*ADD*/ config = new Configuration(new File(directory.getPath(), "modtut.cfg"));
		/*ADD*/ Config.readConfig();
		ModBlocks.init();;
	}
	
	@Mod.EventHandler
	public void initializationEvent(FMLInitializationEvent event) {		
		//proxy.initializationEvent(event);
		ModRecipes.init();
	}
	
	@Mod.EventHandler
	public void postInitializationEvent(FMLPostInitializationEvent event) {
		//proxy.postInitializationEvent(event);
		/*ADD*/ if (config.hasChanged()) {
		/*ADD*/     config.save();
		/*ADD*/ }
	}
	
	public static String appendModID(String value) {
		return MOD_ID + ":" + value;
	}
	
	/*ADD*/ @SubscribeEvent
	/*ADD*/ public static void registerItems(final RegistryEvent.Register<Item> event) {		
	/*ADD*/ 	ModItems.registerItems(event);
	/*ADD*/ 	ModBlocks.registerItemBlocks(event);
	/*ADD*/ }
	
	/*ADD*/ @SubscribeEvent
	/*ADD*/ public static void registerBlocks(final RegistryEvent.Register<Block> event) {
	/*ADD*/ 	ModBlocks.registerBlocks(event);
	/*ADD*/ }
	
	/*ADD*/ public void registerRenderers() {
	/*ADD*/ 	
	/*ADD*/ }
	
}

The code in comments will be deleted, code with "/*ADD*/" will be added.

Furthermore this critical change, should I do another critical change.

Is fine my "registerItems" and "registerBlocks" event or I should try also to move them?. So, should I delete that last 3 methods and just add "@SubscribeEvent" on each one in their classes's methods ("ModItems", "ModBlocks")

Quote

August 5, 20187 yr

Don't make your proxies extend your mod class, that makes no sense. Make an interface for them(IProxy for example) where you would define the methods your proxies should provide like I do here.

14 minutes ago, Enderlook said:

Is fine my "registerItems" and "registerBlocks" event or I should try also to move them?. So, should I delete that last 3 methods and just add "@SubscribeEvent" on each one in their classes's methods ("ModItems", "ModBlocks")

There is no harm in the way it is. It is my personal opinion that having a method the only function of which is to call another static method is redundant and adds unnecessary code. If you are registering your blocks in method X make that method the event handler - there is no need to have a separate method that only calls this one and does nothing else.

Quote

August 5, 20187 yr

The only other thing I suggest you do is get rid of the IHasModel interface, if you have one per Code Style #3. Your ModelRegistryEvent handler can do all of the work just fine without having to cast and request.

Apparently I'm a complete and utter jerk and come to this forum just like to make fun of people, be confrontational, and make your personal life miserable. If you think this is the case, JUST REPORT ME. Otherwise you're just going to get reported when you reply to my posts and point it out, because odds are, I was trying to be nice.

Exception: If you do not understand Java, I WILL NOT HELP YOU and your thread will get locked.

DO NOT PM ME WITH PROBLEMS. No help will be given.

Quote

August 5, 20187 yr

Author

4 hours ago, V0idWa1k3r said:

Don't make your proxies extend your mod class, that makes no sense. Make an interface for them(IProxy for example) where you would define the methods your proxies should provide like I do here.

There is no harm in the way it is. It is my personal opinion that having a method the only function of which is to call another static method is redundant and adds unnecessary code. If you are registering your blocks in method X make that method the event handler - there is no need to have a separate method that only calls this one and does nothing else.

Ok, thanks, I'll perform the update.

2 hours ago, Draco18s said:

The only other thing I suggest you do is get rid of the IHasModel interface, if you have one per Code Style #3. Your ModelRegistryEvent handler can do all of the work just fine without having to cast and request.

Ok, I was about to ask that also. Thanks, I'll perform the modifications.

Quote

August 5, 20187 yr

Author

4 hours ago, V0idWa1k3r said:

Don't make your proxies extend your mod class, that makes no sense. Make an interface for them(IProxy for example) where you would define the methods your proxies should provide like I do here.

There is no harm in the way it is. It is my personal opinion that having a method the only function of which is to call another static method is redundant and adds unnecessary code. If you are registering your blocks in method X make that method the event handler - there is no need to have a separate method that only calls this one and does nothing else.

I've tried to remove the inheritance from ExtraCraft making an interface but it produces a crash:

Description: There was a severe problem during mod loading that has caused the game to fail

net.minecraftforge.fml.common.LoaderException: net.minecraftforge.fml.common.LoaderException: Attempted to load a proxy type net.enderlook.extracraft.proxy.ClientProxy into net.enderlook.extracraft.ExtraCraft.instance, but the types don't match
	at net.minecraftforge.fml.common.ProxyInjector.inject(ProxyInjector.java:102)
	at net.minecraftforge.fml.common.FMLModContainer.constructMod(FMLModContainer.java:603)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
	at java.lang.reflect.Method.invoke(Unknown Source)
	at com.google.common.eventbus.Subscriber.invokeSubscriberMethod(Subscriber.java:91)
	at com.google.common.eventbus.Subscriber$SynchronizedSubscriber.invokeSubscriberMethod(Subscriber.java:150)
	at com.google.common.eventbus.Subscriber$1.run(Subscriber.java:76)
	at com.google.common.util.concurrent.MoreExecutors$DirectExecutor.execute(MoreExecutors.java:399)
	at com.google.common.eventbus.Subscriber.dispatchEvent(Subscriber.java:71)
	at com.google.common.eventbus.Dispatcher$PerThreadQueuedDispatcher.dispatch(Dispatcher.java:116)
	at com.google.common.eventbus.EventBus.post(EventBus.java:217)
	at net.minecraftforge.fml.common.LoadController.sendEventToModContainer(LoadController.java:218)
	at net.minecraftforge.fml.common.LoadController.propogateStateMessage(LoadController.java:196)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
	at java.lang.reflect.Method.invoke(Unknown Source)
	at com.google.common.eventbus.Subscriber.invokeSubscriberMethod(Subscriber.java:91)
	at com.google.common.eventbus.Subscriber$SynchronizedSubscriber.invokeSubscriberMethod(Subscriber.java:150)
	at com.google.common.eventbus.Subscriber$1.run(Subscriber.java:76)
	at com.google.common.util.concurrent.MoreExecutors$DirectExecutor.execute(MoreExecutors.java:399)
	at com.google.common.eventbus.Subscriber.dispatchEvent(Subscriber.java:71)
	at com.google.common.eventbus.Dispatcher$PerThreadQueuedDispatcher.dispatch(Dispatcher.java:116)
	at com.google.common.eventbus.EventBus.post(EventBus.java:217)
	at net.minecraftforge.fml.common.LoadController.distributeStateMessage(LoadController.java:135)
	at net.minecraftforge.fml.common.Loader.loadMods(Loader.java:593)
	at net.minecraftforge.fml.client.FMLClientHandler.beginMinecraftLoading(FMLClientHandler.java:225)
	at net.minecraft.client.Minecraft.init(Minecraft.java:513)
	at net.minecraft.client.Minecraft.run(Minecraft.java:421)
	at net.minecraft.client.main.Main.main(Main.java:118)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
	at java.lang.reflect.Method.invoke(Unknown Source)
	at net.minecraft.launchwrapper.Launch.launch(Launch.java:135)
	at net.minecraft.launchwrapper.Launch.main(Launch.java:28)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
	at java.lang.reflect.Method.invoke(Unknown Source)
	at net.minecraftforge.gradle.GradleStartCommon.launch(GradleStartCommon.java:97)
	at GradleStart.main(GradleStart.java:25)
Caused by: net.minecraftforge.fml.common.LoaderException: Attempted to load a proxy type net.enderlook.extracraft.proxy.ClientProxy into net.enderlook.extracraft.ExtraCraft.instance, but the types don't match
	at net.minecraftforge.fml.common.ProxyInjector.inject(ProxyInjector.java:95)
	... 43 more

Not sure why. I've done this interface:

public interface IProxy {
	public void preInitializationEvent(FMLPreInitializationEvent event);
	public void initializationEvent(FMLInitializationEvent event);
	public void postInitializationEvent(FMLPostInitializationEvent event);	
}

@Mod(modid = ExtraCraft.MOD_ID, name = ExtraCraft.NAME, version = ExtraCraft.VERSION)
@Mod.EventBusSubscriber(modid = ExtraCraft.MOD_ID)
public class ExtraCraft implements IProxy {
	
	public static final String MOD_ID = "extracraft";
	public static final String NAME = "ExtraCraft Mod";
	public static final String VERSION = "0.1.0.0";	
	public static Configuration config;
	static Logger logger;
	
	@SidedProxy(clientSide = "net.enderlook.extracraft.proxy.ClientProxy", serverSide = "net.enderlook.extracraft.proxy.ServerProxy")
	
	@Mod.Instance
	private static ExtraCraft instance;
	
	@Mod.EventHandler
	public void preInitializationEvent(FMLPreInitializationEvent event) {
		logger = event.getModLog();
		File directory = event.getModConfigurationDirectory();
		config = new Configuration(new File(directory.getPath(), "modtut.cfg"));
		Config.readConfig();
		ModBlocks.init();;
	}
	
	@Mod.EventHandler
	public void initializationEvent(FMLInitializationEvent event) {
		ModRecipes.init();
	}
	
	@Mod.EventHandler
	public void postInitializationEvent(FMLPostInitializationEvent event) {
		if (config.hasChanged()) {
			config.save();
		}
	}
	
	public static String appendModID(String value) {
		return MOD_ID + ":" + value;
	}
	
	/*public void registerRenderers() {}*/
}

@Mod.EventBusSubscriber(modid = ExtraCraft.MOD_ID, value = Side.CLIENT)
public class ClientProxy implements IProxy {
	
	@Override
	public void preInitializationEvent(FMLPreInitializationEvent event) {}
	
	@Override
	public void initializationEvent(FMLInitializationEvent event) {}
	
	@Override
	public void postInitializationEvent(FMLPostInitializationEvent event) {}
	
	@SubscribeEvent
	public static void registerModels(ModelRegistryEvent event) {		
		ModItems.initializeItemsModels();
		ModBlocks.initializeBlockModels();
	}

	/*public void registerRenderers() {}*/
}

@Mod.EventBusSubscriber(modid = ExtraCraft.MOD_ID, value = Side.SERVER)
public class ServerProxy implements IProxy {
	
	@Override
	public void preInitializationEvent(FMLPreInitializationEvent event) {}
	
	@Override
	public void initializationEvent(FMLInitializationEvent event) {}
	
	@Override
	public void postInitializationEvent(FMLPostInitializationEvent event) {}
}

Edited August 5, 20187 yr by Enderlook

Quote

August 5, 20187 yr

6 minutes ago, Enderlook said:


	@SidedProxy(clientSide = "net.enderlook.extracraft.proxy.ClientProxy", serverSide = "net.enderlook.extracraft.proxy.ServerProxy")
	
	@Mod.Instance
	private static ExtraCraft instance;

You're missing the proxy field.

Apparently I'm a complete and utter jerk and come to this forum just like to make fun of people, be confrontational, and make your personal life miserable. If you think this is the case, JUST REPORT ME. Otherwise you're just going to get reported when you reply to my posts and point it out, because odds are, I was trying to be nice.

Exception: If you do not understand Java, I WILL NOT HELP YOU and your thread will get locked.

DO NOT PM ME WITH PROBLEMS. No help will be given.

Quote

August 5, 20187 yr

Author

1 minute ago, Draco18s said:

You're missing the proxy field.

Oh, before the "update" I had:

private static CommonProxy proxy;

But now I don't use a CommonProxy, so, what should I write? ClientProxy, ServerProxy or make both proxies?

Quote

August 5, 20187 yr

Author

9 minutes ago, diesieben07 said:

A variable with a value of the interface itself? It works... but I don't understand its usage...thanks

Edited August 5, 20187 yr by Enderlook

Quote

August 5, 20187 yr

Author

4 minutes ago, diesieben07 said:

If you do not understand the purpose of interfaces I suggest you read up on basic OOP.

Ok, I'm quite new in Java and its the first programming language I see with interfaces (I also now just a bit of Python, JS and VBA).

Edited August 5, 20187 yr by Enderlook

Quote

August 5, 20187 yr

Author

Also, I've another question. In my class when I register all the items or all the blocks, should I declare each variable (item or block) as null and then in a method (init) give them values?

I mean, instead of (a piece of my class):

@EventBusSubscriber(modid = ExtraCraft.MOD_ID)
public class ModItems {
	
	public static final ToolMaterial EMERALD_MATERIAL_TOOL = EnumHelper.addToolMaterial("Emerald", 3, 1200, 9f, 3f, 22).setRepairItem(new ItemStack(Items.EMERALD));
			
	@ObjectHolder(ExtraCraft.MOD_ID + ":candy")
	public static final ItemCandy ITEM_CANDY = new ItemCandy();	
	@ObjectHolder(ExtraCraft.MOD_ID + ":diamond_apple")
	public static final ItemDiamondApple ITEM_DIAMOND_APPLE = new ItemDiamondApple();  
	@ObjectHolder(ExtraCraft.MOD_ID + ":emerald_axe")
	public static final ItemEmeraldAxe ITEM_EMERALD_AXE = new ItemEmeraldAxe();
  
	public static final CItemFood[] ITEMS_FOOD = new CItemFood[] {
		ITEM_CANDY, ITEM_DIAMOND_APPLE
	};
	public static final CItemToolAxe[] ITEMS_TOOL_AXE = new CItemToolAxe[] {
		ITEM_EMERALD_AXE
	};
	[...]
}

Or use an init method:

@EventBusSubscriber(modid = ExtraCraft.MOD_ID)
public class ModItems {
	
	public static ToolMaterial EMERALD_MATERIAL_TOOL = null;
			
	@ObjectHolder(ExtraCraft.MOD_ID + ":candy")
	public static ItemCandy ITEM_CANDY = null;
	@ObjectHolder(ExtraCraft.MOD_ID + ":diamond_apple")
	public static ItemDiamondApple ITEM_DIAMOND_APPLE = null;  
	@ObjectHolder(ExtraCraft.MOD_ID + ":emerald_axe")
	public static ItemEmeraldAxe ITEM_EMERALD_AXE = null;
  
	public static CItemFood[] ITEMS_FOOD;
	public static CItemToolAxe[] ITEMS_TOOL_AXE;

	public static void init() {
		EMERALD_MATERIAL_TOOL = EnumHelper.addToolMaterial("Emerald", 3, 1200, 9f, 3f, 22).setRepairItem(new ItemStack(Items.EMERALD));
		ITEMS_FOOD = new CItemFood[] {
			new ItemCandy(), new ItemDiamondApple()
		};
		ITEMS_TOOL_AXE;= new CItemToolAxe[] {
			new ItemEmeraldAxe()
		};
	}

	[...]
}

Or even maybe I should instead of use "new ItemCandy(), new ItemDiamondApple()" etc use "ITEM_CANDY, ITEM_DIAMON_APPLE" etc in the array? But in order to do that I won't be able to "ModItems.init()" on "preInitializationEvent" in ExtraCraft because items will be null...

Quote

August 5, 20187 yr

22 minutes ago, Enderlook said:


	public static final CItemFood[] ITEMS_FOOD = new CItemFood[] {
		ITEM_CANDY, ITEM_DIAMOND_APPLE
	};
	public static final CItemToolAxe[] ITEMS_TOOL_AXE = new CItemToolAxe[] {
		ITEM_EMERALD_AXE
	};

Congrats you've created an array of nulls!

Apparently I'm a complete and utter jerk and come to this forum just like to make fun of people, be confrontational, and make your personal life miserable. If you think this is the case, JUST REPORT ME. Otherwise you're just going to get reported when you reply to my posts and point it out, because odds are, I was trying to be nice.

Exception: If you do not understand Java, I WILL NOT HELP YOU and your thread will get locked.

DO NOT PM ME WITH PROBLEMS. No help will be given.

Quote

August 5, 20187 yr

Author

1 minute ago, Draco18s said:

Congrats you've created an array of nulls!

Are you talking about the first example or the second one? Because I'm using right now the first for items and it works perfectly. Also, I'm using something similar to the second one for blocks and also work, that is why I'm asking about it.

Quote

August 5, 20187 yr

Ah, yes. I missed seeing the break/incorrect visual parsing.

In any case, yes, you want to use an Init method.

Apparently I'm a complete and utter jerk and come to this forum just like to make fun of people, be confrontational, and make your personal life miserable. If you think this is the case, JUST REPORT ME. Otherwise you're just going to get reported when you reply to my posts and point it out, because odds are, I was trying to be nice.

Exception: If you do not understand Java, I WILL NOT HELP YOU and your thread will get locked.

DO NOT PM ME WITH PROBLEMS. No help will be given.

Quote

August 5, 20187 yr

Author

1 minute ago, Draco18s said:

Ah, yes. I missed seeing the break/incorrect visual parsing.

In any case, yes, you want to use an Init method.

Ok, and it's fine using ´new´ for the items? Or I should do something especial?

		ITEMS_FOOD = new CItemFood[] {
			new ItemCandy(), new ItemDiamondApple()
		};

Quote

August 5, 20187 yr

If your items set their own registry name, yes.

Apparently I'm a complete and utter jerk and come to this forum just like to make fun of people, be confrontational, and make your personal life miserable. If you think this is the case, JUST REPORT ME. Otherwise you're just going to get reported when you reply to my posts and point it out, because odds are, I was trying to be nice.

Exception: If you do not understand Java, I WILL NOT HELP YOU and your thread will get locked.

DO NOT PM ME WITH PROBLEMS. No help will be given.

Quote

August 5, 20187 yr

Author

1 minute ago, Draco18s said:

If your items set their own registry name, yes.

...mmm... each item class has its own ´setRegistryName()´ with a value, do you mean that?

Quote

August 5, 20187 yr

Yes.

Apparently I'm a complete and utter jerk and come to this forum just like to make fun of people, be confrontational, and make your personal life miserable. If you think this is the case, JUST REPORT ME. Otherwise you're just going to get reported when you reply to my posts and point it out, because odds are, I was trying to be nice.

Exception: If you do not understand Java, I WILL NOT HELP YOU and your thread will get locked.

DO NOT PM ME WITH PROBLEMS. No help will be given.

Quote

1

August 5, 20187 yr

Author

Ok, thanks!

Quote

Sign In

How to improve my codding style for registry

Featured Replies

Join the conversation

Posts

Orbital strike cannon fishing rod mod

Bad pathfinding for big mobs (1.21.1)

Processor failed, invalid outputs

Processor failed, invalid outputs

Orbital strike cannon fishing rod mod

Who's Online (See full list)

Important Information

Account

Navigation

Search

Configure browser push notifications

Chrome (Android)

Chrome (Desktop)

Safari (iOS 16.4+)

Safari (macOS)

Edge (Android)

Edge (Desktop)

Firefox (Android)

Firefox (Desktop)