-
Notifications
You must be signed in to change notification settings - Fork 39
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
Fix path loading when working with optifine data. #94
Conversation
var firstChunk = pathChunks.get(0); | ||
|
||
// Remove the file name from the location path or trailing slash | ||
var locationPath = new LinkedList<>(Arrays.asList(location.getPath().split("/"))); |
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.
Strips the end of the location path if it is a path to a file with extension, and removes trailing stashes.
return transformPath(path.substring(3), new Identifier(location.getNamespace(), location.getPath().substring(0, location.getPath().lastIndexOf('/')))); | ||
|
||
// Uses a explicit namespace | ||
if (pathChunks.get(0).contains(":")) { |
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.
Handles loading from namespaces other than 'minecraft' or 'dorianpb'.
return new Identifier("dorianpb", "cem/" + path.substring(2)); | ||
|
||
// Specifies an absolute path | ||
if (firstChunk.equals("")) { |
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.
If the path had a leading slash then start the path from the root of the location namespace.
|
||
// Specifies a relative path | ||
else if (firstChunk.equals(".")) { | ||
pathChunks.removeFirst(); |
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.
If the path is relative (starts with "./") then the path starts from the path of location.
return new Identifier(location.getNamespace(), location.getPath().substring(0, location.getPath().lastIndexOf('/') + 1) + path); | ||
|
||
// Specifies an optifine folder or dorianpb folder path | ||
else if (firstChunk.equals("~")) { |
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.
This is a bit tricky here. If we are loading a resource from within the "dorianpb" namespace, then we follow the existing logic for paths starting with "", however in the event of handling an optifine resource pack, it is expected that "" refers to "minecraft/optifine" (excluding the cem folder). I double checked this logic by referring to Optifine source code.
|
||
// Move the location path up for each ".." chunk at the beginning of the path | ||
else if (firstChunk.equals("..")) { | ||
var basePathChunks = new LinkedList<>(Arrays.asList(location.getPath().split("/"))); |
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.
If the path starts with parent folder path expressions ("..") we walk up the path from parent to parent for each ".." chunk in the path. This is the expected behavior when working with *nix path expressions.
|
||
// Deal with optifines rather stange pathing behaviour | ||
// When Optifine loads a texture path does not contain "/", it loads relative to the file. | ||
} else if (location.getNamespace().equals("minecraft") && path.contains("/")) { |
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.
This was added because of some nuts behavior I found in the optifine source code. If a path contains slashes then the path is assumed to be relative to the namespace root. I assume you don't want to do with for cem data in the "dorianpb" namespace, so I only perform the path adjustment if we are in the "minecraft" namespace.
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs within 28 days. Thank you for your contributions. |
This issue has been automatically closed due to it being stale for 28 days. Thank you to everyone who contributed. |
This PR updates CemFairy#transformPath to better handle transforming resource paths.
This includes a few major adjustments such as proper handling of parent path expressions ("..") as well as fixes the way resource paths are loaded from Optifine compatible resource packs containing cem data.
Likely fixes: #75, #72, #58, #56, and others.