CyclopsMC / IntegratedTunnels

Transfer other energy, items and fluids over Integrated Dynamics networks
MIT License
20 stars 13 forks source link

`exact amount` and `check stack size` (mis)behaviour #215

Closed met4000 closed 3 years ago

met4000 commented 3 years ago

Issue type:


Short description:

See original conversation on the discord; initial question: https://discord.com/channels/386052815128100865/386054223688630272/814828281982353449 initial response: https://discord.com/channels/386052815128100865/386054223688630272/814889692289826816

The exact amount and check stack size aspects settings of item importers and exporters do not work as intended. More precisely, check stack size seems to work as exact amount should, and exact amount seems to do nothing.

This has only been tested with item tunnels and using the export item aspect, but it is highly likely that this problem also occurs with other item tunnel aspects (and other tunnel types such as fluid tunnels) that share those two aspect settings.

Steps to reproduce the problem:

Skip steps 3 and 4 if you instead hard-code the item variable card using a logic programmer.

  1. Place the 'input chest' and attach an item interface
  2. Place the 'output chest' and attach an item exporter
  3. Place the 'reference chest' and attach an inventory reader
  4. Encode a variable card using the slot item aspect (set to slot 0) of the inventory reader
  5. Put the item variable card from step 4 into the export item aspect of the item exporter

image

The same setup was used to test item importers, but instead with an item importer on the input chest (step 1) and a item interface on the output chest (step 2).

To test the behaviour, 2 gold blocks were placed in the reference chest (i.e. the item variable card had a value of '2 gold blocks'), and the transfer rate and ticks/op of the item tunnel were both set to 20. The two aspect settings were then set, and the behaviour when placing a stack of 64 gold blocks into the input chest was recorded.

exact amount check stack size transfer behaviour
false false 3 stacks of 20, followed by 1 stack of 4
false true 3 stacks of 20 (the remaining stack of 4 is not transferred)
true false 3 stacks of 20, followed by 1 stack of 4
true true 3 stacks of 20 (the remaining stack of 4 is not transferred)

Expected behaviour:

From @rubensworks (see this message on the discord):

"check stack size": move a stack (per tick) with up to the given stack size, can also be less. "exact amount": only move a stack (per tick) with exactly the given stack size, nothing more, nothing less.

However; Given that the 'normal' behaviour (with both of these setting set to false) seems to allow it to transfer 'up to' the set stack size, check stack size seems to be identical to the normal behaviour and hence redundant. I would propose leaving exact amount as is, but making check stack size instead define whether the "item's stack size" or transfer rate is used to determine the... transfer rate. I.e. check stack size = false => transfer rate-worth of items is transported, and check stack size = true => "item's stack size"-worth of items is transported. As this does suggest a change to intended behaviour, it may be appropriate to instead make this its own issue.


Versions:

mod version
MC 1.12.2
FML 8.0.99.99
Forge 14.23.5.2854
Cyclops Core 1.12.2-1.6.7
Common Capabilities 1.12.2-2.4.8
InDy 1.12.2-1.1.11
InTu 1.12.2-1.6.14
InDy Compat 1.0.0
InTu Compat 1.0.0

(retrieved using the in-game mod list from the pause menu)

Log file:

N/A

rubensworks commented 3 years ago

Thanks for reporting!

met4000 commented 3 years ago

@rubensworks I noticed you added the mc-1.16 label; I tested this in 1.12.2, and while I believe this happens in 1.16 as well I haven't confirmed it.

( Ignore this if you did testing yourself to check it :) )

rubensworks commented 3 years ago

Haven't done testing yet. 1.12 is not maintained anymore, which is why I would only fix/change it in 1.16.

Jack-McKalling commented 3 years ago

I have reproduced the same results in 1.16

rubensworks commented 3 years ago

Thanks for checking!

rubensworks commented 3 years ago

Note to self: this is more complex than I anticipated. It looks like the exactQuantity flag is not working properly inside the IngredientStorageHelpers. First do in-depth (unit) tests to see if things work correctly there, then translate to IT.

rubensworks commented 3 years ago

Another note to self: it looks like exactQuantity was implemented in IngredientStorageHelpers in the same way as the checkStackSize flag, which leads to conflicting semantics in IT. IT interprets exactQuantity as to exact quantity of a given slot, while CC interprets it as the exact quantity across the whole storage. The IT-interpretation was in fact the intended one, and the interpretation in CC is incorrect, but serves its uses at the moment.

Since the original exactQuantity interpretation is not trivial to implement, I intend to remove the aspect setting, and open a separate issue as feature request. (unless I come up with another idea by tomorrow)

Can it be implemented purely using a predicate when using slot-based transfer?

rubensworks commented 3 years ago

TL;DR, exact quantity is removed, and the description of check stack size is improved.