Jump to content

[1.12.1] @ObjectHolder versus IForgeRegistry.getValue()


jabelar

Recommended Posts

Is there any technical reason that the object holder injection is better than using the getValue() method of the registry itself and storing that in a public field? Are they equivalent? Or is there a performance or logical reason that object holder is preferred method? 

 

I'm asking because I'm porting some older mods and in most I set the registry name within the block or item constructor so if I use injection now I have to go through all my blocks and items and dig up the registry names and copy those into the object holder annotations. Seems easier for me to simply to use the getValue() and retrieve the registry name instead rather than hard-coding it into annotations. Just being lazy, but it made me think about how the injection works and mostly I just want to understand whether it is equivalent -- I plan to mostly use object holder in the future.

Check out my tutorials here: http://jabelarminecraft.blogspot.com/

Link to comment
Share on other sites

Lookup performance wise, ObjectHolder is probably faster. After that, it's going to be the same.

 

Personally I'd recommend ObjectHolder.

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.

Link to comment
Share on other sites

s/probably/DEFINITLY/

Static field access vs a map lookup everytime you're accessing the value? That's a DEFINITE performance increase.

Beyond that, for your own stuff it should be simple as hell to use @ObjectHolder

@ObjectHolder("my_mod")
	public class MyBlocks {
	  public static final name1 = null;
	  public static final name2 = null;
	  public static final name3 = null;
	  public static final name4 = null;
	}

I do Forge for free, however the servers to run it arn't free, so anything is appreciated.
Consider supporting the team on Patreon

Link to comment
Share on other sites

37 minutes ago, LexManos said:

s/probably/DEFINITLY/

Static field access vs a map lookup everytime you're accessing the value?

No one said "every time" Lex:

3 hours ago, jabelar said:

using the getValue() method of the registry itself and storing that in a public 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.

Link to comment
Share on other sites

Eah missed that part. But point still stands as most people who use getValue don't cache it. And you have issues with making sure you rebuild that cache when things change. There is a event for it. But it's more work then just using @ObjectHolder which is what they brought up.

I do Forge for free, however the servers to run it arn't free, so anything is appreciated.
Consider supporting the team on Patreon

Link to comment
Share on other sites

6 minutes ago, LexManos said:

But it's more work then just using @ObjectHolder which is what they brought up.

Sure. Which is why I recommended it.

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.

Link to comment
Share on other sites

Thanks guys. As discussed I understood that lookup would be somewhat of a performance hit, although frankly the vanilla code uses lookups a lot. But yeah I meant store it for access as well.

 

Lex, regarding your statement about "your own stuff should be simple as hell", the problem with your example is the names of the fields. They either need to match the registry name (which is kinda weird because they use the underscore style naming) or you have to further add additional object holder annotations to match it all up. Furthermore, for my own stuff there is stuff I'm porting in which case knowing the registry name actually takes a bit of work -- I have to go into each of my classes and copy the text out and put it into the annotation.

 

Anyway, I just wanted to make sure they were functionally equivalent in case I was missing something fundamental about the object holder concept. Thanks!

Check out my tutorials here: http://jabelarminecraft.blogspot.com/

Link to comment
Share on other sites

Yes the field names need to match up. But that shouldn't be an issue.

You issue seems to be that you don't want to lookup the registry names.

There isn't anything we can do to fix that.

Even get calls would need to have the name...

I do Forge for free, however the servers to run it arn't free, so anything is appreciated.
Consider supporting the team on Patreon

Link to comment
Share on other sites

The issue arises because of my existing codebase, where all the unlocalized/registry names were tucked within each class and so now collecting them all in the registration class is painful.

 

From an older version of my mod I had unlocalized name / registry name set in my constructors and so when I create a public static field I would simply call new MyCustomItem1() and so forth and to register them I would pull up the unlocalized name.

 

So in my registration (which previously was in a proxy) it would look like this:

 

 public static final customItem1 = new MyCustomItem1();

 public static final customItem2 = new MyCustomItem2();

...

 public static final customItem50 = new MyCustomItem50();

 

and I would register them simply by the old methods where I could do:

 

  GameRegistry.registerItem(customItem1, customItem1.getUnlocalizedName());
   GameRegistry.registerItem(customItem2, customItem2.getUnlocalizedName());

....
   GameRegistry.registerItem(customItem50, customItem50.getUnlocalizedName());
 

 

 So to convert to @ObjectHolder I have to go to dozens of item classes, dozens of block classes, sound instances sprinkled throughout my code and such and find the actual unlocalized name string and copy it over into annotations and it would be easier to continue to use a method similar to the above.

 

The problem is that maintaining mods across versions becomes a fulltime job if you've got even a few significant code bases. Not a big deal but it adds up.

 

I appreciate all the efforts to keep improving Forge, but it is a bit disheartening every time a new approach is used. Converting IEEPs to capabilities, achievements to advancements, recipes to JSON etc. Depending on what your mod focuses on it can actually be a significant rewrite.

 

So I'm just being lazy and whining. It is taking a few hours per mod, but I am converting over to Object Holder method...

Edited by jabelar

Check out my tutorials here: http://jabelarminecraft.blogspot.com/

Link to comment
Share on other sites

6 hours ago, jabelar said:

  GameRegistry.registerItem(customItem1, customItem1.getUnlocalizedName());

   GameRegistry.registerItem(customItem2, customItem2.getUnlocalizedName());

....
   GameRegistry.registerItem(customItem50, customItem50.getUnlocalizedName());

OH FOR FUCK'S SAKE

 

DO NOT USE getUnlocalizedName() HERE. THIS IS WHY WE HAVE REGISTRY NAMES AND getRegistryName()!

 

Not to mention that in 1.12, GameRegistry.register(...) is private (use the RegistryEvent.Register<Item> event) and GameRegistry.registerItem() has been removed as it was deprecated back in 1.10.2 in favor of GameRegistry.register(...)!

 

Also, I see no point in using ObjectHolder for your own items. None what so ever. You're already creating them, just stuff them into the static field yourself. ObjectHolder exists to get other mod's items without needing some kind of API or using getValue().

Edited by Draco18s

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.

Link to comment
Share on other sites

You should also never statically construct your blocks/items.

So really you need to go through and cleanup your code.

Yes, it's work, but you should do it.

 

@Draco18sThere IS reason to use @ObjectHolder for your own items, that his current method doesn't support. OVERRIDES. If anyone comes along and says "Screw you I want to override the functionality of your item" then you just silently detect and use it. This way doesn't support that.

  • Like 1

I do Forge for free, however the servers to run it arn't free, so anything is appreciated.
Consider supporting the team on Patreon

Link to comment
Share on other sites

Ah, thanks for that @LexManos

:)

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.

Link to comment
Share on other sites

  • 1 year later...
On 9/13/2017 at 5:44 AM, LexManos said:

And you have issues with making sure you rebuild that cache when things change. There is a event for it.

Sorry for the necro, whats the event? I can't find it anywhere. The fields I'm using for caching are instance fields so I can't use the @ObjectHolder annotation.

 

Edit: its the RegistryEvent.Register<> event

Edited by Cadiboo

About Me

Spoiler

My Discord - Cadiboo#8887

My WebsiteCadiboo.github.io

My ModsCadiboo.github.io/projects

My TutorialsCadiboo.github.io/tutorials

Versions below 1.14.4 are no longer supported on this forum. Use the latest version to receive support.

When asking support remember to include all relevant log files (logs are found in .minecraft/logs/), code if applicable and screenshots if possible.

Only download mods from trusted sites like CurseForge (minecraft.curseforge.com). A list of bad sites can be found here, with more information available at stopmodreposts.org

Edit your own signature at www.minecraftforge.net/forum/settings/signature/ (Make sure to check its compatibility with the Dark Theme)

Link to comment
Share on other sites

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.
Note: Your post will require moderator approval before it will be visible.

Guest
Unfortunately, your content contains terms that we do not allow. Please edit your content to remove the highlighted words below.
Reply to this topic...

×   Pasted as rich text.   Restore formatting

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

Announcements



×
×
  • Create New...

Important Information

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