Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
Prospector opened this issue Jan 13, 2020 · 18 comments · Fixed by #47
Open

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

Prospector opened this issue Jan 13, 2020 · 18 comments · Fixed by #47
Labels
bug Something isn't working

Comments

@Prospector
Copy link

See vanilla and everything else which uses plural named tags.

@Prospector Prospector added the bug Something isn't working label Jan 13, 2020
@Juuxel
Copy link
Member

Juuxel commented Jan 13, 2020

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
Copy link
Author

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
Copy link

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
Copy link

Sollace commented Jan 13, 2020

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
Copy link
Contributor

i509VCB commented Jan 13, 2020

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
Copy link

Sollace commented Jan 13, 2020

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
Copy link
Contributor

i509VCB commented Jan 13, 2020

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
Copy link
Member

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

@Prospector
Copy link
Author

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

@LemmaEOF
Copy link
Member

oh right, circular reference issues.

@Prospector
Copy link
Author

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
Copy link
Contributor

i509VCB commented Jan 13, 2020

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
Copy link
Author

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

@i509VCB
Copy link
Contributor

i509VCB commented Jan 14, 2020

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
Copy link
Member

Juuxel commented Jan 16, 2020

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
Copy link
Contributor

i509VCB commented Jan 16, 2020

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 i509VCB linked a pull request Mar 3, 2020 that will close this issue
@i509VCB
Copy link
Contributor

i509VCB commented Mar 3, 2020

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

@modmuss50
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants