[Question] NestingPen Resource Chicken Drops
gabizou opened this issue ยท 3 comments
While writing and debugging and fixing SpongePowered/SpongeForge#1259, I stumbled across some oddities in how you're handling the drop captures for the NestingPen.
Specifically here:
https://github.com/GenDeathrow/Hatchery/blob/1.12/src/main/java/com/gendeathrow/hatchery/block/nestpen/NestPenTileEntity.java#L235-L243
Where you're performing the entity onUpdate before you set captures to true.
Because in Sponge I've ended up fixing the bug in such a way that entity drops are captured regardless of circumstance, the captureDrops will still contain the resource item.
However, the thing that I've stumbled on is that [the chicken is never told to stop captureDrops by setting it to false, even though here:
The drops are being added to the hatchery inventory.
I'm not sure the intention of the captures are for, but I've been able to verify now that as SpongePowered/SpongeForge#2110 somewhat fixes the original issue, but I've noticed that the resource drops now are included in the Hatchery inventory.
So, my question is whether Hatchery is intending to be dropping the resource items from the chicken outside the Hatchery, and weather the other drops (like feathers and manure) to be captured into the Hatchery inventory?
Not sure when I had done that but I think I must have moved the "capturedrops = true" at some point below the first update, but that will only affect the first tick. but wouldn't hurt to be moved to the top.
It gets reset to false, when the nesting pen is broke or the entity is removed from the pen. It needs to be true as long as the entity is in the pen.
The intention of capturing drops is to catch modded chickens. Like the chickens mod, or any mod that adds in custom chickens / custom drops. Seeing that forge has a capture drop hook already added, it made the code simple to catch any drop that entity drops. As long as other modders use the correct dropping method. My nesting pens should capture anything those entities drop. So yes its intended to capture dropped resources from mods outside of hatchery.
The feathers are only dropped into nesting pens inventory, and manure is a bonus drop, on top of what chickens already drop.
I hope that clears up any questions. I appreciate ya taking the time on this.
Now this makes a lot more sense. I had considered that in the PR I've submitted to Forge would eliminate the field setter for capture drops as the original intention for capturing drops in Forge's patches was to handle the extra drops for entities (in cases like Notch dying) to throw the drop events properly. But the biggest change would be that the drop capture list would be more "functional" and "transient" in that it's not a permanent field and only used during the specific entry point so the event is more formalized to be controlled by the mod developer and not left to sit around unknown to everyone's interpretation of how the list should can be used. Not to burst the bubble for how it's being handled now, but I'm on the side of the boolean field is rather useless in most cases, as it appears that it's not even considered for most use cases.
At the very least, in Sponge, we'll be ensuring the lists are synchronized for whenever they are used (be it sponge's event/phase management or mods like Hatchery using them to pull items into inventories), but this proved to be a good glimpse at some interesting uses of the field. As this is the only issue that I've seen where drops are used on an entity that is technically stored in a TileEntity, I can't imagine it's a common practice to do what Hatchery is doing.
Last question from me about the nesting/nesting pens/etc. When the entity is stored into the tile entity to tick, is the entity still added to the world? Or is the entity semi removed from the world but has a reference to the world of the tile entity?
I completely get it, this isn't the way I wanted to do it originally. At first I wanted to use forge events, but nothing worked for what I needed.I feel forge could have added a better method for using this, But my bet is that it was not used enough for them to optimize it.
Its semi removed from the world ,and references the tile entity when needed.