[SOLVED] GameRegistry.ObjectHolder method-based initialisation

[hoodwink_dude]

I want to reduce literals usage so that I have the flexibility to change the registry name in the future but I also don't want to create global constants. Considering the fact that getRegistryName is marked with the Final access modifier, the result should be constant for each class.

Am I just being stupid or is there any way for me to achieve this without a Constants class?

Edited April 29, 2018 by hoodwink_dude
Marked as solved

[Cadiboo]

57 minutes ago, hoodwink_dude said:

I want to reduce literals usage so that I have the flexibility to change the registry name in the future but I also don't want to create global constants. Considering the fact that getRegistryName is marked with the Final access modifier, the result should be constant for each class.

Am I just being stupid or is there any way for me to achieve this without a Constants class?

I don't know if this is what you were looking for but this is how I instantiate my items

public class ModItems {
	
	public static final ItemFlamethrower			FLAMETHROWER			= new ItemFlamethrower("flamethrower");
	public static final ItemHammer					HAMMER					= new ItemHammer("hammer");
	public static final ItemRailgun					RAILGUN					= new ItemRailgun("railgun");
	public static final ItemCoilgun					COILGUN					= new ItemCoilgun("coilgun");
	public static final ItemPlasmagun				PLASMA_GUN				= new ItemPlasmagun("plasmagun");
	
	public static final Item[] ITEMS = {

		FLAMETHROWER,
		HAMMER,
		RAILGUN,
		COILGUN,
		PLASMA_GUN

	};
	
}

 

[hoodwink_dude]

3 minutes ago, Cadiboo said:

I don't know if this is what you were looking for but this is how I instantiate my items

public class ModItems {
	
	public static final ItemFlamethrower			FLAMETHROWER			= new ItemFlamethrower("flamethrower");
	public static final ItemHammer					HAMMER					= new ItemHammer("hammer");
	public static final ItemRailgun					RAILGUN					= new ItemRailgun("railgun");
	public static final ItemCoilgun					COILGUN					= new ItemCoilgun("coilgun");
	public static final ItemPlasmagun				PLASMA_GUN				= new ItemPlasmagun("plasmagun");
	
	public static final Item[] ITEMS = {

		FLAMETHROWER,
		HAMMER,
		RAILGUN,
		COILGUN,
		PLASMA_GUN

	};
	
}

 

That approach is what I have seen multiple times online. However, I personally think that the logical place to define the names of items is within the respective item classes. I guess that is just a result of me working in Industry for the past 3 months as a Software Engineer Apprentice...

[Cadiboo]

1 minute ago, hoodwink_dude said:

That approach is what I have seen multiple times online. However, I personally think that the logical place to define the names of items is within the respective item classes. I guess that is just a result of me working in Industry for the past 3 months as a Software Engineer Apprentice...

yeah it is an odd way to do it, but it places all your registration logic in one file.

 

Whats wrong with putting

public Blah() {
	super();
	this.setRegistryName(new ResourceLocation(Reference.ID, "blah"));
	this.setUnlocalizedName("blah");
}

in every class?

[hoodwink_dude]

4 minutes ago, Cadiboo said:

yeah it is an odd way to do it, but it places all your registration logic in one file.

 

Whats wrong with putting

public Blah() {
	super();
	this.setRegistryName(new ResourceLocation(Reference.ID, "blah"));
	this.setUnlocalizedName("blah");
}

in every class?

I guess it's just a case of me going a bit overboard with the whole "reduce code duplication" idea...

[Cadiboo]

Personally, I think the first way is great because it allows you to instantiate the same class multiple times with different names. Isn't this the ultimate reduction of code?

 

for example over half my items are instantiated in this way
public static final ItemBase TITANIUM_NUGGET = new ItemBase("titanium_nugget").setNugget().setBeaconPayment();

Or even this way

public static final ItemTool COPPER_SHOVEL = new ItemTool("copper_shovel", ItemTool.COPPER, ItemTool.SPADE_EFFECTIVE_ON, ToolTypes.SHOVEL);

 

this allows me to have 1 class for all my tools, 1 class for all my armor, 1 class for all my normal items (ingots, nuggets etc.) and very few other classes

 

Edited April 29, 2018 by Cadiboo

[hoodwink_dude]

2 minutes ago, Cadiboo said:

Personally, I think the first way is great because it allows you to instantiate the same class multiple times with different names. Isn't this the ultimate reduction of code?

 

for example over half my items are instantiated in this way
public static final ItemBase TITANIUM_NUGGET = new ItemBase("titanium_nugget").setNugget().setBeaconPayment();

Or even this way

public static final ItemTool COPPER_SHOVEL = new ItemTool("copper_shovel", ItemTool.COPPER, ItemTool.SPADE_EFFECTIVE_ON, ToolTypes.SHOVEL);

Good point. Thanks dude

[hoodwink_dude]

1 minute ago, diesieben07 said:

No! Do not do this! This breaks shit.

Registry entries must be created during the proper events (or at least preInit). Not "at some random point when my class gets loaded".

So it's gonna be a bunch of string literals then. Unless I do some form of Dependency Injection (or would that break shit too? If I call the method for loading it all in the preinit then I shouldn't run into issues?)

[hoodwink_dude]

1 minute ago, diesieben07 said:

All that matters is that you create (i.e. instantiate) your registry entries (e.g. items) during some kind of event and register them during RegistryEvent.Register. The rest is completely up to you.

Ah. That makes much more sense now. Thanks

[Cadiboo]

19 minutes ago, diesieben07 said:

No! Do not do this! This breaks shit.

Registry entries must be created during the proper events (or at least preInit). Not "at some random point when my class gets loaded".

I've never had any problems doing this, and I mis-spoke, this is how I am creating them, not how I'm registering them. Registration is done this way

 

@SubscribeEvent
public static void registerItems(final RegistryEvent.Register<Item> event) {
	event.getRegistry().registerAll(cadiboo.wiptech.init.Items.ITEMS);
	//... blocks & ore dict
}

 

Does this still break stuff? I've never had a single problem with this method.

 

Edited April 29, 2018 by Cadiboo

[Cadiboo]

2 minutes ago, diesieben07 said:

setRegistryName must be called when the current mod is known

Do you know why?

I call setRegistryName(MOD_ID, name); does this change this?

[Cadiboo]

1 minute ago, diesieben07 said:

No, not really. It will set the correct ID all the time, but you will get "Dangerous alternative prefix" warnings potentially.

so your saying that I need to move all my Item = new Item() s into RegistryEvent.Register<Item> event

[Cadiboo]

@diesieben07

I'll do that then, thanks!

[jabelar]

5 hours ago, Cadiboo said:

 I've never had a single problem with this method.

 

This is a logical fallacy when talking about proper code development. In most languages the compiler (or in this case JVM) has some leeway in some areas. For example, in some languages the way certain data types are stored can vary so if you depend on that your code might work "without a single problem" for you but then fail in weird ways for someone else.

 

The way to avoid stepping into these subtleties is to understand the allowable variations in the compiler/JVM. In this case I think the main concern is that static initialization order is not guaranteed across classes. Besides registration, modders often run into this with crop blocks where the order of the seed item and the crop block get mixed up and the seed actually gets a null block because the seed gets instantiated before the block does..

 

So the general principle in coding, and particularly in modding related to singleton instantiation, is that you want to fully control the order and timing of the instantiation to be in proper place.

Edited April 29, 2018 by jabelar