Implementing banner

[xoft]

[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]

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]