CivClassic / FactoryMod

Configurable factories for automating item production - Built for Paper 1.16.5
Other
2 stars 17 forks source link

Many factory types continue to run when chest is full #25

Open squareblob opened 4 years ago

squareblob commented 4 years ago

This is fixed for compactors (#13). Other factory types, such as production factories, will continue to run and in the process waste output. It would reduce annoyance (particularly for new players) if all factories automatically deactivated when insufficient output storage space exists.

Inframission commented 4 years ago

This is also a bug with current 'working' factories. At least some of the time compactors will continue running and delete stacks. When this happens the factory can still be started even when the chest is full.

Maxopoly commented 4 years ago

Fixed by https://github.com/CivClassic/FactoryMod/pull/27 for production recipes, remains an open issue for other types.

Falvyu commented 4 years ago

This is also a bug with current 'working' factories. At least some of the time compactors will continue running and delete stacks. When this happens the factory can still be started even when the chest is full.

Weird, that's not supposed to happen for compactors. Try to decompact materials when the compactor is full stops the compactor and does not consume any item. Are you able to reproduce it ?

Inframission commented 4 years ago

Try to decompact materials when the compactor is full stops the compactor and does not consume any item.

Yes, compactors are still affected. See https://gfycat.com/frigidwaterloggedgrosbeak

Falvyu commented 4 years ago

Alright, I think I've found the issue.

The "fix" to factories relies on the fitsIn(Inventory) method. This method works as follows:

  1. A "fictive" inventory (invCopy) is created
  2. All the items from the given inventory are added to it using the addItemStack() method
  3. The item stack is then added into it.
  4. If the size exceeds the original one then that means that the item stack doesn't fit.

This might seems fine at first. However step 2 will stack the item and will "defragment" the given inventory.

Thus, if we give this inventory as a parameter: Input

The representation will be: Output

Since the representation has 1 available slot, the fitsIn() function will return true and the factory will still execute the recipe.

I think there are 2 "basic" solutions to that:

Either choice shouldn't be too hard to implement. But I'm curious to hear which one @Maxopoly prefers.

Maxopoly commented 4 years ago

Fix fitsIn(), it was made with this use case in mind.

Diet-Cola commented 3 years ago

Is this still an issue? Can't seem to replicate on test