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

Allow passing multiple BiomeTags at the same time for BiomeSelectors #3695

Closed
wants to merge 2 commits into from

Conversation

JT122406
Copy link

@JT122406 JT122406 commented Apr 9, 2024

I have run into situations where with ores for example I want to add them to more than one biomeTag biomes and have to make a separate entry, this solves that by allowing developers to pass in more than one.

@haykam821
Copy link
Contributor

Does creating a tag that references other tags not work for your use case? In most cases, element.isIn(ElementTags.COMBINED) is preferred over element.isIn(ElementTags.FIRST) || element.isIn(ElementTags.SECOND).

@JT122406
Copy link
Author

JT122406 commented Apr 9, 2024

You could do that but I feel like it's unnecessary and you'd need to make tags for every combo you do

/**
* {@return true if this biome is in any of the given {@link TagKey}s}.
*/
default boolean hasTag(TagKey<Biome>... tags) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't sound obvious. I think we need to add anyTagMatch, allTagMatch and noneTagMatch

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree I think something like this would be ideal here

Copy link
Contributor

@maityyy maityyy Apr 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

*To be clear: I'm suggesting the same thing as Stream::anyMatch, Stream::allMatch, Stream::noneMatch

@apple502j
Copy link
Contributor

Hmm, can't we just use && or Predicate.and?

@maityyy
Copy link
Contributor

maityyy commented Apr 10, 2024

Hmm, can't we just use && or Predicate.and?

We can, but we have convenient helper methods and the author suggests expanding it

@modmuss50
Copy link
Member

I'm unsure how useful this boilerplate code is, as pointed about above this is really easy to do in your own mod with Predicate.and.

@modmuss50 modmuss50 added small change priority:low Low priority PRs that don't immediately need to be resolved labels Apr 10, 2024
@JT122406 JT122406 closed this by deleting the head repository May 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority:low Low priority PRs that don't immediately need to be resolved small change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants