CIT Resewn

CIT Resewn

14M Downloads

Does Not Support Sprite Path Suffixes

PepperCode1 opened this issue ยท 7 comments

commented

CIT Resewn does not support sprite path suffixes. A "sprite path suffix" is a string that is appended onto a sprite path (the path of an Identifier of a Sprite). In Optifine, this is used for normal maps (_n), specular maps (_s), and emissive textures (_e usually, but pack configurable). For example, if the emissive sprite path needs to be retrieved from the sprite path item/campfire, the _e suffix would be appended, creating item/campfire_e. The identifier namespace would be reused and a new Identifier would be created. There are only two mods that currently do such transformations: Iris (normal and specular) and Continuity (emissives).

The issue is that CIT Resewn uses a subclass of Identifier to determine how a sprite path is resolved to an absolute path. If the base identifier was a ResewnTextureIdentifier, Iris and Continuity do not know of its existence and create a new regular Identifier anyway. CIT Resewn is not able to resolve it correctly meaning that no suffixed textures are loaded.

Adding a hard dependency on CIT Resewn to construct the subclass instead of the base class is not preferable.

commented

Adding a hard dependency on CIT Resewn to construct the subclass instead of the base class is not preferable.

That's understandable. I'll look for a solution once I'm back developing. (Though a fix might need to wait for the asset rewrite because I dont like how these identifiers work now either)

commented

I removed the usage of ResewnTextureIdentifier completely in favor of another approach(for v1.1.1).

Basically, whenever there is a need for an absolute path, the .png is added before it was being changed to a ResewnTextureIdentifier. And since SpriteAtlasTexture#getTexturePath is the one responsible for adding the .png with normal textures, it is safe to assume that normal textures that enter it will not end with a .png. Now the paths that go through it with a .png suffix will be assumed to be absolute paths that don't need the textures/ prefix.
See here: 9ff75d0#diff-ee145790ad7f2432880252f0714622ee80dc9cc206916c762ab1b6a4256f9955L13-L17

You should not need to add a dependency on CIT Resewn now to work around this. Please see if you can make that work and let me know if the issue could be closed.

commented

A tester has confirmed that this change does not solve the issue. Sprite paths are not meant to have extensions since this is how it works in vanilla.

Let us take a look at the example with item/campfire again. If this is a CITR sprite path, it will be item/campfire.png with the new changes. Since Continuity expects the vanilla assumption of no extensions to be upheld, it creates item/campfire.png_e, which is both invalid and not recognized by CITR's check.

It is possible for Continuity to check for extensions in the sprite path, but since CITR is the only mod that adds extensions, I consider this change in Continuity to be special casing. Is there another solution that can be achieved without special casing code on Continuity's side?

commented

yeah I can confirm that emmisive CITs still do not work with v1.1.1

commented

It was not meant to fix the issue, it just allows fixing it without adding a dependency to CIT Resewn.

Currently there is no clean way(afaik) to add a way to add absolute resourcepack paths without this workaround which the cit format needs.

I will not personally look further into this issue until I rewrite and clean up the item type, but even then I don't see a way around the issue for absolute paths.

commented

I still think that CITR should not break the vanilla assumption that sprite identifiers do not have extensions.

I completely agree. This is one of the main reasons I wanted to rewrite the item type. Thank you for providing the hotfix I intended with 9ff75d0.

My plan when I return to work on CIT Resewn currently was to modify the identifiers that need absolute paths and move their textures to identifiers that conform to the proper texture location standards. Essentially "porting" incompatible packs into modern packs on the fly. Then I plan on adding explicit support for modifying the identifiers of suffixed textures as well to move them next to the "fixed" identifiers in the ported pack.

This should completely get rid of incompatibilities between old CIT packs and modern mods seeking support for features.
The intention as of right now is to backport this fix and the item type rewrite in general to 1.19 and 1.18. (ending 1.18 updates with that rewrite)

I am, however, currently working on a separate mod so the item type rewrite is going to take a bit longer than anticipated, apologies for that ๐Ÿ˜…

commented

I still think that CITR should not break the vanilla assumption that sprite identifiers do not have extensions. Continuity is able to load sprites from directories other than textures/ and avoid breaking this assumption. However, I understand that reworking a large portion of CITR's code will take a lot of effort and the changes may not be backported to 1.18.2 and 1.19.2, so I fixed the issue in PepperCode1/Continuity@3fe7a74 by supporting sprite identifiers with the .png extension if CITR Defaults is installed.