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

[1.20.6] Prevent the @OnlyIn being misused on @EventBusSubscriber and @Mod annotated classes #9891

Merged

Conversation

danorris709
Copy link
Contributor

Adds checks to prevent the OnlyIn annotation being used in erroneous places such as classes annotated with the @Mod, or @EventBusSubscriber as outlined here

@autoforge autoforge bot added Triage This request requires the active attention of the Triage Team. Requires labelling or reviews. 1.20 labels Mar 21, 2024
@autoforge autoforge bot requested a review from a team March 21, 2024 15:58
@PaintNinja PaintNinja self-requested a review March 21, 2024 20:16
@LexManos
Copy link
Member

Is there a real need for this? Have you seen many mods do this?
The code looks fine, a little verbose, but good enough.
If Paint wants it, he can pull it.

@PaintNinja
Copy link
Contributor

PaintNinja commented Mar 22, 2024

Is there a real need for this? Have you seen many mods do this?

I added it to the wishlist because I noticed some devs do this thinking it fixes crashing servers, when in reality it just gives a different error at best. It's not common, but the less mods using OnlyIn incorrectly, the better for everyone.

The code looks fine, a little verbose, but good enough. If Paint wants it, he can pull it.

Thanks, I'll merge once my requested changes are implemented. As for the verbosity, half of it seems to be a cleanup of the RuntimeDistCleaner to use less streams

@PaintNinja PaintNinja added Feature This request implements a new feature. Assigned This request has been assigned to a core developer. Will not go stale. Queued Indicates that this PR is queued for merge in the next merge train. Depending on however triggers it and removed Triage This request requires the active attention of the Triage Team. Requires labelling or reviews. labels May 30, 2024
@PaintNinja PaintNinja changed the title fix(annotations): prevent the OnlyIn annotation being used on EventBusSubscriber and Mod annotated classes [1.20.6] Prevent the @OnlyIn being misused on @EventBusSubscriber and @Mod annotated classes May 30, 2024
@PaintNinja PaintNinja merged commit 5505e47 into MinecraftForge:1.20.x May 30, 2024
9 checks passed
@PaintNinja PaintNinja added Cleanup This request reorganizes messy code, or fixing it would require doing so. and removed Queued Indicates that this PR is queued for merge in the next merge train. Depending on however triggers it labels May 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.20 Assigned This request has been assigned to a core developer. Will not go stale. Cleanup This request reorganizes messy code, or fixing it would require doing so. Feature This request implements a new feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants