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

Fix path loading when working with optifine data. #94

Closed
wants to merge 1 commit into from

Conversation

RobertWHurst
Copy link

@RobertWHurst RobertWHurst commented Oct 21, 2022

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.

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("/")));
Copy link
Author

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(":")) {
Copy link
Author

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("")) {
Copy link
Author

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();
Copy link
Author

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("~")) {
Copy link
Author

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("/")));
Copy link
Author

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("/")) {
Copy link
Author

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.

@stale
Copy link

stale bot commented Nov 18, 2022

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.

@stale stale bot added the stale this issue is not being responsive label Nov 18, 2022
@stale
Copy link

stale bot commented Dec 16, 2022

This issue has been automatically closed due to it being stale for 28 days. Thank you to everyone who contributed.

@stale stale bot closed this Dec 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale this issue is not being responsive
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Resourcepack not working correctly
1 participant