Skip to content

Remove bad supports() in key impl#4184

Closed
MrHell228 wants to merge 2 commits intoSpongePowered:api-12from
MrHell228:api-12-key-fixes
Closed

Remove bad supports() in key impl#4184
MrHell228 wants to merge 2 commits intoSpongePowered:api-12from
MrHell228:api-12-key-fixes

Conversation

@MrHell228
Copy link
Copy Markdown
Contributor

These supports make no sense because these keys reflect components that can be applied to any stack.

For firework data the only remaining limitation is that FIREWORK_STAR assumed to be the only item that can have FIREWORK_EXPLOSION component. For other stacks Keys.FIREWORK_EFFECTS sets regular firework component (and will do the same whenever firework star component gets it's own key).

@aromaa
Copy link
Copy Markdown
Member

aromaa commented Mar 30, 2025

Did you test whatever these actually work when offered to any item type? For example, books still need to be WritableBookItem as thats the only place where ServerPlayer#openItemGui is called.

@MrHell228
Copy link
Copy Markdown
Contributor Author

I don't really see where it requires book to be WritableBookItem. ServerPlayer#openItemGui, as I can see, will open book gui for any item with book component.
WRITTEN_BOOK component currently will not add lore on any item other than WrittenBookItem in vanilla (which I believe is pretty temporary because most components now simply implement TooltipProvider). And right click on non-book items will not open book gui (same as tooltip, game slowly moving towards this).
But that's not the goal. The main issue with these supports is that they break compatability with any mod that reuse these vanilla components. I had this problem with enchanted book component (had PR for that too).

@aromaa
Copy link
Copy Markdown
Member

aromaa commented Mar 31, 2025

I'm just worried on what the developer expectation is. Vanilla will not open just any item that has the component attached to it.

If a mod uses these components to implement their own custom logic, I'm fine with merging this then.

@MrHell228
Copy link
Copy Markdown
Contributor Author

I don't have anything to say about developer expectation tbh. But I do know that mods can use these components. Not sure specificially about book-related ones, but I had mod that used enchanments component and believe there can exist mod that at least makes book component work on any item (not a hard thing to implement, written book can be implemented even by plugin)

@Yeregorix
Copy link
Copy Markdown
Member

Merged by 59f1841

@Yeregorix Yeregorix closed this Sep 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants