Add ability to override item images using meta

[rubenwardy]

Fixes #5686

This allows setting the inventory image and wield image of an item stack using meta:

meta:set_string("inventory_image", "default_apple.png")
meta:set_string("wield_image", "basetools_icesword.png^testitems_overridden.png")

To do

This PR is Ready for review

• Caching / memory leaks
• Fix threaded clientcached dropping meta

How to test

See testitems:image_meta in devtest

[sfan5]

pinging @velartrill as I think she was also interested in doing this

[velartrill]

hey awesome! thanks for taking this on, lack of this functionality has been a pain point for me for a while. but there's a significant gotcha with this PR's approach as i pointed out on the old PR:

it'd take a bit more work but i'd suggest changing the format of the texture_string property to something like <state>:<texspec>, where <state> is a short label describing the change made to the inventory image, and then <texspec> would hold the actual texture spec itself. then override.txt could be altered to allow lines like mymod:bow inventory:arrow_loaded my_drawn_bow.png to override a specific state of the item (this is necessary because it might not be possible to achieve the texturepack author's desired effects simply by changing the textures used to compose the dynamic texture)

i'd be inclined to make the state field mandatory because most modders probably won't use it otherwise, but we could also just allow a :<texspec> format specifier if the author doesn't want texture packs overriding the image, or if the logic involved is too complex for discrete overrides to be useful

e.g. one texture pack might assemble icons for various items and their states programmatically by combining a small number of textures at runtime, while a texture pack author might want to provide higher-quality hand-drawn variants for those states. or vice-versa

additionally, maybe this part is out of scope and deserves its own PR, but (before you beat me to making a new PR :p) i was also considering folding in a meta key that allows dynamically changing the textures of nodes in the world when i got around to it, since it's a bit silly for itemstacks to be able to change their appearance in inventory but not in the world. the latter would also be very useful for e.g. signs and would obviate many terrible entity hacks. opinions? is this a desirable feature? is a meta key the right way to go about it? should it be a separate PR?

---

oh, ETA: since these features are closely related, it might be worth revisiting the hotbar API as well if this gets merged (in a separate PR ofc). key item callbacks here would be the obvious on_{,un}wield for when an item in the hotbar is (de)selected (currently this kind of functionality requires abusing globalstep, which implies both performance and responsiveness issues), and possibly on_hotbar_{put,remove} for when an item is newly installed in the hotbar from elsewhere in the inventory -- i think the latter could be implemented in terms of the existing API but it might be worth defining them (even if as callbacks from the game rather than the engine) since currently inventory move callbacks only give you names and positions (rather than semantics), and different games can handle inventories very differently.

(mainly i'm thinking about this in terms of "how can we make it as straightforward as possible to implement minecraft-style bows and compasses")

i also wonder if it might be worthwhile to have an image change prediction property -- consider what happens when you have a fully-"charged" bow and then switch away without firing. even with full engine support for on_unwield and texture metadata overrides, there will still be a brief, confusing lag before the bow image resets to normal. what i'm thinking of is an item property like node_{placement,dig}_prediction which offers information to clients so they can locally give quicker feedback. obviously (as usual) the ideal way to go about this for bow-like simple UI feedback would be SSCSM (since there's no good reason that should be done on the server at all), but until SSCSM becomes an actual thing a "reset prediction" property could be a decent bandaid. i'm mostly thinking of the (probably pretty common) case where on_unwield or possibly some other event causes the inventory_ or wield_image keys to be nulled so the itemstack reverts to its default, hardcoded texture spec. so maybe something vaguely like

{
    description = "Bow";
    texture_prediction = {
        on_unwield = {
            mode = "reset"; 
            inventory_image = true;
            wield_image = true;
       };
    };
}

though that could obviously be simplified depending on how fancy you want texture predictions functionality to be

(cf. #9862)

---

anyway, these are really just nitpicks, and i don't mean to detract from this PR! apart from the item texture state spec thing these suggestions all could be progressively implemented in consequent PRs; i think even the PR as is is very much worth merging since it gives us a totally new capability and my suggestions are basically just nice-to-have refinements of that capability. :)

[rubenwardy]

e.g. one texture pack might assemble icons for various items and their states programmatically by combining a small number of textures at runtime, while a texture pack author might want to provide higher-quality hand-drawn variants for those states. or vice-versa

Hmm, overrides.txt compat is an interesting point, not something I considered. I'm not sure support for this is needed in the first version though. Maybe for forwards compat we could ignore stuff before some character like :

additionally, maybe this part is out of scope and deserves its own PR, but (before you beat me to making a new PR :p) i was also considering folding in a meta key that allows dynamically changing the textures of nodes in the world when i got around to it, since it's a bit silly for itemstacks to be able to change their appearance in inventory but not in the world. the latter would also be very useful for e.g. signs and would obviate many terrible entity hacks. opinions? is this a desirable feature? is a meta key the right way to go about it? should it be a separate PR?

This is definitely out of scope

[velartrill]

I'm not sure support for this is needed in the first version though

my concern about that is that if this isn't in there from the start, vanishingly few modders will define state names and texture pack designers will be (further) crippled by this new feature. i suspect modders probably don't often think about how to make their mods easily and fully "skinnable" (and when i do, i often find myself forced to choose between skinability and reasonable design -- it seems like not a lot of thought has hitherto gone into this on the engine side either :/ )

[rubenwardy]

Hi, please can we keep the discussion relevant to this PR thanks

[Desour]

Tested, works fine.

[rubenwardy]

@Desour please can you retest? Found and fixed a bug with inventory animations

[Desour]

You could here just pass item name, inventory_image and inventory_overlay (as opposed to item), to avoid similar cache_key issues in future changes to getItemMesh.

[Desour]

Oops, looks like I didn't test well enough. Should be fine now though. Approval holds.

[epCode]

Can this possibly be done without the animation of item switching happens?
What I mean by that is can it be done easily.

[Desour]

Can this possibly be done without the animation of item switching happens?
What I mean by that is can it be done easily.

That's a different issue.
The items switch animation happens whenever the wielded item changes, AFAIK. And an item with different meta is a different item.

[epCode]

That's a different issue.
The items switch animation happens whenever the wielded item changes, AFAIK. And an item with different meta is a different item.

Ah, I see. That is a problem. Is that deeply integrated?

[Desour]

That's a different issue.
The items switch animation happens whenever the wielded item changes, AFAIK. And an item with different meta is a different item.

Ah, I see. That is a problem. Is that deeply integrated?

I don't know. But I guess it's not too hard to change.