Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Per-item (not per-itemstack) metadata #14607

Closed
grorp opened this issue May 2, 2024 · 5 comments
Closed

Per-item (not per-itemstack) metadata #14607

grorp opened this issue May 2, 2024 · 5 comments
Labels
Feature request Issues that request the addition or enhancement of a feature @ Server / Client / Env.

Comments

@grorp
Copy link
Member

grorp commented May 2, 2024

Problem

In my "GGraffiti" mod, I want spray-painted nodes to simply keep their graffiti when they are dug and placed again. In the inventory, spray-painted nodes should still stack normally. You could have a stack of 99 Stone, and some of the nodes in that stack could have graffiti on them and some not.

Because inventory items with metadata don't stack, this is currently impossible.

See grorp/ggraffiti#10.

Solutions

A way to set metadata per-item instead of per-itemstack, something like ItemStack:get_meta(item_index) -> ItemMetaRef. When stacking or moving items with per-item metadata, they would keep their metadata. The item indices would change as appropriate.

There's probably a lot of special cases to be found here. The only way to figure those out is to implement the thing (and also unittests ;)

Alternatives

I don't currently know any other way to implement the desired behavior in my mod.

Additional context

No response

@grorp grorp added @ Server / Client / Env. Feature request Issues that request the addition or enhancement of a feature Supported by core dev Not on the roadmap, yet some core dev decided to take care of this PR and removed Supported by core dev Not on the roadmap, yet some core dev decided to take care of this PR labels May 2, 2024
@cx384
Copy link
Contributor

cx384 commented May 3, 2024

The none-stacking constraint of metadata items is a real pity, but I don't like the solutions. It causes way too many problems and is still very restrictive.
For example, you can't use it to implements stack size dependent item textures (another game has this feature), or fractional stack numbers (using count_meta).

Proper stacking callbacks would be way better. Very briefly functions like can_stack, on_stack, on_destack, on_splitstack, ...
and for instance, inv/stack:add_item() would call can_stack before the item gets added to the inventory/stack.

For the metadata problem, you could then just serialize the meta for every time in the stack and implement the behavior by yourself.

@grorp
Copy link
Member Author

grorp commented May 3, 2024

I realize that my solution proposal is not optimal because it only covers a narrow use case.

Callbacks for manually controlling stacking behavior would be a solution if there was SSCSM. Right now, I think they would break client prediction, resulting in a much worse inventory experience because any inventory action would have to wait for a network roundtrip.

It causes way too many problems

?

@cx384
Copy link
Contributor

cx384 commented May 4, 2024

Callbacks for manually controlling stacking behavior would be a solution if there was SSCSM. Right now, I think they would break client prediction, resulting in a much worse inventory experience because any inventory action would have to wait for a network roundtrip.

Of course, client side predictions would be nice to increase the responsiveness, but I think you can live without them, since the current inventory item movement prediction is also more or less not-existing. (There is only some for player inventories if I'm not wrong.)
Also, a stacking action happens at the same time as item movement. For the prediction, a similar problem currently exists for item movement, when you use count_meta and only update it after the item has been moved.

?

Here are a few problems for you:

  • Which meta should be used for the behavior of the stack? The top one? (E.g. for the description field.)
  • Do I lose information of the stack when a deprecated mod only saves the result of get_meta()?
  • How are the items in the stack ordered? LIFO? How to consistently combine stacks and what is the leftover? Same for splitting stacks. How does reordering work?
  • With the current inventory actions, it's very inconvenient for players to take a specific item from the stack.
  • What happens with set_count()?
  • Various other compatibility problems
  • ...

I don't say that stacking callbacks wouldn't cause similar problems, but at least mods/games could handle this themselves and don't have to deal with a forced metadata stacking feature.

@Montandalar
Copy link
Contributor

Montandalar commented May 5, 2024

-1.

Here is what I would do:

  • Serialize an array of graffitied nodes as a table of metadata tables.
  • Override the item-on-ground entity to check for a graffitied node on the ground and add it to the stack's metadata by adding to the metadata list instead of running the normal code that adds it to the inventory.
  • Register a global minetest.register_ondignode to check for a graffitied node and add it to the existing stack just like pick the item entity up would.
  • Register a global minetest.register_onplacenode to check for a graffitied stack and place the next item in the table of metadatas (it's your choice of FIFO vs LIFO).
  • set_count should work fine if you include a check for an empty queue of graffiti or throw out entries of graffitied nodes if there are more queue entries than available nodes. set_count is rare anyway.

There are other alternatives too like have a graffiti bag with its own inventory which fits in one slot of the main inventory but has many of its own slots specifically for graffitied nodes; A global on_dignode would place into that bag as a priority.

These issues have naturally opinionated answers and I don't think it should be up to the engine to support your specific opinions.

@grorp
Copy link
Member Author

grorp commented May 5, 2024

Closing this issue. cx384's alternative suggestion might be feasible, I'd have to implement it to find out. Not changing anything regarding client-side prediction, i.e. always predicting the default behavior, might be just good enough.

Montandalar's suggestion with a queue per itemstack would break in many situations, for example when splitting or joining itemstacks.

@grorp grorp closed this as not planned Won't fix, can't repro, duplicate, stale May 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature request Issues that request the addition or enhancement of a feature @ Server / Client / Env.
Projects
None yet
Development

No branches or pull requests

3 participants