EnderTurret / PatchedMod

A Minecraft mod that adds the ability for data/resource packs and mods to patch JSON files, allowing them to avoid the incompatibility of replacing files wholesale.
GNU Lesser General Public License v2.1
2 stars 0 forks source link

Allow patch the tags like resources #20

Closed SettingDust closed 3 weeks ago

SettingDust commented 1 month ago

https://linkie.shedaniel.dev/mappings?namespace=mojang&version=1.21.1&search=getResourceStack&translateMode=ns&translateAs=yarn

The resources like tags are loaded by the method above, which patched ignored.

EnderTurret commented 1 month ago

Yes, Patched does not out-of-the-box allow patching tags, lang files, sound jsons et. al., since the context there is even more lack thereof than with the other methods.

The problem is that tags and friends are obtained from each PackResources and then merged together, meaning there's potentially hundreds of versions of this file floating around the filesystem -- so which one do we patch? If it's a specific one, then you now need an entire subsystem for defining pack-specific patch targets. Otherwise, if it's the final result, you're going to need to turn the merged tag back into JSON just to turn it into a tag again immediately after applying one (1) patch. As a bonus, this removes your ability to rely on indices in patches, since the location of a given tag entry could be anywhere -- you'd have to use find + remove to remove entries.

This problem is why I haven't tried tackling this issue yet, combined with the fact that many mod loaders do offer ways to do most of the things you'd want patches for in tags -- adding optional entries (this is supported by vanilla actually) and removing specific entries. The only thing these don't cover I think is removing arbitrary entries based on a condition, like removing all of Pam's Harvestcraft's sandwiches from a given tag.

That is, assuming Fabric/Quilt have builtin ways to remove tag entries. If they don't, then someone will have to either take it up with them or write a mod to add it.

I think my question is, do you have a specific goal here, or are you suggesting this because Patched doesn't support it?

SettingDust commented 1 month ago

Patched supporting remove with condition is the reason. I think it's possible to patch all the resource.

EnderTurret commented 1 month ago

I've thought of a way I could allow patching these files, although it has its own problems. Specifically, I could allow people to "self-patch" their tags/lang files/whatever, which would open up conditional removal et al., but only for their own files -- there's simply not enough context without inventing an entire unreliable subsystem to allow patching the merged version.

To clarify, the idea is that you'd have the following:

data
└ mymod
  └ tags
    └ item
      ├ functional.json
      └ functional.json.patch

Let's say functional.json contains this:

{
  "replace": false,
  "values": [
    "mymod:functional_item",
    "minecraft:stick"
  ]
}

This feature could allow writing this patch:

[
  // Check for whether the stick is disabled in mymod's config.
  {
    "op": "test",
    "type": "mymod:config_flag",
    "value": "disable_stick_use"
  },
  // Remove the 'minecraft:stick' entry if so.
  {
    "op": "remove",
    "path": "/values/1"
  }
]

However, it would not allow one to patch others' tag contributions. For example, you couldn't use patches to banish someone's multi-tool from the c:pickaxes tag, unless you're the one adding it to begin with.

The main problem that comes with this implementation is the potential to be mislead -- someone might see such a patch and assume it applies to the merged version, when in reality it only applies to the pack's own file. However, this likely won't be a huge problem in practice.

Regardless, this would address configurable tag changes, assuming that's what people want from the ability to patch tags. Anything more advanced would either require virtual packs or manually-enabled ones.

The alternative (which I don't want to have to maintain) is to inject into whatever loads each tag, turn the tag into JSON, run patches over it, and then turn it back into a Set or whatever. I expect this would be pretty slow, especially since Patched doesn't know if there's any (tag-targeting) patches ahead of time.

Would this solution (that is, allowing self-patches) work for you?

SettingDust commented 1 month ago

For clarification, what is the missing context for not being able to patch tags?

The alternative (which I don't want to have to maintain) is to inject into whatever loads each tag, turn the tag into JSON, run patches over it, and then turn it back into a Set or whatever. I expect this would be pretty slow, especially since Patched doesn't know if there's any (tag-targeting) patches ahead of time.

Based on my experience in previous project, it's able to patch the stream directly like how patched doing now. https://github.com/SettingDust/HoconResourceLoader/blob/main/src/main/java/settingdust/hoconresourceloader/mixin/ResourceFinderMixin.java#L16-L32 Record the current FileToIdConverter that is holding prefix and extensions. @Share should work in this situation instead of ThreadLocal

https://github.com/SettingDust/HoconResourceLoader/blob/main/src/main/java/settingdust/hoconresourceloader/mixin/NamespaceResourceManagerMixin.java#L117-L184 findResources is listResources. findAndAdd is listPackResources which is using by listResourceStacks. With the context from FileToIdConverter. It's possible to patch the resources like tags

For language-like resources. Have to mixin into the getResourceStack and patch the stream there. There is the context.

EnderTurret commented 1 month ago

Alright, so after some code analysis it looks like tag patching is already possible, since it's indeed controlled by listResourceStacks() and not getResourceStack() (the method I was thinking of). This is actually a very concerning development, since it enables some extremely janky patches.

Consider, if you will, three packs:

Let's say both pack B and pack C add things to the c:ores/copper item tag, and pack A attempts to patch this tag. Let's also say that the packs are prioritized as listed, such that pack A has the highest priority.

The problem that follows is that pack A's patch will now apply to three files:

For someone who is used to only patching single files using Patched, this is highly surprising behavior. Even worse, common patching operations like add, remove, etc. now apply to all instances of the tag, resulting in complete chaos. A single remove patch could unpredictably remove half the contents of the tag. On the other hand, an add patch could add an entry to every file. If this were a nonexistent entry, you could get 60 MB of log errors blaming every data pack and mod that touches the tag.

To truly illustrate the scale here, let's assume the tag is something else, like minecraft:mineable/pickaxe. Just about every mod that adds a block adds entries to this tag, and consequently has a file to do so. Imagine if pack A patched this tag instead. This means that pack A's patch applies to all 300 or so instances of this tag -- however many are in typical large modpacks. Not only is this pretty inefficient, but now the patch for pack A has to be written perfectly to avoid all potential issues (such as accidentally removing unexpected entries). This means that your only safe operations are test and find -- you cannot safely use add as it will appear in every file.

Now one might say, why not just rely on people writing patches correctly?

Here's an example of how patch writing can go wrong:

  1. Someone decides they want to remove an entry from a vanilla tag, but don't want to bother with (possibly nonexistent) mod-loader-specific extensions
  2. They write a patch targeting the tag, and use the remove operation to do it
  3. They test their new data pack in a mostly vanilla environment, and it seems to work fine
  4. Someone else later downloads this data pack and runs it in a heavily modded environment
  5. The game violently explodes

This is easily something that could happen in the wild -- not everyone will think about the long-term ramifications of their patches, and certainly they won't test in a variety of circumstances. It's really easy to assume your patch only applies to the base file when nothing else is touching it, and nobody would expect that the patch would apply to every such file unless they spent enough time thinking about it -- the documentation never calls this behavior out or warns about it.

Hopefully this all adequately explains why allowing blind patching is a horrendously terrible idea, even if it's technically possible to do so.

I'm going to remove this footgun of epic proportions, but I'll compromise by making it so you can patch your own file. This isn't ideal if you're writing a data pack to conditionally remove entries, since you can't patch the other instances. However, there's a way around this -- just conditionally add your removal to each loader-specific entry removal area. The only problem with this is that (as far as I'm aware) Fabric API doesn't have a tag entry removal API, so some other mod is needed to fill in the gap.

EnderTurret commented 3 weeks ago

This has now been implemented as of 7.3.0+1.21.1 and 3.4.0+1.20.1.