Tsuser1 / Modern-LWC

Welcome to the future of LWC, get ready to get back into this classic plugin!
MIT License
29 stars 14 forks source link

Request: Adjust cpublic for villagers to prevent damage but allow trades #9

Open SXRWahrheit opened 6 years ago

SXRWahrheit commented 6 years ago

Hi! As requested on the forums, here's a github issue for this feature request. I was hoping the plugin could already do it, but I guess not.

I've seen a few people ask for a way to allow players to lock villagers to prevent damage to them, but still allow trades with the non-owner. I think using cpublic would be a great way to do this!

Thanks for all your hard work!

lifehome commented 5 years ago

Hmmm I must confess this seems not appropriate to be a feature for LWC... This plugin aims to protect containers and entities that is owned by players and groups of them.

We got private lock that only enlisted players can access; public lock for everyone to access but not damage the container/entity itself; donation lock for one-way item insertion into the container.

There are actually plenty of entity protection plugin that does the job, such as using WorldGuard flags, or plugins like "SecurityVillagers".

SXRWahrheit commented 5 years ago

A WorldGuard flag requires a defined region. A separate plugin requires... a separate plugin. EntityLWC already allows for public and private locking of villagers. Why not make this change as a distinction between the two?

lifehome commented 5 years ago

A WorldGuard flag does not requires to define a region, __global__ is already a region that covers the entire world. As for the plugin part, what I mean is that plugins have their own usages, and a separate plugin could better manage on that use case.

The reason I said it is not appropriate, is because of the management interface of LWC. From the PoV of players, they know where are their locked containers and entities, but as the scale grows up, for example a zoo of villagers, you can't really tell which is locked or not.

On the other hand, ModernLWC already support the lock of villager, just the interaction of villagers are blocked somehow. No idea why, nor this is a supported feature. I suggest you read the repo wiki again.

SXRWahrheit commented 5 years ago

As you've just stated, __global__ affects the entire world. It does not support a use case where some players would like to protect their villagers, but others would not.

You can't tell whether a chest is locked or not before you click on it, either.

If you read the actual issue, you'll see this is a feature request.

pop4959 commented 5 years ago

Happy to see such good discussion on this post. I thought it was a reasonable idea, as LWC does support different types of locks that cause various behaviours for blocks. This could be extended to entities as well.

For example: Chests cprivate makes them inaccessible and indestructible, while cpublic makes them accessible but indestructible. Villagers cprivate could do the same thing.

The problem is that there are currently a lot of bugs with the entity locking features that make it unrecommended for use on servers currently. I am planning to try to look into redesigning it entirely, and until then implementing this feature would be pointless and probably introduce more bugs. Also hopefully it would mean that defining special behaviour for different entities would be more modular and thus easier to implement and maintain.

SXRWahrheit commented 5 years ago

No worries, @pop4959! I'm patient - as noted in the initial filing, this is just a feature request. It was odd to me to see such an attack on it from someone who hadn't otherwise commented on it yet. I don't think any of their proposed alternatives offer feature-parity, and if LWC is going to have entity locking anyway, it might as well have neat features!

If people want cpublic villagers to have locked trades, too, it seems easy enough to add a config setting for that. But it seems like cpublic and cprivate for villagers are exactly the same if trades are locked to the owner, so the distinction feels like a natural addition to me.

pop4959 commented 5 years ago

I don't think it was meant as an attack. I think he was just trying to be helpful and find an alternative solution to your issue.

Anyways, I just wanted to give a little status update. This issue has certainly been on the backburner for a while.

SXRWahrheit commented 5 years ago

1.13 is broken as all hell, so no worries. I'm waiting on quite a few plugins to update. 🙃