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

Add conventional tag for all animals #4168

Open
wants to merge 4 commits into
base: 1.21.1
Choose a base branch
from

Conversation

CammiePone
Copy link

I'm currently working on a mod, and found a sudden need for a tag that included all animals, ideally one that other mods would include their animals in. It doesn't exist in the base game, and this is the next best thing.

As for why I can't just use an instanceof check, it's because I also need to cover players, illagers, villagers, and wandering traders while still allowing the list of acceptable entities to be configurable by the end user. I'm using a tag of my own, and then including players, villagers, and wandering traders explicitly, then using the existing minecraft:illagers tag for all the illagers, and would like to use a c:animals tag for animals.

@CammiePone
Copy link
Author

I've already made a PR to NeoForge neoforged/NeoForge#1596, and if wanted, I can make a PR to the 1.21.2 branch as well

@TelepathicGrunt
Copy link
Contributor

Seems fine to me

@Shnupbups
Copy link
Contributor

Shnupbups commented Nov 9, 2024

What counts as an animal?
You're missing Bats, but I assume that's an oversight.
But I see you have, for example, the Strider - so fictional mobs count - but not the Allay. What exactly excludes that? But then, Vexes are corrupted Allays, so maybe them too...
Do Spiders, Cave Spiders, and Silverfish count, despite also being monsters? If so, what about Endermites, Phantoms, Ravagers, Guardians and Elder Guardians?
You have Hoglins, so I assume being hostile doesn't exclude something - though you're missing the Zoglin despite having the Zombie Horse, which seems contradictory.
But then what about Piglins, Piglin Brutes and Zombified Piglins? They're somewhat animal, too...

My point is, being 'animal' is somewhat arbitrary to a degree - I'm not opposed to there being a tag for it, but there would need to be a consensus on what counts.

@TelepathicGrunt
Copy link
Contributor

TelepathicGrunt commented Nov 9, 2024

@CammiePone Can you add a javadoc to the tag on fabric and neo to say the tag is specifically about mobs that extend Animal class? This means bats, cods, spiders, tadpole, and other creatures are excluded from the tag. If this is not what you were looking for with the animal tag, then some rethinking might be needed. Maybe it should be c:breedable for mobs that can be bred. Not restricted to Animal class. Or c:feedable? Or something else

@CammiePone
Copy link
Author

yeah, i can do that when i get home

@@ -36,6 +36,10 @@ private ConventionalEntityTypeTags() {

public static final TagKey<EntityType<?>> MINECARTS = register("minecarts");
public static final TagKey<EntityType<?>> BOATS = register("boats");

/**
* Tag containing entity types that extend AnimalEntity
Copy link
Contributor

Choose a reason for hiding this comment

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

If this is the case, then Tadpole, Bats, Squid, Glow Squid, and all the fish needs to be removed from the tag.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants