build an API on top of the resource registry
Frontrider opened this issue ยท 38 comments
This all seems to boil down to
public Block register(String name, Block block, Item item);
public Item register(String name, Item item);
We provide a creativeTab in public static space where users can earmark provided items to our tab. Then the interface is way smaller, easier to read, and absolutely as flexible as we can make it.
I don't trust users to use our Tab, so forcing it on them unless they explicitly tell us to use another tab is kind of necessary imo. Trust me, users are idiots.
But if you want I can remove some of the overloading methods.
I will die on the tab hill. If we want re-organized tabs, there are organized tab mods coming down the pipeline.
Open questions:
- We may want to require and return the BlockItem instead of the Block in the case of blocks, so the user has both the item and the block for the chosen registration.
so, like:
public BlockItem register(String name, Block block, BlockItem item);
public Item register(String name, Item item);
When I compare blocks most of the time I need the Block and not the Item. And I think you can get the BlockItem from any given Block by calling Blocks.COBBLESTONE.getItem(). Btw, we probably should also use that to register the BlockItem
I'd be in for getter methods, that can be used to retrieve the correct instance later.
You can retrieve the Block from any BlockItem immediately by calling getBlock(), rather than roundtripping the registry.
You don't have to use the registry. That was just an example. You can just do:
myCustomBlock.getItem();
let me present to you Block's implementation of getItem():
public Item getItem() {
return Item.getItemFromBlock(this);
}
So you're roundtripping the registry any way except BlockItem.getBlock()
Vanilla's register Block doesn't also register an item for that block.
If this was Kotlin I'd just return a tuple<Item, Block>, but that's bad form for Java, which has really no destructuring support.
Hmm, true. It's just that Vanilla's register Block also returns a Block. I would like to follow their standards.
Ultimately the registry roundtrip isn't the end of the world. If that's the compromise that needs to be done, that's fine.
Tupel would be awesome but, java...
My primary problem is that I usually store all my Blocks in public static fields. I never really need the corresponding items outside of crafting and dropped items. And now that these things are handled by jsons I just don't know how often anyone will need the Items.
My usage is really similar, so I don't really have any further objections. Let's get it rolling so we can work on the resources part :)
Give me a few minutes to switch out ItemSetting with BlockItem. Let's hope that users will use the correct namespace.
Still don't like the tab sig and the tab-changing hardware. It's way more moving parts than we need.
The default action for users, if they don't realize they can use our tab or that they can set a tab, is "it doesn't appear in the creative menu", and that's perfectly fine.
If they want it to show up in our tab specifically, they can register it to our tab. It's right there. If you want to remind users to do it, remind them in the javadocs.
If they want it to show up in their own tab, they'll register it to their tab. Both our job and the API user's job are super easy without that extra mixin.
Yeah, but what happens if a user who doesn't use cotton-resources uses our system to register his own copper-ingot and forgets to set a tab? If his mod is loaded before cotton-resources his ingot becomes the default. And suddenly there is no more copper ingot in the creative menu.
It's a failsafe to stop people from screwing up things other mods may use by accident.
then the Issue gets forwarded to their issue tracker. I don't see why this is our problem.
Figuring out which mod is responsible is going to be a nightmare. Can we leave it in for now, merge, and talk about it later?
It changes the semantics of the API use to set their tabs behind their back. We can't really move forward till this is resolved.
Yeah, but honestly that is a compromise I am willing to make. And we do allow our users to specify their own tab if they really want to use it. If you want I can update the javadoc to make it clearer that we will default to the common tab unless they tell us not to. Would that work for you?
That's not a compromise I'm willing to make. This is turning what is easily a two-method API with the same functionality, into a five-method API with a mixin attached which creates no more functionality. I can deal with the convenience method to skip the BlockItem. I really hate the mixin.
Very well, I will remove the mixin. But be warned that every second line on the wiki page will remember the user to use our creative tab.
Because if they register a copper_ingot before we can and add it to their own tab all other mods will use this copper_ingot. Which will no longer be in the common tab. The mixin was a way to make sure that all common items will be in our tab unless whoever registers the item explicitly tells us to use another tab.
The Fabric ItemGroup system allows items to be in another tab, and to auto-assemble ItemGroups. We can just make an ItemGroup of all things namespaced cotton, and have no problems.
Yeah.... probably the best we can do without the mixin can you make a pr for that after I merge mine?
No, it's a legitimate question to answer: When people provide a copper_ingot with no tab set, the selected copper_ingot may well have no tab assigned. If we must have a programmatic solution to this, then I reccommend whining on the console that the mod submitting the item is doing so without an itemGroup set.
That will make it clear in an Issue who is being the bad actor, without becoming a bad actor and misusing other mods' objects ourselves.
Yeah, and I think this is a clusterfuck. Cotton-resources make it so mods don't have to register ANY copper_ore.
I get that the core is "just" about unifying, but in cotton-resources, we can't just remove every other mod's ingots/ores, that is just being mean.
I would suggest simply merging the 2.