EnderChest support 👻

[applenick]

trafficstars

EnderChest support

Hooray! 👻 This PR adds proper EnderChest support to PGM.

XML Example

<!-- Add drop-off locations for when player switches teams or leave the match -->
<enderchest>
  <!-- Each drop-off requires a region & filter attribute -->
  <dropoff region="red-spawn" filter="red-only"/>
  <dropoff region="blue-spawn" filter="blue-only"/>
</enderchest>

<!-- Specify a fallback location option for when no drop-off locations are defined (AUTO, KEEP, or DELETE) -->
<enderchest fallback="DELETE"/>

If you've got any feedback let me know and I'd be more than happy to make accommodations. As always these changes have been tested and should work as intended :+1:

Signed-off-by: applenick [email protected]

[KingOfSquares]

I believe players still would be blocked from placing ender chests because of the listener in EventFilterMatchModule https://github.com/PGMDev/PGM/blob/963f02a2974c471f356b782b5e0fd741a77869fc/core/src/main/java/tc/oc/pgm/modules/EventFilterMatchModule.java#L259-L271 Maybe also filter that on the enabled boolean?

EDIT: Or maybe not? Since you are cancelling the event, but then this listener is redundant

Also, with this addition maybe it would also make sense to add ender chest support to kits?

[applenick]

@KingOfSquares Great point! I believe you're correct, my changes don't currently adjust EventFilterMatchModule blocking the placing/breaking of EnderChests. Personally I don't foresee many instances where map authors would want players to be able to freely place them, so chose to preserve that function while allowing interacting with existing EnderChests to be supported. Since the more common usage will be interacting with strategically placed chests on a map.

Would you suggest allowing them to be placed?

Also, with this addition maybe it would also make sense to add ender chest support to kits?

Could you elaborate on this?

Thanks!

[KingOfSquares]

@KingOfSquares Great point! I believe you're correct, my changes don't currently adjust EventFilterMatchModule blocking the placing/breaking of EnderChests. Personally I don't foresee many instances where map authors would want players to be able to freely place them, so chose to preserve that function while allowing interacting with existing EnderChests to be supported. Since the more common usage will be interacting with strategically placed chests on a map.

Would you suggest allowing them to be placed?

Also, with this addition maybe it would also make sense to add ender chest support to kits?

Could you elaborate on this?

Thanks!

For the kit thing, there is some support already in Slot.java for being able to place items in players ender chests through kits. A kit could potentially have an <item slot="enderchest.1" material="stone sword"/>

For placing ender chests: I agree that the ability to being able to place ender chests is not correct in most situations, but since they are now usable I only think its a matter of time before someone requests how to enable placing them(for some reason). Why not default it to false and include the possibility as an attribute? I also believe the error message is a little misleading, Ender chests are disabled should probably be something like Placing ender chests are disabled

[Electroid]

Hey @applenick, thanks for working on this.

I think it would be better to integrate ender chest support into the existing Kit and Slot system. Also, maybe we already have support??

https://github.com/PGMDev/PGM/blob/a3992e87639f52f5d3c7ed6f5d2d9690f6c56d4f/core/src/main/java/tc/oc/pgm/kits/Slot.java#L169

[applenick]

Hey @applenick, thanks for working on this.

I think it would be better to integrate ender chest support into the existing Kit and Slot system. Also, maybe we already have support??

https://github.com/PGMDev/PGM/blob/a3992e87639f52f5d3c7ed6f5d2d9690f6c56d4f/core/src/main/java/tc/oc/pgm/kits/Slot.java#L169

No problem! Excellent point about the kit/slot support, I believe Simon mentioned that too in earlier comments. Perhaps we should just scrap the custom inventory logic and have it rely on the built-in enderchests. That way they'll already support slots like mentioned. Though I believe it's important to keep some of the new functionality added like drop-off locations and the fallback attribute.

Will look into fixing this up over the weekend 👍

[applenick]

Finally had some time and fixed this up 🎉 Switched away from using the custom enderchest implementation we originally had, relying now on the traditional enderchest contents. We do lose the capability to customize enderchest row count, but I believe it's a fair tradeoff for being able to have enderchests remain compatible with the Kit/Slot system.

If there's any additional feedback let me know and I'll be happy to make adjustments 👍