Skip to content

Conversation

@FDUTCH
Copy link
Contributor

@FDUTCH FDUTCH commented Sep 18, 2025

I think it is better to have this ability

Copy link
Member

@TwistedAsylumMC TwistedAsylumMC left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is better to have this ability
Do you have an actual use case for this feature? Could you give an example?

@FDUTCH
Copy link
Contributor Author

FDUTCH commented Sep 18, 2025

I think it is better to have this ability
Do you have an actual use case for this feature? Could you give an example?

I'm not going to argue, I know it's pretty pointless, but I just need it

@Dasciam
Copy link
Contributor

Dasciam commented Sep 18, 2025

we do not have any need to restrict the developer from making items with incompatible enchantments anyway

@TwistedAsylumMC
Copy link
Member

I think that's fair enough, but I do feel like this method deserves a better name. Currently it sounds like every enchantment you provided has to be incompatible. What do you think of something like WithIncompatibleEnchantmentsAllowed() or something similar?

@FDUTCH
Copy link
Contributor Author

FDUTCH commented Sep 18, 2025

I think that's fair enough, but I do feel like this method deserves a better name. Currently it sounds like every enchantment you provided has to be incompatible. What do you think of something like WithIncompatibleEnchantmentsAllowed() or something similar?

maybe, I'm not too good at naming

@Flonja
Copy link
Contributor

Flonja commented Sep 18, 2025

or have the WithEnchantments method be WithEnchantments(ignoreIncompatibles bool, enchantments Enchantment...). You would break the original usage for little gain tho...

@FDUTCH
Copy link
Contributor Author

FDUTCH commented Sep 18, 2025

or have the WithEnchantments method be WithEnchantments(ignoreIncompatibles bool, enchantments Enchantment...). You would break the original usage for little gain tho...

I think it's better to create a separate method for this

@FDUTCH
Copy link
Contributor Author

FDUTCH commented Sep 18, 2025

I guess it is ready to be merged

@FDUTCH
Copy link
Contributor Author

FDUTCH commented Nov 16, 2025

maybe merge? 🤔

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.

4 participants