Inventory image animation API second try #16538

cx384 · 2025-10-04T12:37:56Z

Revision of #15979 (Can't reopen the old PR because of rebase force push.) Closes #4758

All issues from the previous version should be resolved.
~~There are now 4 different animation definitions such that the overlay animations can be changed independently.~~
Item image can now be a table
~~Animations can be changed via metadata (Json serialized and properly cached). ✨~~
Removed for this PR

Problems (currently not planned to be resolved with this PR):

The image of visual = "wielditem" and visual = "item" objects (dropped items) is not animated.
As discussed in the old PR, there is no function to push the TileAnimationParams back to Lua.

To do

Ready for Review.

How to test

Start devtest and use
testitems:inventory_image_animation
testitems:inventory_image_animation_overlay
testitems:wield_image_animation
testitems:wield_image_animation_overlay
testitems:inventory_image_animation_meta (new, for metadata)
Test compatibility with older versions, which only use the first frame.

doc/lua_api.md

cx384 · 2025-10-09T09:29:43Z

Rebased
Removed meta animation override
inventory_image can now be a table containing the animation "for the greater good"
Similar to tiledef, but a different specification because e.g. texture can't depend on in world coordinates.
Could be extended to support other things like mesh item images in the future.

cx384 · 2025-10-12T06:13:37Z

Another big rebase, next time I will squash the commits to reduce rebase work.

src/itemdef.cpp

src/network/networkprotocol.cpp

src/util/string.h

SmallJoker

I'd like to propose a few changes:
0001-Use-AnimationInfo-for-simplicity.patch.tar.gz

Excerpt from the commit message:

Moved 'ItemVisuals' to the source file (internal for 'ItemVisualsManager')
Re-used 'AnimationInfo' with one tile: This makes it possible to use
one class/interface to contain as many textures as wanted. (generic)
Replaced 'OwnedAnimationInfo' with 'AnimationInfo' + 'freeFrames()' to
(hopefully) reduce complexity.

cx384 · 2025-10-18T18:16:48Z

Thanks for the improvements.
Using AnimationInfo for all item images, including non-animated ones, certainly reduces complexity, and it probably doesn't matter memory/performance-wise.
I don't know if the TODO. Change this to a shared pointer. for the frames vector is a good idea, though.

SmallJoker

LGTM aside from that. I find the animations very distracting, but it works.

src/script/common/c_content.cpp

SmallJoker · 2025-10-19T16:57:19Z

src/network/networkprotocol.cpp


 // Note: Also update core.protocol_versions in builtin when bumping
-const u16 LATEST_PROTOCOL_VERSION = 50;
+const u16 LATEST_PROTOCOL_VERSION = 51;


Design question to the dev team: Should we bump already, or in the 5.15.0-rc build?

Typically people bump the protocol version because a network change requires it.
If this isn't the case here we could delay the bumping, but I don't see a strong reason to.

I am mainly asking this in case there would be a second PR for 5.15.0 that also increases the version. I guess it does not matter. We could bump it multiple times per version.

src/itemdef.h

Only send first frame texture names to old clients Replace image string by item image definition Bump protocol version 1. Moved 'ItemVisuals' to the source file (internal for 'ItemVisualsManager') 2. Re-used 'AnimationInfo' with one tile: This makes it possible to use one class/interface to contain as many textures as wanted. (generic) 3. Replaced 'OwnedAnimationInfo' with 'AnimationInfo' + 'freeFrames()' to (hopefully) reduce complexity. Co-authored-by: SmallJoker <[email protected]>

cx384 · 2025-10-24T08:23:06Z

Rebased (and squashed beforehand).

sfan5 · 2025-10-27T11:06:46Z

src/client/item_visuals_manager.cpp

+
+	~ItemVisuals()
+	{
+		inventory_normal.freeFrames();


wouldn't it be cleaner if ItemVisuals owned the frames ptr separately, so that AnimationInfo is always just a reference?

This was the case before at some point, but with Krocks's suggestions it changed "to (hopefully) reduce complexity".
Though, it probably would be best to directly store it in ItemVisuals instead of allocating additional heap memory.

sfan5 · 2025-10-27T11:41:55Z

doc/lua_api.md

      -- 'chunksize' mapgen setting can be a vector, instead of a single number (5.15.0)
      chunksize_vector = true,
+      -- Item definition fields `inventory_image`, `inventory_overlay`, `wield_image`
+      -- and `wield_overlay` accept a table containing animation definitions. (5.16.0)


sfan5 · 2025-10-27T11:43:34Z

src/client/item_visuals_manager.cpp

+		if (image.name.empty()) {
+			// no-op
+		} else if (image.animation.type == TileAnimationType::TAT_NONE) {
+			frames->push_back({0, tsrc->getTexture(image.name)});


couldn't createAnimationFrames just handle the one-frame case?

src/client/item_visuals_manager.h

sfan5 · 2025-10-27T11:45:47Z

src/client/item_visuals_manager.h

+	AnimationInfo *getInventoryOverlayAnimation(const ItemStack &item, Client *client) const;
+
+	// Get item mesh
 	// Once said to return nullptr if there is an inventory image, but this is wrong


line should be removed.

sfan5 · 2025-10-27T11:47:58Z

src/client/wieldmesh.cpp


+std::vector<FrameSpec> createAnimationFrames(ITextureSource *tsrc,
+		const std::string &image_name, const TileAnimationParams &animation,
+		int &result_frame_length_ms)


I would move this to another file, but not sure where

sfan5 · 2025-10-27T11:50:54Z

src/client/wieldmesh.cpp

-void getItemMesh(Client *client, const ItemStack &item, ItemMesh *result)
+void createItemMesh(Client *client, const ItemDefinition &def,
+		AnimationInfo &animation_normal,
+		AnimationInfo &animation_overlay,


sfan5 · 2025-10-27T11:58:32Z

src/client/wieldmesh.cpp

-				extractTexture(f.tiledef_special[0], l0, tsrc), nullptr
-			);
-			result->buffer_info.emplace_back(0, l0.has_color, l0.color);
+			mesh = getExtrudedMesh(l0.texture);


no, the extractTexture call is important and was needed for preparation for array textures.
(in the future) l0.texture may be an array texture, which isn't usable for the item mesh.

src/itemdef.h

sfan5 · 2025-10-27T12:05:59Z

src/tileanimation.h

+
+	// Modifies the texture name such that it only contains the first frame
+	// If the texture_size is know (client code), getTextureModifer should be used instead
+	void extractFirstFrame(std::string &name) const;


I would put a disclaimer here that this function is only for compatibility purposes with old clients.

cx384 added @ Script API @ Client / Audiovisuals Feature ✨ PRs that add or enhance a feature Concept approved Approved by a core dev: PRs welcomed! labels Oct 4, 2025

Zughy added Roadmap: supported by core dev PR not adhering to the roadmap, yet some core dev decided to take care of it and removed Concept approved Approved by a core dev: PRs welcomed! labels Oct 4, 2025

SmallJoker reviewed Oct 6, 2025

View reviewed changes

doc/lua_api.md Outdated Show resolved Hide resolved

SmallJoker reviewed Oct 6, 2025

View reviewed changes

doc/lua_api.md Outdated Show resolved Hide resolved

sfan5 added the Rebase needed The PR needs to be rebased by its author label Oct 7, 2025

cx384 force-pushed the item_animation_api branch from f06fffe to ff95981 Compare October 9, 2025 09:23

cx384 removed the Rebase needed The PR needs to be rebased by its author label Oct 9, 2025

SmallJoker self-requested a review October 11, 2025 14:03

cx384 force-pushed the item_animation_api branch from d75ed95 to 06a265a Compare October 12, 2025 06:13

SmallJoker reviewed Oct 12, 2025

View reviewed changes

src/itemdef.cpp Outdated Show resolved Hide resolved

SmallJoker reviewed Oct 12, 2025

View reviewed changes

src/network/networkprotocol.cpp Outdated Show resolved Hide resolved

SmallJoker reviewed Oct 12, 2025

View reviewed changes

src/util/string.h Show resolved Hide resolved

SmallJoker reviewed Oct 18, 2025

View reviewed changes

SmallJoker reviewed Oct 19, 2025

View reviewed changes

SmallJoker approved these changes Oct 19, 2025

View reviewed changes

SmallJoker added the One approval ✅ ◻️ label Oct 19, 2025

cx384 force-pushed the item_animation_api branch from b2958d1 to 298a6c5 Compare October 24, 2025 08:20

cx384 mentioned this pull request Oct 24, 2025

Add meta item image override functions #16608

Open

cx384 merged commit 93ccb4b into luanti-org:master Oct 26, 2025
20 checks passed

sfan5 reviewed Oct 27, 2025

View reviewed changes

cx384 added a commit to cx384/minetest that referenced this pull request Oct 27, 2025

Fixes for luanti-org#16538

1b8086f

cx384 mentioned this pull request Oct 27, 2025

Fixes and improvements for item image animations #16620

Merged

Inventory image animation API second try #16538

Inventory image animation API second try #16538

Uh oh!

Conversation

cx384 commented Oct 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problems (currently not planned to be resolved with this PR):

To do

How to test

Uh oh!

Uh oh!

Uh oh!

cx384 commented Oct 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cx384 commented Oct 12, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

SmallJoker left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cx384 commented Oct 18, 2025

Uh oh!

SmallJoker left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cx384 commented Oct 24, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sfan5 Oct 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

cx384 commented Oct 4, 2025 •

edited

Loading

cx384 commented Oct 9, 2025 •

edited

Loading

SmallJoker left a comment •

edited

Loading

sfan5 Oct 27, 2025 •

edited

Loading