Am I doing custom CatalogTypes right?

Basically, I’m developing this Menu plugin, and at the moment it’s very WIP, but I’m trying to follow all the fancy Sponge conventions, so I want to convert the three enums currently in the plugin to CatalogTypes (especially since I want users to have the ability to add to these enums). I tried converting one, but I’m unsure if it’s set up properly, or if it’s written conventionally (in sponge’s eyes), so I was wondering if someone who knows about catalog types could look it over and verify that it’s conventionally correct and won’t break anything (and, at the moment, I don’t have time to test on a server… which is annoying, and I’ll do that some later, but having a second opinion is always helpful). Anyways, the relevant code is:

The actual catalog type:
https://github.com/SocraticPhoenix/Menu/blob/master/src/main/java/com/gmail/socraticphoenix/sponge/menu/InputType.java
The enum-imitation interface with fields and stuff:
https://github.com/SocraticPhoenix/Menu/blob/master/src/main/java/com/gmail/socraticphoenix/sponge/menu/InputTypes.java

The registry module:
https://github.com/SocraticPhoenix/Menu/blob/master/src/main/java/com/gmail/socraticphoenix/sponge/menu/catalogs/InputTypeCatalog.java

The data translator:
https://github.com/SocraticPhoenix/Menu/blob/master/src/main/java/com/gmail/socraticphoenix/sponge/menu/data/CatalogDataTranslator.java

and the registration:
https://github.com/SocraticPhoenix/Menu/blob/master/src/main/java/com/gmail/socraticphoenix/sponge/menu/MenuPlugin.java#L115

Thanks for any help!
(and if I have no idea what I’m doing, please point that out :slight_smile:)

  • Rather than an interface, InputTypes should be a final class, with a private no-args constructor that does nothing. This ensures the impossibility of instantiation.
  • Each InputType should be not only static but final.
  • IIRC @DelayedRegistration goes on registerDefaults so as to make sure its registration phase is after the module is added (and for that reason module registration should probably go in pre-initialization).
  • You may want to do a final field in CatalogDataTranslator for the TypeToken, since it only needs to be constructed once.

Other than that, though, looks fantastic.
Actually, I think Sponge already auto-serializes all CatalogTypes by ID only. I could be wrong, but try not registering the translator and see what happens.

Thanks very much! And it does indeed appear that sponge handles catalog types (albeit, not with a DataTranslator…)

Edit: also, is the registerDefaults called by sponge? And what Registration Phase should I put in the value? I have the module registration in preinit now, so I put in the init phase, but I’m not sure that’s correct

Yeah, see here:
https://github.com/SpongePowered/SpongeCommon/blob/bleeding/src/main/java/org/spongepowered/common/registry/util/RegistryModuleLoader.java#L61
I’m not sure on the correct init phase for registry modules.

I did some testing, and If I don’t include the annotation, I can just register them myself. At the moment, I think that’s the safest… especially if I want to be able to say "The plugin will be available after phase X)