Implementing banner - Printable Version +- Cuberite Forum (https://forum.cuberite.org) +-- Forum: Cuberite (https://forum.cuberite.org/forum-4.html) +--- Forum: Development (https://forum.cuberite.org/forum-13.html) +--- Thread: Implementing banner (/thread-2451.html) |
RE: Implementing banner - xoft - 05-24-2016 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? RE: Implementing banner - tigerw - 05-24-2016 Because Minecraft uses NBT. RE: Implementing banner - tigerw - 05-24-2016 and besides, I would hardly call "storing data in member variables" reinventing wheels RE: Implementing banner - Seyaku - 05-25-2016 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) RE: Implementing banner - tigerw - 05-25-2016 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. RE: Implementing banner - Seyaku - 05-25-2016 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; }; RE: Implementing banner - xoft - 05-25-2016 @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. RE: Implementing banner - tigerw - 05-25-2016 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. RE: Implementing banner - Seyaku - 05-25-2016 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. RE: Implementing banner - PureTryOut - 11-14-2016 @Seyaku are you still working on this? I have not yet seen a PR from you. |