Allow non-tools to use wear to display a bar

[thePalindrome]

Do note that destroying items due to damage only occurs on tools.

This change is compatible, even if you load a "damaged" craftitem on an older version.

EDIT: Addresses #6448

[est31]

What are usecases for this?

I'm not really sure why non-tools should have different behaviour from tools here.

[stujones11]

What are usecases for this?

Armor might be one, since I am currently forced to register armor somewhat incorrectly as tool items just so they can take damage.

[est31]

Yes, but I still don't get why items should not be destroyed when the bar is empty, and tools should.

[stujones11]

Ah, I missed that bit. I would certainly want the armor to be destroyed when the bar is empty.

[SmallJoker]

Space after the two if's, otherwise fine.

[stujones11]

I suppose you might want to use the bar for something other than wear but you don't want the item to disappear just because the level became zero. I do that with my shooter mod for gun ammo so there are already ways around that problem, though I am not saying they are elegant.

[Rogier-5]

I suppose you might want to use the bar for something other than wear but you don't want the item to disappear just because the level became zero.

How about lua callback: on_worn_out() whose default action is to make the item disappear. A non-default action could be to disable the effects of the item/tool (e.g. oil lamp empty: lamp remains, but won't burn. And indeed ammo: weapon remains, but can't fire).

[est31]

@Rogier-5 that idea sounds good, it would enable people to get rid of hacks like this.

E.g. the return value true of on_worn_out would make the item disappear, and false/nil would make it not disappear.

[thePalindrome]

I heartily agree that an on_worn_out callback would be a much better implementation than simply relying on mods to properly handle item destruction (which minetest could not reasonably determine)

My personal opinion on the matter is that this pull request be denied, such that the first implementation is the correct one, but I shall leave that decision to those more responsible.

[stujones11]

I am all in favour of a callback, however, I think that tool items should be given the same option for the sake consistency. The problem would be backwards compatibility, on_wear_out would need to be optional returning nil (not present) when you do want it to disappear (existing behaviour).

[Rogier-5]

Allow non-tools to use wear to display a bar

@thePalindrome you're not explaining why you actually need craftitems to display a bar ? And why you can't use a tool in those cases ? What properties does a tool lack that craftitems-with-bar possess ?

I am currently forced to register armor somewhat incorrectly as tool items just so they can take damage.

@stujones11: it sounds like your main problem is the fact that 'armor is not a tool', i.e. the name 'tool' is not appropriate for armor. If 'tool' were renamed to 'craftitem_with_level`, that would solve your issue ?

BTW: I do think armor is, technically speaking, a tool, albeit one that protects against damage instead of one that does damage.

[Ferk]

Buckets and books could be, technically speaking, considered tools as well, and yet they are craftitems.
It certainly looks like 'tool' is meant to really mean 'craftitem_with_level' and not really "tool".

Also I don't understand why "tool_capabilities" is a tool thing when you actually can use any craftitem you hold for digging or damaging (so long as it doesn't have an on_use), and there are even tools that implement on_use and actually can't dig or damage (like the hoe).

I think the explanation in lua_api.txt doesn't really hold up. It doesn't even mention wear level (apparently the main difference), and can mislead people to think it implies that only tools can dig and damage.

[thePalindrome]

I'd say the difference between a tool and a craftitem_with_bar is that a lot of code that makes sense for tools (damaging when digging for example) doesn't make sense for craftitems_with_bars (in my example, a battery of sorts that could feasibly be empty), if I took a AA battery and somehow dug a hole in my yard, is the battery now containing less energy?

My main reason for this is to avoid any special regards for tools (like a GregTech style mod that makes a lot of tools worthless)

[Ferk]

Shouldn't this 65535 be a #define constant somewhere?

[thePalindrome]

That was from the original code, so I'm not sure the reasoning behind that.

[SmallJoker]

The constant would be called U16_MAX. There can be no higher wear than that, so it might be a good idea to replace this now.

[Rogier-5]

IMHO a better name would be WEAR_MAX. However, if the 65535 is replaced with a constant, even if it's U16_MAX instead of WEAR_MAX, then for consitency it should also be replaced everywhere else where 65536 is used for maximum wear. Using U16_MAX or WEAR_MAX in some places, and 65535 in others makes the code messy, and harder to maintain.

Symbolic constants are not recommended for their own sake. They are not a goal, but a means. The goal is to make code more maintainable. If making a constant symbolic decreases understandability of the code, or maintainability, then it should definitely not be done.

[Ferk]

damaging when digging for example

If the "uses" number for the tool_capabilities is zero you don't get any wear when digging (which is already the default since when you don't define any tool_capabilities it's the "hand" tool the one that is used and that one has uses=0).

I assume for an item like a battery or an armor you would not define any tool_capabilities at all (they'll be nil) when you register it as a tool, so the hand defaults will be used and it would not wear out the item when digging with it.

Btw, I just tested in current master and craftitems do support tool_capabilities.
In builtin, register_tool and register_craftitem are just a wrapper to the same minetest.register_item method, which basically just uses different defaults (values here) depending on the type (and btw, tool_capabilities defaults to nil for both).

It might be interesting to test these two axes in your branch:

minetest.register_craftitem("testmod:axe_craftitem", {
    description = "Craftitem Axe",
    inventory_image = "default_tool_woodaxe.png^[colorize:#044:128",
    tool_capabilities = {
        full_punch_interval = 1.0,
        max_drop_level=1,
        groupcaps={
            choppy = {times={[1]=2.10, [2]=0.90, [3]=0.50}, uses=10, maxlevel=2},
        },
        damage_groups = {fleshy=7},
    },
})



minetest.register_tool("testmod:axe_indestructible", {
    description = "Indestructible Axe",
    inventory_image = "default_tool_diamondaxe.png^[colorize:#404:128",
    tool_capabilities = {
        full_punch_interval = 1.0,
        max_drop_level=1,
        groupcaps={
            choppy={times={[1]=2.10, [2]=0.90, [3]=0.50}, uses=0, maxlevel=2},
        },
        damage_groups = {fleshy=7},
    },
})

See if the craftitem axe also wears out when chopping trees.
If it does, then this PR basically makes crafitems and tools the same.

That's not necessarily a bad thing, imho. I think it's still good to have separate registering and types for the sake of organization (creative mode tabs?), even if functionality-wise they are the same.

Maybe the tool repair logic is the only remaining thing that makes a distinction. But if wear is added to craftitems, then that might as well be added too.

Definitely the docs need to be updated, whether we decide to make tools and crafts the same or not.

[stujones11]

Another consideration is that tool items are not stackable, how would wear level be shown on a stack of craft items? I think @Rogier-5 is correct, in my case at least, it doesn't matter to the player whether armor is registered as a tool item or not so long as it does the job. I do still like the idea of a callback to avoid the need for hacks when you don't want an item to be automatically destroyed.

[Rogier-5]

I'd say the difference between a tool and a craftitem_with_bar is that a lot of code that makes sense for tools (damaging when digging for example) doesn't make sense for craftitems_with_bars

@thePalindrome: taking damage after a hit could just be a difference in behavior. Easily implemented using different callbacks. That might be the same for other types of behavior ?

I even think that it might also be sensible to allow nodes to have a bar as well. The wear could be automatically converted to a param2 value, or to a metadata value when the node is placed, and vice versa when dug. Or a callback could take care of that.

Another consideration is that tool items are not stackable, how would wear level be shown on a stack of craft items?

I'd say that at least tools at 0% and at 100% should be easily stackable. Any two identical tools with the same wear could also be stackable. That may seem strange for tools like axes and picks (and it could be disabled for those), but it would make sense for other types of tools, e.g. tools that have just a few possible levels of wear (e.g. 0%, 50%, 100%).

If you hit something using a stack of tools, then the 'rest' of the stack would be moved to another inventory slot. If none is available, the rest of the stack would be dropped...

Regarding the 'bar': Instead of (or in addition to) a bar, it would also be neat to have different images for tools at different levels of wear. At the moment, the only option is to convert the tool to a different tool...

[paramat]

Author writes:

My personal opinion on the matter is that this pull request be denied,

It seems to need a better implementation. Also no progress in a year.