Implementing banner

[xoft]

I think buckets are handled specially by the crafting code. There's no generic support for this, however, and it would be nice to have one.

@tigerw Why not? The Json::Value object is not *that* large, is universal and anything we may cook up to replace it will be a reinvented wheel. Why not use something that is proven to work?

[tigerw]

Because Minecraft uses NBT.

[tigerw]

and besides, I would hardly call "storing data in member variables" reinventing wheels

[Seyaku]

I don't think the fact that minecraft uses NBT is relevant in this matter. Fact is we need a generic solution for storing any kind of metadata for items, which can be converted to both NBT and Json.

Some ways of doing that are:
-Add a storage class (like I did before)
-Use Json::Value (like I'm doing now)
-Add new fields for all items that have extra metadata (like what is done with fireworks, but personally I think this is a really bad design and is more of a quick hack)
-Store as raw data (could be NBT data or a struct)

[tigerw]

I mentioned NBT to discourage further use of JSON in on-disk storage, as per FS#359. I agree that in memory, adding fields to cItem which does not bear relevance to most items is unsustainable.

I propose that, for the latter, some form of native method (i.e. not JSON or NBT) should be used (preferably a class containing member variables and a ToNBT function, see cFireworkItem), since methods of serialisation or encoding for storage is unsuitable for working data.

[Seyaku]

Before using Json I made this class, to be implemented by each item handler that needs custom metadata.

/** Storage class for item meta data */
class cItemMeta
{
public:
  virtual ~cItemMeta() {}

  /** Load from NBT source. */
  virtual void FromNBT(const cParsedNBT & a_NBT) = 0;

  /** Load from copy. */
  virtual void FromCopy(const cItemMeta * a_Meta) = 0;

  /** From JSON */
  virtual void FromJSON(const Json::Value & a_Value) = 0;

  /** Write to NBT */
  virtual void ToNBT(cFastNBTWriter & a_Writer) = 0;

  /** To JSON */
  virtual void ToJSON(Json::Value & a_OutValue) = 0;

  /** Is equal to */
  virtual bool IsEqual(cItemMeta * a_ItemMeta) = 0;
};

[xoft]

@tigerw Minecraft uses NBT for storage, but Json for commands. So using Json altogether is not that far-fetched after all, it saves one conversion.

I don't like how firework data is implemented, because it would mean that each item would sooner or later get its own data member in cItem.

[tigerw]

Nevertheless, I would argue that it would be strange to be accessing, manipulating, and working with a data structure represented in JSON when it can be effectively done in a pure C++ way.

I would suggest that the class inheritance proposed by Seyaku would work well, though I don't think the To/FromJSON functions will be used anywhere.

[Seyaku]

Ok, did some final testing and tweaking and everything appears to work properly. Once a decision has been made regarding the metadata, I will implement that and make it ready for a pull request.

[PureTryOut]

@Seyaku are you still working on this? I have not yet seen a PR from you.