CandyNim
Tiller
Bug: `Farmer.addItemToInventoryBool` only returns True if the item is fully added to the inventory, instead of partially - and this behavior is expected in some locations, and not expected in others.
Symptoms: If you have a cow golden animal cracker’d, and your inventory is full except for 998 of the relevant milk item, you can milk the cow, get 1 milk, and the cow will still be milkable allowing infinite milk. This can also be done with shears, or with calico egg rewards from calico statues (this one only lets you double the reward).
Code Analysis: The relevant lines of code are
```c#
remainder = this.addItemToInventory(item);
bool success = remainder?.Stack != item.Stack || item is SpecialItem;
```
(There are intermediate lines of code, but they’re irrelevant in this case). The bug lies in how `addItemToInventory` functions; if we check the comment above `addItemToInventory`, we notice "Else returns the input item with its stack reduced to the amount that couldn't be added”. It does not return a copy of the input item with its stack reduced; no, it reduces the stack of the input item and then returns the input item back. What does this mean? Well, it means that remainder is the exact same as item unless the item got fully added (in which case item.Stack is 0 and remainder is null) and so this code doesn’t check if the stack has changed.
Notably, however, this behavior is expected in several other locations, like harvesting a machine and only picking up part of the stack (at least, I sure hope its expected because it's very nice). Fixing the cow / sheep / calico statues might be better?
(Note that addItemtoInventory’s behavior of reducing the stack size of the original item is used in other places, such as WoodChipper.checkForAction, FishPond.doAction, Object.CheckForActionOnMachine, and probably another place or two.)
Suggested Solution: Unsure - it's a relatively niche bug (and a duplication bug which only works when you have 999 of the item already is much worse of a duplication bug lol). Given the high chance of unexpected side effects of changing additemtoinventorybool, making milking/shearing/calico statues [who spawn the full stack of items on the ground when used with full inventory] expect this behavior would be a better fix I think.
Attached is a save file where you can see the bug in action, and it isn't patched on the most recent version as far as I can tell.
Symptoms: If you have a cow golden animal cracker’d, and your inventory is full except for 998 of the relevant milk item, you can milk the cow, get 1 milk, and the cow will still be milkable allowing infinite milk. This can also be done with shears, or with calico egg rewards from calico statues (this one only lets you double the reward).
Code Analysis: The relevant lines of code are
```c#
remainder = this.addItemToInventory(item);
bool success = remainder?.Stack != item.Stack || item is SpecialItem;
```
(There are intermediate lines of code, but they’re irrelevant in this case). The bug lies in how `addItemToInventory` functions; if we check the comment above `addItemToInventory`, we notice "Else returns the input item with its stack reduced to the amount that couldn't be added”. It does not return a copy of the input item with its stack reduced; no, it reduces the stack of the input item and then returns the input item back. What does this mean? Well, it means that remainder is the exact same as item unless the item got fully added (in which case item.Stack is 0 and remainder is null) and so this code doesn’t check if the stack has changed.
Notably, however, this behavior is expected in several other locations, like harvesting a machine and only picking up part of the stack (at least, I sure hope its expected because it's very nice). Fixing the cow / sheep / calico statues might be better?
(Note that addItemtoInventory’s behavior of reducing the stack size of the original item is used in other places, such as WoodChipper.checkForAction, FishPond.doAction, Object.CheckForActionOnMachine, and probably another place or two.)
Suggested Solution: Unsure - it's a relatively niche bug (and a duplication bug which only works when you have 999 of the item already is much worse of a duplication bug lol). Given the high chance of unexpected side effects of changing additemtoinventorybool, making milking/shearing/calico statues [who spawn the full stack of items on the ground when used with full inventory] expect this behavior would be a better fix I think.
Attached is a save file where you can see the bug in action, and it isn't patched on the most recent version as far as I can tell.