Skip to content

Conversation

@ThexXTURBOXx
Copy link
Contributor

@ThexXTURBOXx ThexXTURBOXx commented Feb 18, 2025

I am using symlinks to self-built mods on my system. All those broke in alpha.4 due to the new approach of resolving Namespaces.
This approach tries to address this issue by using fabric's JarFileSystem internals.
It is definitely not the cleanest approach, but it tackles all my use cases as well as the standard use case (just throwing mods into the mods/ folder).

@ThexXTURBOXx ThexXTURBOXx marked this pull request as draft February 20, 2025 20:46
@ThexXTURBOXx
Copy link
Contributor Author

ThexXTURBOXx commented Feb 24, 2025

My new attempt was to fix this issue by reverting all the changes from this PR and just replacing the following line:

with the following:

try (val fs = FileSystemUtil.getJarFileSystem(callerPath, false).get()) {

Since this should always get the same FileSystem and hence also the same Path/URI as Fabric. But even that does not work!
I am out of ideas now, I think...

Edit: FileSystemUtil is net.fabricmc.loader.impl.util.FileSystemUtil

@ThexXTURBOXx ThexXTURBOXx changed the title Compare "canonical paths" Properly compare mod root paths for namespace resolution Feb 25, 2025
@ThexXTURBOXx ThexXTURBOXx marked this pull request as ready for review February 25, 2025 08:44
@ThexXTURBOXx
Copy link
Contributor Author

Fixed - the resulting FileSystem should NOT be closed since the Fabric Loader depends on it!
This PR is now ready for reviews/merge!

Copy link
Member

@calmilamsy calmilamsy left a comment

Choose a reason for hiding this comment

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

Good stuff.

@calmilamsy calmilamsy changed the title Properly compare mod root paths for namespace resolution Don't close filesystems when resolving mod namespaces Feb 25, 2025
@calmilamsy calmilamsy merged commit 2d3c037 into ModificationStation:develop Feb 25, 2025
@mineLdiver
Copy link
Member

While this might work, FileSystemUtil is a part of Fabric Loader's implementation and must not be used outside of it, as it may change in the future without notice and thus make StationAPI not work on that future version of Fabric Loader.

@calmilamsy
Copy link
Member

Ah, I missed that part, I'll push some changes when I'm on proper.

@calmilamsy
Copy link
Member

I also need to fix up the PR name and body again considering my inaccurate read of the code.

@calmilamsy calmilamsy changed the title Don't close filesystems when resolving mod namespaces Use JarFileSystem When Scanning Mods Mar 3, 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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants