Cotton

Cotton

148k Downloads

build an API on top of the resource registry

Frontrider opened this issue ยท 38 comments

commented

build an API on top of the resource registry

commented
commented

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.

commented

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.

commented

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);
commented

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

commented

I'd be in for getter methods, that can be used to retrieve the correct instance later.

commented

You can retrieve the Block from any BlockItem immediately by calling getBlock(), rather than roundtripping the registry.

commented

You don't have to use the registry. That was just an example. You can just do:
myCustomBlock.getItem();

commented

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()

commented

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.

commented

Hmm, true. It's just that Vanilla's register Block also returns a Block. I would like to follow their standards.

commented

Ultimately the registry roundtrip isn't the end of the world. If that's the compromise that needs to be done, that's fine.

commented

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.

commented

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 :)

commented

Give me a few minutes to switch out ItemSetting with BlockItem. Let's hope that users will use the correct namespace.

commented

Done. Can I merge?

commented

Still don't like the tab sig and the tab-changing hardware. It's way more moving parts than we need.

commented

Let those items live on 1 tab, that is enough.

commented

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.

commented

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.

commented

It's a failsafe to stop people from screwing up things other mods may use by accident.

commented

And the mixin isn't really that bad, all it does is add a simple setter to Item.

commented

then the Issue gets forwarded to their issue tracker. I don't see why this is our problem.

commented

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?

commented

I think one setter is not the end of the world.

commented

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.

commented

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?

commented

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.

commented

I told you I would die on the tab hill. I will die on tab hill if needed.

commented

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.

commented

Why? Why is it important for users to use our creative tab?

commented

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.

commented

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.

commented

Yeah.... probably the best we can do without the mixin can you make a pr for that after I merge mine?

commented

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.

commented

Yeah, I'll set it up. Have you made a PR yet?

commented

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.

commented

No, cotton-resources won't even add an item if it already exists.