Fix cloned mob item dupe - #122
Conversation
Cloning a mob with an inventory (e.g. pack mule) copied its full NBT including items, allowing item duplication. Fixed by clearing all inventory slots after createEntityFromNBT.
tiffit
left a comment
There was a problem hiding this comment.
small nit, looks good otherwise
|
Have we verified that this actually works? Also, even if this did work, this triggers every time the vat ticks, not when the vat duplicates an entity, so if I take my already-existing mule and put it in the vat, its inventory would get voided immediately, even though I didn't duplicate any items. |
…Vat.java Co-authored-by: tiffit <10173431+tiffit@users.noreply.github.com>
|
Good catch on IInvBasic. Fixed by stripping inventory-related NBT keys (Items, ArmorItem, SaddleItem) from a copy of the critter compound before passing it to createEntityFromNBT, so it covers all entity types regardless of which inventory interface they implement. On the second point - the clearing only happens inside the |
…vBasic (EntityHorse)
|
Why not delete it directly from |
|
The blood sample item intentionally stores the full mob NBT (it's used for aspect display and other lookups). Stripping inventory keys at storage time would lose data that may be needed elsewhere. Cleaning up only at clone time in TileVat keeps the sample intact and targets exactly the problem - items being duplicated on entity creation. |
|
Actually, looking at ItemSyringeBloodSample more carefully - it only reads critterName and CreatureInfusion from the critter NBT, so stripping inventory keys at sample creation time would be fine too. Both approaches work; the question is where the responsibility sits. Cleaning in TileVat keeps the item layer untouched and makes the fix explicit at the point of entity creation. Cleaning in ItemSyringeBloodSample puts it closer to the data source and prevents the dirty NBT from ever being stored. What do you think is the better place? |
|
It depends on the responsibility. But I'm leaning toward ItemSyringeBloodSample. After all, logically, data like items shouldn't be contained in a blood sample. It should only contain the mob type itself and its "genetic" (individual) parameters. And it's simply not sensible to store unused data. |
|
The "ChestedHorse" tag still needs to be removed, otherwise you'll clone a dressed chest on a donkey. |
|
Good catch, added ChestedHorse to the strip list. |
|
Agreed, moved the stripping to ItemSyringeEmpty at sample creation time. A blood sample shouldn't carry inventory data at all. |
Cloning a mob via blood sample copied its full NBT including carried items, allowing item duplication. Fixed by stripping Items, ArmorItem, SaddleItem and ChestedHorse from the critter tag in ItemSyringeEmpty when the sample is taken - a blood sample should only carry genetic data.
Nikolay-Sitnikov
left a comment
There was a problem hiding this comment.
Should work for now. This isn't a particularly good check, because we can never really know where a mod might store its items in its NBT, but this should work for Luggage & Mules.
Taking a blood sample copied the mob's full NBT, including its inventory, so cloning the mob duplicated any carried items. Nesting jarred pack mules inside each other made the duping exponential. A blood sample should carry only genetic data, so it no longer keeps the mob's inventory.
Closes https://github.com/GTNewHorizons/Dupes-Exploits-GTNH/issues/231