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

Add c:indestructible tag #941

Open
Dev0Louis opened this issue May 7, 2024 · 23 comments
Open

Add c:indestructible tag #941

Dev0Louis opened this issue May 7, 2024 · 23 comments
Labels
enhancement New (or improvement to existing) feature or request

Comments

@Dev0Louis
Copy link

This Issues asks to add the c:indestructible tag for items.

This tag is there to represent items which shall not be destroyed.

I don't know if c:indestructible is the correct naming. But I don't have a better idea.

I have not added a block tag. As I would be unsure if bedrock would go into it? For some mods bedrock may be considered as destructible.

This tag does have no gameplay altering effect. That means that items are still destructible my vanilla methods. This will need to be prevented by the mod itself. The tag is there to tell other mods that something should not be destroyed.

This issue is there to get feedback from both mod loaders for this.
FabricMC/fabric#3758

@Dev0Louis Dev0Louis added the enhancement New (or improvement to existing) feature or request label May 7, 2024
@TelepathicGrunt
Copy link
Sponsor Contributor

Note, some clarification from the reporter on how the tag is to be used:

  • items shouldn’t be destroyed by trash can mods

  • items should not be destroyed by despawning time

Basically eternal items that are permanent always. Tbh, I would see despawning as its own thing separate to destructibility

@Tslat
Copy link

Tslat commented May 26, 2024

Noting that UNBREAKABLE is its own DataComponent now

@dhyces
Copy link
Contributor

dhyces commented May 26, 2024

I would consider that to have different meaning than an indestructible tag. Unbreakable is used to prevent damageable items from taking damage. That's different than just saying that an item should not take any "damage" ever, ie from trash slots, lava, void, etc. Would I promote this tag as this sort of blanket solution? probably not

@Tslat
Copy link

Tslat commented May 26, 2024

My question is -
If the mod itself is the one implementing the functionality, then why should NeoForge be the one adding the tag?

It'd be like adding a tag for IMMUNE_TO_POISON, but not using it - functionally it's misleading and will be repeatedly misconstrued as a bug

@TelepathicGrunt
Copy link
Sponsor Contributor

If the mod itself is the one implementing the functionality, then why should NeoForge be the one adding the tag?

We already have tags that state what mods should do with them. See c:hidden_from_recipe_viewers block, fluid, item tags. c:hidden_from_locator_selection and c:hidden_from_displayers and c:no_default_monsters biome tags. c:teleporting_not_supported and c:capturing_not_supported entity type tags. c:relocation_not_supported block tag.

The javadoc on all these tags explain exactly what they are for and how they should be checked and handled by other modders. The reason these tags exist is because doing the tag per mod is horrifying painful. Especially for the capturing not supported when I ended up needing to package like 15 different mod specific tags just to have my entity not be captured into items and stuff. So there is precedence for modloaders providing a common tag for modders to utilize and implement.

Though c:indestructable is a bit too vague for what other modders would see without javadoc. Perhaps c:cannot_be_deleted would be more clear? The modder adding to the tag would need to handle vanilla cactus and lava (lava by using the component that makes it immune) but for notifying other mods that delete items, a common tag would be needed.

Granted this is a niche use case though

@KnightMiner
Copy link

KnightMiner commented May 27, 2024

Deleted sounds like it would block trash can slots/trash can blocks, which is probably not the intention.

From my understanding, the tag is just meant to say it's invincible in world, correct? If so, like you often hear me say, it should be a tool action as I add that sort of behavior to items based on NBT. Given you need to implement behavior in code anyways for the tag to work this case actually has no benefit to being a tag (a modpack maker has no reason to add something to the tag, it's actually qiite misleading for them as they see no javadocs)

@TelepathicGrunt
Copy link
Sponsor Contributor

Note, the original requester @Dev0Louis came from fabric iirc so they don’t have the option of ToolActions equivalent over there. Hence why they wanted to do a tag. But I’ll let Louse talk more about their use cases

@KnightMiner
Copy link

Whether fabric has tool actions or not does not matter if tool actions are the better way to solve this problem. I think it's perfectly reasonable for the tag to only exist on Fabric if Neo handles it via a different system, you have to use tool actions to write a good Neo mod anyways and it does not hurt having an unused tag

@Tslat
Copy link

Tslat commented May 27, 2024

If we want it to be NBT dependent then maybe it should be a DataComponent
This doesn't at all feel like a ToolAction to me

@Shadows-of-Fire
Copy link
Contributor

I'll be honest a tag like this that does not have any gameplay altering effects does not seem particularly useful. What good is a marker that dictates an item should not be deleted if it doesn't prevent vanilla from doing so?

@KnightMiner
Copy link

If we want it to be NBT dependent then maybe it should be a DataComponent This doesn't at all feel like a ToolAction to me

Do you know how tool actions work? They are literally just NBT sensitive tags.

Like I said on #1005, using data components to replace tool actions sounds terrible as you are now forced to mirror whatever conditions you have in custom data into the data component. In my case I often have 2-3 different sources of a tool action (including sources that would provide the proposed indestructible action) which becomes a pain to update when one source gets removed. Data components are not a replacement for tool actions.

@Tslat
Copy link

Tslat commented May 27, 2024

If we want it to be NBT dependent then maybe it should be a DataComponent This doesn't at all feel like a ToolAction to me

Do you know how tool actions work? They are literally just NBT sensitive tags.

Like I said on #1005, using data components to replace tool actions sounds terrible as you are now forced to mirror whatever conditions you have in custom data into the data component. In my case I often have 2-3 different sources of a tool action (including sources that would provide the proposed indestructible action) which becomes a pain to update when one source gets removed. Data components are not a replacement for tool actions.

Right, except this is neither for tools, not is it an action

Also this is not already a toolaction, so #1005 is irrelevant here

@KnightMiner
Copy link

KnightMiner commented May 27, 2024

Tool action is poorly named, yes. However, it's still the best API for an NBT sensitive tag. Data components might be okay for non-tools as an API but are terrible for tools, thus your argument of "it's not a tool" is irrelevant as your suggestion does not work for tools where this would be most used.

@Tslat
Copy link

Tslat commented May 28, 2024

Are datacomponents not nbt sensitive by definition?

@KnightMiner
Copy link

KnightMiner commented May 28, 2024

As I said in #1005 (which you deemed as irrelevant, I suggest you go read it): that requires copying data from your NBT to the component which is a pain to sync. Unless of course you have a non tool.

@Dev0Louis
Copy link
Author

Deleted sounds like it would block trash can slots/trash can blocks, which is probably not the intention.

From my understanding, the tag is just meant to say it's invincible in world, correct? If so, like you often hear me say, it should be a tool action as I add that sort of behavior to items based on NBT. Given you need to implement behavior in code anyways for the tag to work this case actually has no benefit to being a tag (a modpack maker has no reason to add something to the tag, it's actually qiite misleading for them as they see no javadocs)

My use-case for this tag would be to have an item that can not be destructed under any circumstances in normal gameplay.
And yeah as you said the tag does do nothing out of default, but with conventional tags there is no way around that.
These tags are there for inter-mod communication on what can be done as @TelepathicGrunt already pointed out.

Idk what the heck tool actions are. I am a fabric modder and I have never modded using forge so I don't know if there already is a solution on this modding platform.

And to the name yeah that is up to debate. c:indestructible is the one I came up with, but if there is a better name I am happy to change it.

@Dev0Louis
Copy link
Author

Whether fabric has tool actions or not does not matter if tool actions are the better way to solve this problem. I think it's perfectly reasonable for the tag to only exist on Fabric if Neo handles it via a different system, you have to use tool actions to write a good Neo mod anyways and it does not hurt having an unused tag

I think the goal of the conventional tags it to be conventional and as such should be keeped in sync.

@Dev0Louis
Copy link
Author

I'll be honest a tag like this that does not have any gameplay altering effects does not seem particularly useful. What good is a marker that dictates an item should not be deleted if it doesn't prevent vanilla from doing so?

Conventional tags as far as my understanding goes can by definition not have gameplay altering effects.

This tag is a I have explained above only there to add inter-mod awareness about items that indestructible or permanent whatever we wanna call it.

@KnightMiner
Copy link

KnightMiner commented May 28, 2024

I think the goal of the conventional tags it to be conventional and as such should be keeped in sync.

There will be times when Neo has a system that makes the tag the wrong solution. In those cases, it's better to not have competing standards. The point of the common tags is to make multiloader easier, but it should not come at the expense of ignoring Neo systems. It's perfectly fine for a tag to be fabric only in such a case.

@TelepathicGrunt
Copy link
Sponsor Contributor

@Dev0Louis Suggestion, how about have to tag be modloader namespace with functionality attached? So fabric:cannot_be_destroyed, patches vanilla to not destroy the itemstacks that have the tagged item, and mods can check for that fabric tag. In Neo, it can be a ToolActions (which is a special string-backed object that items can return in a special method dynamically based on the itemstack data)

@Tslat
Copy link

Tslat commented May 28, 2024

As I said in #1005 (which you deemed as irrelevant, I suggest you go read it): that requires copying data from your NBT to the component which is a pain to sync. Unless of course you have a non tool.

I'm not sure I understand how that is the case?
You can make a datacomponent, have its default value be set to true, and bam, it works
Then users can modify it via command/whatever on an individual basis to their will

@KnightMiner
Copy link

KnightMiner commented May 28, 2024

Then users can modify it via command/whatever on an individual basis to their will

Imagine I have an enchantment that makes a tool "indestructible". Every time I add the enchantment, I need to manually update the indestructible component. Every time I remove it, I need to manually remove the component. This sounds straight forward in theory, but there are many sources of adding and removing enchantments and not all have relevant events.

Now imagine I can also make the tool indestructible via an anvil recipe. If I remove the enchantment, it should not remove the anvil recipe indestructible, but how do I know that unless I control both sources? All my enchantment knows is when the enchantment is removed I need to remove the component. Having to manually copy over the data requires me to have a complete knowledge of every source of the flag being set, this is absolutely horrible code design.

Compare this to tool actions. I ask "is this indestructible?" and then each source can respond yes or no. It is guaranteed to be correct for the given state of the stack, rather than having 1 field that may be out of sync with the other two.

@Tslat
Copy link

Tslat commented May 28, 2024

Fair enough, thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New (or improvement to existing) feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants