CC: Tweaked

CC: Tweaked

42M Downloads

Pocket upgrades do not attach when crafted

SquidDev opened this issue ยท 0 comments

commented

Minecraft Version

1.20.1/1.21.1

Version

1.112+

Details

One of the nastier things about pocket computers is keeping the ItemStack in-sync with the public IPocketAccess. For instance, if someone calls IPocketUpgrade.setUpgradeData, we need to update the ItemStack too.

Historically, we treated the ItemStack as the source-of-truth, and changes to upgrade data immediately got written back to the ItemStack. However, this made lots of assumptions that we still had a reference to the correct item stack, that I wasn't happy with.

In ed0b156, I changed IPocketAccess to be the source of truth, with upgrade data getting written back when the stack is ticked. This is still technically incorrect (for instance, upgrade data isn't updated if the computer is still running and in a chest).

One of the (intended) side-effects of this change, is that setting the upgrade directly on the stack does not update the computer's peripheral. However, I had forgotten that as of #1888, we now preserve the computer instance throughout crafting, meaning the upgrade is no longer set until the computer is destroyed and recreated (see #1956 for the original discussion).

I think there's a couple of options here:

  • Don't preserve the computer instance when crafting: This is the easiest fix.

    I do have some worries here. We can't stop the previously running computer (as assemble isn't guaranteed to be called on the server), so there will be a small window (5s) where multiple computers with the same ID are running. This means people could create files larger than the disk quota. There's limited exploitability here, but it's still not great.

  • Find a better way of handling keeping IPocketAccess and the ItemStack in sync. I don't have a good solution here. We may have to redesign the interface IPocketAccess exposes, so that you can't update upgrades/upgrade data at arbitrary times.

I did think about storing upgrade data outside the item stack, but I think that's a no-go due to how recipes work. Alas.