CC: Tweaked

CC: Tweaked

42M Downloads

Improved interface for storing upgrade data

SquidDev opened this issue ยท 3 comments

commented

This is effectively copied from my comments in #873. This issue attempts to address its needs and those in #847:

  • Capabilities on turtles. Aside, do we want capabilities on turtle item stacks too? It might be cool if turtles could, for instance, act as tanks in item form.
  • Persisting data when turtles are broken/placed (#847).
  • Handling additional NBT data on the upgrade ItemStack (we also need to handle doing this in a crafting grid).

I think the first two of these are relatively easy to solve in a cohesive manner by adding the following:

  • interface TurtleUpgradeData extends ICapabilityProvider, INBTSerializable<Tag> { }
  • public TurtleUpgradeData ITurtleUpgrade.createData(ITurtleAccess, TurtleSide).
  • public TurtleUpgradeData ITurtleAccess.getUpgradeData(TurtleSide).

I think we probably also need a generic ITurtleAccess.setChanged() method which just calls TileEntity.setChanged().

Basically upgrades can define a class which implements TurtleUpgradeData and stores data about their upgrade (such as a fluid tank's contents or a list of waypoints). Those can then be persisted via INBTSerializable<Tag> on both the turtle block entity and item stack.

createData will be called before createPeripheral, so you can then access the constructed object when constructing your peripheral, rather than accessing it every method call.

ICapabilityProvider also allows this interface to expose capabilities if it wants, though those are not required.

As far as saving/reading to the upgrade item, perhaps we add two additional methods to TurtleUpgradeData which are basically serializeToStack(ItemStack) (or ItemStack serializeToStack()) and deserializeFromStack(ItemStack)?

Example

class FluidData implements TurtleUpgradeData {
  private final ITurtleAccess turtle;
  private final FluidTank current = new FluidTank();

  @Override
  public deserializeFromStack(ItemStack stack) {
    current.readFromNBT(stack.getCompoundTag().get(FLUID_TANK_TAG)); // TODO: more error checking please
  }

  // TODO: Storage and capabilities
}

class FluidUpgrade implements ITurtleUpgrade {
  @Override
  public TurtleUpgradeData createUpgradeData(ITurtleAccess turtle, TurtleSide side) {
    return new FluidData(turtle);
  }

  @Override
  public IPeripheral createPeripheral(ITurtleAccess turtle, TurtleSide side) {
    return new FluidPeripheral((FluidData)turtle.getUpgradeData(side));
  }
}

We also need to handle syncing data to the client. I guess there's a few options here:

  • Add toClientTag/fromClientTag from methods to the data object, and add a pushToClient method as you mention. I think it makes sense to avoid sending the whole store to the client when the turtle is a BlockEntity.

  • Use getUpgradeNBTData/updateUpgradeNBTData for now (as this is synced to the client). Then when 1.17 drops rename them to something client specific and change the semantics so it's not persisted by the turtle.

I'm not quite sure what I prefer here. I think it's the first, but we'll see.

commented

And one more additional, sorry me for this.

Is this possible to add general extra turtle data, not related to updates?

For usage example, it can help to implement additional fuel turtle system, like https://plethora.madefor.cc/cost-system.html.

This is possible and without storing data inside turtle, but if we talk about storing data in turtle, this could be some addition to task

commented

One thing I overlooked here is that there's no ITUrtleAccess when the turtle is in item form.

This means using deserializeFromStack to get upgrade data from the item wouldn't be possible when upgrading the turtle via crafting, which is not ideal. I wonder if an alternative is to make TurtleUpgradeData a plain-old data object, and then just expose a getCapability(...) function like #873. Was kinda hoping to avoid that, but don't see an alternative :/.

Thoughts?

commented

Maybe we can just reduce scope of task to data object without capability tricks? It's sounds cool but not quite intuitive and can break things