Improved interface for storing upgrade data
SquidDev opened this issue ยท 3 comments
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 apushToClient
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.
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
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?