Draw node animation for items #15930

cx384 · 2025-03-22T14:45:46Z

This is a step towards Animated item textures in the inventory #4758
It uses the existing node animation system, and applies it when drawing node items.
~~Only active if the inventory_items_animations setting is on.~~

~~(It does not work for the wield mesh without flickering)~~ solved

Click me (Videos)

mtg.mp4

dev_animated.mp4

To do

Ready for Review.

Maybe some refactoring.

How to test

~~Enable the inventory_items_animations setting.~~
Start devtest and get some Animated Test Nodes.

src/client/wieldmesh.h

DragonWrangler1 · 2025-03-22T18:23:06Z

So glad to see this finally getting implemented (even if only in part)

DragonWrangler1 · 2025-03-24T19:45:13Z

This is a step towards Animated item textures in the inventory #4758

It uses the existing node animation system, and applies it when drawing node items.

Only active if the inventory_items_animations setting is on.

(It does not work for the wield mesh. I tried to implement it, but using the material type of the WieldMeshSceneNode caused some flickering, with other material types it looks fine, but the lighting does not update. So I leave this to someone else, since I'm not very familiar with the code, it probably involves some shader programming.)

Click me (Videos)

To do

Ready for Review.

Maybe some refactoring.

How to test

Enable the inventory_items_animations setting. Start devtest and get some Animated Test Nodes.

Disclaimer: I haven’t tested yet.

I see it’ behind the animated inventory items setting. Which also has the rotating items. Could this be changed to perhaps a new property that can be set per block that allows it to have the animation or not ?

cx384 · 2025-03-24T21:01:40Z

Could this be changed to perhaps a new property that can be set per block that allows it to have the animation or not ?

This will most likely be doable using (animated) mesh inventory images in the future.
(Currently, you can only override it by a flat (non-animated) inventory_image.)

I don't know about the rotating items. You should probably fill out a feature request issue in this regard.

DragonWrangler1 · 2025-03-24T21:16:45Z

Could this be changed to perhaps a new property that can be set per block that allows it to have the animation or not ?

I don't know about the rotating items. You should probably fill out a feature request issue in this regard.

No i'm saying that you put it behind the same setting that has the item rotation.

appgurueu · 2025-03-25T19:48:13Z

I think making this node-def-controlled is a good idea. Game developers should have fine-granular control over this; I don't see a priori why players should have much control here.

I agree that "reusing" the inventory_items_animations setting doesn't seem like the best idea for the reason DragonWrangler has pointed out already (it puts two somewhat different things, the spinning items and texture animations, behind the same setting).

cx384 · 2025-03-25T22:29:48Z

I think making this node-def-controlled is a good idea. Game developers should have fine-granular control over this; I don't see a priori why players should have much control here.

(I assume by "this" you mean the texture animation and not the rotation.)
As I already wrote, this will most likely be possible in the future by specifying a "mesh inventory image" in the item/node definition to override the mesh and (animated) textures of the inventory image (also see #3551).
Having a separate flag in the node definition which toggles whether the animated tiles of a node are also animated when it gets drawn in the inventory does not make much sense to me, if you can just replace the whole inventory image (mesh and its tiles). But item definitions also contain a color field, even though you can colorize the inventory image by using texture modifiers.
So, if you insist, I will add an animated_in_inventory flag to the node (not item) definition table. (I suspect you want this for the whole node and not for each tile separately as part of the tile animation definition.)
Would you prefer the flag to be true or false by default?

Allowing players to disable animations is beneficial for accessibility. People with vestibular disorder, epilepsy and similar may suffer from animated images, if I'm not wrong. So I guess it is good to have a global setting for this

I agree that "reusing" the inventory_items_animations setting doesn't seem like the best idea for the reason DragonWrangler has pointed out already (it puts two somewhat different things, the spinning items and texture animations, behind the same setting).

What do you suggest? Should it just ignore the inventory_items_animations setting, or should I add an additional disable_item_inventory_image_animations setting?
Personally, I don't care. I thought the inventory_items_animations setting is meant to enable any animation of items in inventories. If this is not the case, maybe it should be renamed to inventory_selected_item_rotation_animation, and maybe we want a rotate_on_selected flag in the item definition. But for those things, someone should open a feature request issue.

I thought it could even be considered a bug that animated nodes are not drawn animated in inventories, but if there is so much to discuss I should have opened an issue first. Note that this PR is closely related to #4758, but it does not solve it.

src/client/wieldmesh.cpp

sfan5 · 2025-03-26T12:12:55Z

src/client/wieldmesh.cpp

@@ -404,7 +411,7 @@ void WieldMeshSceneNode::setItem(const ItemStack &item, Client *client, bool che
 			setExtruded(tsrc->getTextureName(l0.texture_id),
 				"", wield_scale, tsrc,
 				l0.animation_frame_count);


note that setExtruded cuts the texture down to its first animation frame, so there's no point remembering animation info in this case.

Well, setExtruded is only used for the wield mesh anyway, where the animation does not take effect (yet).
So I made it only remember the color there, like it was before, but for other draw types, both inventory items and the wield mesh uses createGenericNodeMesh which remembers the animation.

Removed, animation for extruded meshes will be added in a follow up PR.

src/client/wieldmesh.h

src/gui/drawItemStack.cpp

cx384 · 2025-03-27T16:24:20Z

Thanks for the review.
I hope I fixed everything.

cx384 · 2025-03-28T00:34:27Z

I finally solved the wield mesh flickering problem by updating the lighting and animation at the same time instead of doing it separately.
So wield item animation is now supported, too.

cx384 · 2025-03-28T15:49:06Z

I moved AnimationInfo somewhere else to avoid code duplication, and none of the extruded meshes stores the animation information anymore.

I suppose it would be too much for this PR if I change large parts of the code to make it work for drawtypes that use a flat inventory image that has no tile layer. I will probably fix this in a follow up PR which also adds an API to set the inventory image animation in the item definition table.

cx384 · 2025-03-28T15:52:10Z

src/client/mapblock_mesh.cpp

@@ -763,6 +761,12 @@ bool MapBlockMesh::animate(bool faraway, float time, int crack,
 	// Cracks
 	if (crack != m_last_crack) {
 		for (auto &crack_material : m_crack_materials) {
+
+			// TODO crack on animated tiles does not work


I guess it never worked, so I just removed it. (because of the AnimationInfo change)
See #5317

SmallJoker · 2025-03-30T09:39:10Z

I like this feature but would prefer to have it only apply to the hovered/selected ItemStack like the existing rotation animation.
(Answer based on the videos; didn't test the PR yet)

DragonWrangler1 · 2025-03-30T12:39:20Z

I like this feature but would prefer to have it only apply to the hovered/selected ItemStack like the existing rotation animation. (Answer based on the videos; didn't test the PR yet)

I can’t say I’m for that. To help with game aesthetics I believe it would be better to define this per item.

cx384 · 2025-03-30T14:31:41Z

I prefer that by default nodes placed in the world and in inventories look the same, including the animation.
Such that you have to set the inventory image (including animation and mesh in the future) in the definition if you want to change it.

In my opinion, it would be better to turn the hovered/selected appearance into another feature, like wield_image, if we want such a feature.
And make it default to the inventory_image or in case it is empty, to the node image (including the animation).

Drawn node animation for items

4ee5725

cx384 commented Mar 22, 2025

View reviewed changes

src/client/wieldmesh.h Outdated Show resolved Hide resolved

Zughy added @ Client / Audiovisuals Feature ✨ PRs that add or enhance a feature Roadmap: supported by core dev PR not adhering to the roadmap, yet some core dev decided to take care of it labels Mar 22, 2025

sfan5 changed the title ~~Drawn node animation for items~~ Draw node animation for items Mar 23, 2025

sfan5 reviewed Mar 26, 2025

View reviewed changes

cx384 added 5 commits March 27, 2025 16:08

Add braces

bbcdfa8

Pair to struct

5b2b29a

Ignore inventory_items_animations setting

1315404

Don't remember animation for the WieldMesh

4fb3fde

Fix comment

316db3e

Add wield item animation

d214c72

cx384 added 2 commits March 28, 2025 16:27

Refactor AnimationInfo struct

7f8fa45

Don't remember animation for extruded mesh

f9a6c89

cx384 commented Mar 28, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Draw node animation for items #15930

Draw node animation for items #15930

cx384 commented Mar 22, 2025 •

edited

Loading

DragonWrangler1 commented Mar 22, 2025 •

edited by appgurueu

Loading

DragonWrangler1 commented Mar 24, 2025

To do

How to test

cx384 commented Mar 24, 2025

DragonWrangler1 commented Mar 24, 2025 •

edited

Loading

appgurueu commented Mar 25, 2025

cx384 commented Mar 25, 2025

sfan5 Mar 26, 2025

cx384 Mar 27, 2025

cx384 Mar 28, 2025

cx384 commented Mar 27, 2025

cx384 commented Mar 28, 2025 •

edited

Loading

cx384 commented Mar 28, 2025 •

edited

Loading

cx384 Mar 28, 2025 •

edited

Loading

SmallJoker commented Mar 30, 2025 •

edited

Loading

DragonWrangler1 commented Mar 30, 2025 •

edited

Loading

cx384 commented Mar 30, 2025

Draw node animation for items #15930

Are you sure you want to change the base?

Draw node animation for items #15930

Conversation

cx384 commented Mar 22, 2025 • edited Loading

To do

How to test

DragonWrangler1 commented Mar 22, 2025 • edited by appgurueu Loading

DragonWrangler1 commented Mar 24, 2025

To do

How to test

cx384 commented Mar 24, 2025

DragonWrangler1 commented Mar 24, 2025 • edited Loading

appgurueu commented Mar 25, 2025

cx384 commented Mar 25, 2025

sfan5 Mar 26, 2025

Choose a reason for hiding this comment

cx384 Mar 27, 2025

Choose a reason for hiding this comment

cx384 Mar 28, 2025

Choose a reason for hiding this comment

cx384 commented Mar 27, 2025

cx384 commented Mar 28, 2025 • edited Loading

cx384 commented Mar 28, 2025 • edited Loading

cx384 Mar 28, 2025 • edited Loading

Choose a reason for hiding this comment

SmallJoker commented Mar 30, 2025 • edited Loading

DragonWrangler1 commented Mar 30, 2025 • edited Loading

cx384 commented Mar 30, 2025

cx384 commented Mar 22, 2025 •

edited

Loading

DragonWrangler1 commented Mar 22, 2025 •

edited by appgurueu

Loading

DragonWrangler1 commented Mar 24, 2025 •

edited

Loading

cx384 commented Mar 28, 2025 •

edited

Loading

cx384 commented Mar 28, 2025 •

edited

Loading

cx384 Mar 28, 2025 •

edited

Loading

SmallJoker commented Mar 30, 2025 •

edited

Loading

DragonWrangler1 commented Mar 30, 2025 •

edited

Loading