CottonMC / CottonResources

Reference mod for ores, ingots, tools, liquids, and all you can imagine.
MIT License
17 stars 12 forks source link

Tags use singular names instead of plural, as per convention #50

Open Prospector opened 4 years ago

Prospector commented 4 years ago

See vanilla and everything else which uses plural named tags.

Juuxel commented 4 years ago

We've talked about this on the Cotton discord and one idea was to use singular for items that are identical (like CR tags, all copper ingots are identical). Plural would be for grouping items (all ingots, for example).

That said, I don't really mind using either format. There just has to be some convention about this.

Prospector commented 4 years ago

I think best is to stick with vanilla's naming and make everything plural. That's what would be intuitive for new modder instead of having to learn a new rule.

modmuss50 commented 4 years ago

Changing now is going to be a right pita, you have given a good reason to do it the current way. It’s a minor detail as well

Sollace commented 4 years ago

I agree, keeping them all plural would be simpler. It also gives us a key distinguishing factor between items and tags when looking at them in json configs.

i509VCB commented 4 years ago

I've been updating to 1.15.1 (see the PR). I did noticed that TechReborn also uses the singular tag name.

I could make a copy of all plural tags inherit their entires from the singular tags, but old mods would break/not function properly if for say TR moved to plural tags but other mods expect the singular tag and TR provides no singular tag to refer to/from.

Sollace commented 4 years ago

I suppose we could set up some aliasing mechanism so the two can be used interchangeably. Just need some way to convert between singular and plural forms.

i509VCB commented 4 years ago

I could try to hack something together but I feel it would be too complicated for its own good.

The other option is just nuke the singular tags and move to plural (this would require mods like TechReborn to change their common tags and break every other mod which uses the recipes).

LemmaEOF commented 4 years ago

Luckily, tags have this neat function where you can put a tag inside another tag!

Prospector commented 4 years ago

That doesn't help as it only works one way. If you try to reference two tags inside each other the game crashes.

LemmaEOF commented 4 years ago

oh right, circular reference issues.

Prospector commented 4 years ago

Also @modmuss50 it being a minor PITA right now is nothing compared to the pain it will be later down the road with even more fabric mods all with their own naming conventions. We've gotta follow vanilla on this or it will be a disaster.

i509VCB commented 4 years ago

I would be fine just nuking it to plurals and breaking now rather than later, the only mod i have which depends on CR optionally could easily have a small update pushed.

Prospector commented 4 years ago

I think TR is the only mod this will be a big pain for due to the amount of recipes and tags

i509VCB commented 4 years ago

I don't think it would be that bad if you opened up every json file in npp at once and just replaced all the singular entries, I could do that mess in a pr if you want.

Juuxel commented 4 years ago

Adorn also has #c:stone_rod, but shouldn't be too hard to make plural (it's used in a lot of recipes, but a batch replace should work).

i509VCB commented 4 years ago

So I am going to shift to plural tags for 1.15.2 (snapshot season). Just putting this to notify @modmuss50 @Prospector and @Juuxel

(any objections please respond)

i509VCB commented 4 years ago

So we are using 1.15.2 as a deprecation step, in 1.16 we will remove the singular tags

modmuss50 commented 4 years ago

Im happy to do this for 1.16 TR, will need to write a script to make the changes.