Add tradable attribute to SlimefunItem

[ybw0014]

trafficstars

Description

This allows addons to set whether the SlimefunItem is allowed to be traded with villagers and wandering traders.

Proposed changes

• Add tradable attribute to Slimefun, with getter and setter.
• VanillaItem and SyntheticEmerald have tradable set to true by default.
• Change the method to determine whether the item is tradable in VillagerTradingListener, now it only reads tradable.

Related Issues (if applicable)

N/A

Checklist

• [x] I have fully tested the proposed changes and promise that they will not break everything into chaos.
• [x] I have also tested the proposed changes in combination with various popular addons and can confirm my changes do not break them.
• [x] I have made sure that the proposed changes do not break compatibility across the supported Minecraft versions (1.16.* - 1.19.*).
• [x] I followed the existing code standards and didn't mess up the formatting.
• [x] I did my best to add documentation to any public classes or methods I added.
• [ ] I have added Nonnull and Nullable annotations to my methods to indicate their behaviour for null values
• [ ] I added sufficient Unit Tests to cover my code.

[WalshyDev]

Can we get a test? :)

I like this.

[ybw0014]

Testing, I got some problems. Turning it to draft for now.

[ybw0014]

btw do you prefer tradable or tradeable?

[ybw0014]

Also do we need to change the message shown? It now says "You cannot trade Slimefun items with Villagers!" What about changing it to "You cannot trade this Slimefun item with Villagers or Wandering Traders!"

[Sefiraat]

Also do we need to change the message shown? It now says "You cannot trade Slimefun items with Villagers!" What about changing it to "You cannot trade this Slimefun item with Villagers or Wandering Traders!"

Yes change it otherwise it gives the impression that it’s all items :)

[Sefiraat]

btw do you prefer tradable or tradeable?

Either is fine. I’d stick with what you have now. I prefer it with the e personally but I think that’s a British vs American thing.

[Sefiraat]

But of a side question - Assuming this is still to aid your work on the trading addon, and given it’s not going to sort out the root of the issue, has consideration been taken for the changes that would be needed to allow slimefun items to be traded regardless of this setting for a custom trade as defined?

[ybw0014]

But of a side question - Assuming this is still to aid your work on the trading addon, and given it’s not going to sort out the root of the issue, has consideration been taken for the changes that would be needed to allow slimefun items to be traded regardless of this setting for a custom trade as defined?

I haven't thought of this, maybe I will open a new pr for the trading item on the list.

[Sefiraat]

But of a side question - Assuming this is still to aid your work on the trading addon, and given it’s not going to sort out the root of the issue, has consideration been taken for the changes that would be needed to allow slimefun items to be traded regardless of this setting for a custom trade as defined?

I haven't thought of this, maybe I will open a new pr for the trading item on the list.

The only reason I mention it is because that change will make any item subsequently tradable but only for a slimefun > slimefun trade meaning the term traceable becomes too ambiguous?

[github-actions[bot]]

Slimefun preview build

A Slimefun preview build is available for testing! Commit: 9c2a7c86

https://preview-builds.walshy.dev/download/Slimefun/3832/9c2a7c86

Note: This is not a supported build and is only here for the purposes of testing. Do not run this on a live server and do not report bugs anywhere but this PR!