Skip to content

Fixed support for new egg types in 1.21.5 and later versions - #4910

Open
MattBDev wants to merge 1 commit into
mainfrom
fix/eggs
Open

Fixed support for new egg types in 1.21.5 and later versions#4910
MattBDev wants to merge 1 commit into
mainfrom
fix/eggs

Conversation

@MattBDev

Copy link
Copy Markdown
Contributor

Overview

Fixes an issue where blue and brown eggs aren't treated the same as regular eggs.

Description

Includes a new variable to detect all eggs in game and treat them the same.

Submitter Checklist

  • Make sure you are opening from a topic branch (/feature/fix/docs/ branch (right side)) and not your main branch.
  • Ensure that the pull request title represents the desired changelog entry.
  • New public fields and methods are annotated with @since TODO.
  • I read and followed the contribution guidelines.

Copilot AI review requested due to automatic review settings July 29, 2026 18:58
@MattBDev
MattBDev requested a review from a team as a code owner July 29, 2026 18:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This pull request updates PlotSquared’s Bukkit PlayerEventListener interaction handling to recognize newly added egg item types (e.g., blue/brown eggs in 1.21.5+) the same way as the classic egg, ensuring they trigger the same spawn-related event classification.

Changes:

  • Introduces a shared isEgg boolean to detect egg-like materials and reuse the check across Paper and non-Paper branches.
  • Expands the Paper branch condition to treat any “*egg” material similarly, matching the non-Paper behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

}
} else {
if (type.toString().toLowerCase().endsWith("egg")) {
if (MaterialTags.SPAWN_EGGS.isTagged(type) || isEgg) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why do we need the || here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Spawn eggs are not the same as regular eggs so the || here is valid and needed. However, I should have probably reversed this check so isEgg is checked BEFORE the MaterialTag since it will probably be true in almost all cases. I'm willing to change it for PR approval, but it honestly won't make much of a difference.

}
// 1.21.5 (Spring to Life) introduced brown_egg and blue_egg.
// This new variable detects all current and future eggs.
// It could be replaced with an Item Tag or the exact Material in the future.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not wanting to be nit-picky, but why not now already, given the eggs tag was introduced in 1.21.5? Something like

boolean isEgg = Optional.ofNullable(Bukkit.getTag(org.bukkit.Tag.REGISTRY_ITEMS, NamespacedKey.minecraft("eggs"), Material.class))
                        .map(tag -> tag.isTagged(type))
                        .orElseGet(() -> type.name().endsWith("EGG"));

with some caching?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

You're not being nitpicky. I've been gone for awhile so I appreciate the constructive feedback. That is actually a more elegant solution and actually almost what I did. I guess I went the lazy way? 😅

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

A simple Material.EGG.equals(type) should suffice in the orElse part, wasn't thinking much

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.

4 participants