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

Do not fail tag datagen when removing IDs not present at runtime #1798

Merged
merged 2 commits into from
Dec 27, 2024

Conversation

62832
Copy link

@62832 62832 commented Dec 24, 2024

Continuing on from the discussion on (the now seemingly redundant PR) #1794, this PR allows Tag data generation in 1.21.1 to proceed as normal in the event that the ITagAppenderExtension::remove methods are used with IDs which may not exist at runtime, bypassing the ExistingFileHelper checks for such IDs should they be marked as removal entries.

@neoforged-automation neoforged-automation bot added the 1.21.1 Targeted at Minecraft 1.21.1 label Dec 24, 2024
@neoforged-pr-publishing
Copy link

  • Publish PR to GitHub Packages

@marchermans
Copy link
Contributor

This is queued to be merged after #1799 as that removes the EFH entirely, possibly superseding this PR.

@marchermans marchermans added the on hold PR or issue is on hold label Dec 24, 2024
@62832
Copy link
Author

62832 commented Dec 24, 2024

Right, but that PR targets the latest version as opposed to 1.21.1. Are you suggesting that the EFH removal would also likely be backported to 1.21.1 in place of this?

@sciwhiz12 sciwhiz12 removed the on hold PR or issue is on hold label Dec 27, 2024
@sciwhiz12
Copy link
Member

Previous comment by @marchermans was mistaken (asked via Discord), as this PR is for 1.21.1 while the ExistingFileHelper changes are in 1.21.4. As those changes will not be backported due to their compatibility-breaking behavior, this PR can move forward.

@sciwhiz12 sciwhiz12 added the enhancement New (or improvement to existing) feature or request label Dec 27, 2024
@sciwhiz12 sciwhiz12 requested a review from embeddedt December 27, 2024 12:52
Copy link
Contributor

@marchermans marchermans left a comment

Choose a reason for hiding this comment

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

Sorry missed that this is for 21.1 this is now fine. My mistake

@marchermans marchermans merged commit e95053c into neoforged:1.21.1 Dec 27, 2024
6 checks passed
@neoforged-releases
Copy link

🚀 This PR has been released as NeoForge version 21.1.91.

@62832 62832 deleted the 1.21.1-tag-remove-optional branch December 27, 2024 21:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.21.1 Targeted at Minecraft 1.21.1 enhancement New (or improvement to existing) feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants