-
Notifications
You must be signed in to change notification settings - Fork 23
cleanup/java-hacks #145
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
Merged
Merged
cleanup/java-hacks #145
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
Author
|
Note: there are a lot of changed files in this pull request, but most of them are StAPI's own entrypoints patched to meet the new requirements, reviewing all of them isn't necessary. |
calmilamsy
requested changes
Jan 9, 2025
Member
calmilamsy
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only one issue, otherwise this looks great.
...pi-v1/src/main/java/net/modificationstation/stationapi/mixin/tools/ToolMaterialAccessor.java
Show resolved
Hide resolved
...e/src/main/java/net/modificationstation/stationapi/api/mod/entrypoint/EntrypointManager.java
Show resolved
Hide resolved
calmilamsy
approved these changes
Jan 9, 2025
# Conflicts: # station-vanilla-fix-v0/src/main/java/net/modificationstation/stationapi/impl/vanillafix/item/VanillaItemFixImpl.java
mineLdiver
added a commit
that referenced
this pull request
Feb 16, 2025
* Switched from HMI to RetroCommands * Removed broken reimplementation of Divisor's old PlayerAPI. (#139) * Removed Indigo renderer (#140) * Updated UnsafeEvents * ContextVariable * ContextVariable#get * Feature/register helper (#146) * Added register helper methods to RegistryEvent * Remove unused BulkConsumer. * Updated all registries to be entry type bound. * Updated all usages of Registry#register to new helper methods where applicable. * Moved bulk registration helpers implementation to Registry interface and redirected event method helpers to that. * Swapped namespace and rawIdGetter placements so they better reflect the original method arguments. * cleanup/java-hacks (#145) * Cleaned up Java hacks related to entrypoints * Removed EnumFactory. * Removed NativeImage Unsafe buffer cleaner invocation. * Replaced fake BlockItem with null, since MixinExtras allows that, unlike vanilla Mixin's Redirect. * Removed UnsafeProvider. * Updated javadoc for EntrypointManager#registerLookup * Fix a NPE in shears override * Fix a NPE in FuelRegistry * Added StationFlatteningBlock#onStateReplaced * Fixed #159 * Change version
This was referenced Feb 18, 2025
mineLdiver
added a commit
that referenced
this pull request
Jul 5, 2025
* Switched from HMI to RetroCommands * Removed broken reimplementation of Divisor's old PlayerAPI. (#139) * Removed Indigo renderer (#140) * Updated UnsafeEvents * ContextVariable * ContextVariable#get * Feature/register helper (#146) * Added register helper methods to RegistryEvent * Remove unused BulkConsumer. * Updated all registries to be entry type bound. * Updated all usages of Registry#register to new helper methods where applicable. * Moved bulk registration helpers implementation to Registry interface and redirected event method helpers to that. * Swapped namespace and rawIdGetter placements so they better reflect the original method arguments. * cleanup/java-hacks (#145) * Cleaned up Java hacks related to entrypoints * Removed EnumFactory. * Removed NativeImage Unsafe buffer cleaner invocation. * Replaced fake BlockItem with null, since MixinExtras allows that, unlike vanilla Mixin's Redirect. * Removed UnsafeProvider. * Updated javadoc for EntrypointManager#registerLookup * Fix a NPE in shears override * Fix a NPE in FuelRegistry * Added StationFlatteningBlock#onStateReplaced * Fixed #159 * Change version * Merge pull request #165 from ThexXTURBOXx/develop Don't close filesystems when resolving mod namespaces * Update modmenu * Merge pull request #168 * Yeet * Wait this is wrong * Merge branch 'develop' into namespace-change * Make stapi get angree (#169) * Update BiomeMixin.java (#196) * Add Block Support to CustomTooltipProvider (#187) * Add Block support to CustomTooltipProvider * Add some documentation to the interface * Make Leaves' Log Check Work With Modded Logs and Leaves (#186) * Fix * And comment this out cause it's ugly * Fix (#185) * FUCK (#200) * Why mine (#197) * Potentially make mine scream (#188) * Implement onBonemealUse on Vanilla Crops (#170) * Yeet * Also do sapling, also fix multiplayer * Use the appropriate random --------- Co-authored-by: mineLdiver <aabesedin@mail.ru> * Fixed cal L moment (cascaded test worldgen) * Change version --------- Co-authored-by: Nico Mexis <nico.mexis@kabelmail.de> Co-authored-by: calmilamsy <bumbill00@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
StationAPI has been locked to exactly Java 17 for a while now due to using unsafe internal APIs.
This pull request cleans up all Java hacks and allows StationAPI to run on modern Java versions and very likely beyond.
Dropping support for Java 17 isn't a goal of this pull request, StationAPI will stay on Java 17 for the foreseeable future.
1. EnumFactory
EnumFactory has been removed in favor of
@InvokerMixins for a few Minecraft enums, such as ToolMaterial.This solution isn't as widely encompassing as EnumFactory, because newly instantiated enums aren't being added to
#valuesarray,#valueOflookup, and a few other internal caches in Java, but such behavior isn't really desired in the first place, because:In an ideal world, all cases where enum instantiation is currently required should be handled by a dedicated API. ToolMaterial is likely to get such treatment in a followup pull request to Tools API.
Impact on mod developers
EnumFactory removal shouldn't affect a normal StAPI mod developer. Specialized factory classes such as ToolMaterialFactory are still there and simply use the invoker mixins internally now. However, EnumFactory was a part of public API, so if a developer depends on it directly, they should reconsider their approach to whatever problem they're solving.
2. Entrypoints
There are a few things about StAPI's handling of entrypoints which didn't work with modern Java.
Non-public utility fields and listener methods
IMPL_LOOKUP isn't meant to be obtainable, and will eventually be properly encapsulated once Unsafe memory access methods are removed.
StationAPI and UnsafeEvents abused IMPL_LOOKUP to gain access to an entrypoint's non-public members, such as
@Namespace,@Logger, and@Instancefields (or "utility fields"), as well as@EventListenermethods.The solution is to make the entrypoints explicitly register their private lookups to StationAPI and UnsafeEvents if they want to keep their utility fields and listener methods non-public. Example:
Final utility fields
Setting final fields with reflection became impossible in Java 18, and the workaround of using Unsafe will eventually be dealt with too.
StationAPI abused reflection to allow the entrypoints to have final utility fields which are later set to their appropriate objects.
The solution is to disallow final utility fields completely, and instead provide getters for the appropriate values, such as:
Namespace.resolve()- returns the caller class's namespace. Caches the result. Currently marked as experimental since it's based on heuristics, but was tested in IntelliJ runtime, Gradle runtime, in prod, and with modularity, and worked in all cases. Example usage:Namespace#getLogger- returns the namespace's logger. Caches the result. Has the same usage as@Entrypoint.Loggerand is now the backend of it. Example usage:@Entrypoint.Instanceis impossible to implement in a getter form because the instance isn't yet available when the class initializes.Impact on mod developers
If your entrypoints contain non-public and/or final utility fields, or non-public listener methods, read above for the ways to update them, otherwise your entrypoints won't work with these changes.
Conclusion
There were also a couple of internal cases where Unsafe was used in StAPI, but those were patched away silently and without any impact to the mod developers.
This pull request will ensure StationAPI's long-term maintainability by properly respecting Java's integrity. Mod developers will also be able to leverage new Java features, and end users will be able to potentially get better performance from newer Java versions.