short term storage of entitites

[12xx12]

So i have a few features that require some short term storage for entities.

TigerW suggested to store this in NBT. There was the idea to recycle the functions in NBTChunkSerializer and WSSAnvil.

BUT

The functions to decode NBT data are protected members of cWSSAnvil. We'd need a way to access functions.

I'd suggest making the decode functions from cWSSAnvil static.

What are your thoughts?

[tigerw]

So this has come to a head, we have PRs
http://github.com/cuberite/cuberite/pull/4616
http://github.com/cuberite/cuberite/pull/4179
http://github.com/cuberite/cuberite/pull/5015
http://github.com/cuberite/cuberite/pull/5120
effectively blocked on a decision about how to implement item NBT (http://minecraft.gamepedia.com/Player.da..._structure).

Item NBT needs to store a couple of custom structures, but notably also block entities in their entirety, and entities (I think also in their entirety).

The current solution of adding more fields to cItem unnecessarily inflates the size of the class, since most of the time the extra fields (cFireworkItem/Damage/Lore/etc.) aren't used.

We have discussed multiple solutions to this, mainly
1) Storing the extra data as JSON in cItem
2) Storing as a std::string containing serialised NBT (and using WSSAnvil to deserialise)
3) Storing as a new NBT class, representing a NBT tree using std::variant + map

I think we should use 3) but in the interests of type-safety, ease of API export, and fast access, hardcode the classes that the variant can contain. cItem would then look like

class cItem
{
  ItemType Item;
  unsigned char Count;
  std::vector<std::variant<Damage, DisplayProperties, BlockEntityTags, std::unique_ptr<cBlockEntity>, ...> Properties;
};

There is a bit of a problem with storing block entities and entities, as 12xx12 discussed. cItems are copyable, so we need a way to copy the abstract cEntity/cBlockEntity classes, ideally without manually copying all the fields as cBlockEntity::CopyFrom currently does. I think it should be possible to provide a Clone function for both entities and block entities; since they both store their type we can switch on it, cast the abstract pointer to a concrete type, and make_unique a new instance invoking the copy constructor.

Another complication is protocol. Serialised item NBT representation changes across versions and we do not want to be re-implementing (block) entity NBT conversion multiple times. The other stuff like DisplayProperties (lore, custom names) are relatively small so are still manageable. Thankfully the client doesn't really need to know the full NBT of most things, so we just need to send it the subset of information that makes the UI show correctly. This should be waaay less work.

When these changes are implemented we will get a big reduction in cItem's size, and it should be faster to construct. This also means everything relying on item NBT can also be implemented; ctrl+mclick (http://github.com/cuberite/cuberite/issues/3682), shulker boxes (http://github.com/cuberite/cuberite/issues/4838) and more.

[NiLSPACE]

Solution 3 seems like the way to go. I'm a bit worried about backwards compatibility though. Would it be possible to add deprecated bindings for everything that this solution would replace?

[tigerw]

I understand it's possible to bind field accesses, probably following what cBlockInfo does. The existing cItem::m_XXX stuff shouldn't be broken, agreed.