I think you approach has a problem with duplicating a cItem. Why not simply use the Json::Value as the actual storage in cItem? That way you save two translations (FromJson and ToJson) and automatically gain easy duplicating, while still retaining universal access.
Duplication should work just fine and ToNBT is called way more often than ToJson, so not much to win there. But yeah, using json::value to store it could work.
If we change it to Json::Value, we need to add ToNBT and FromNBT to the cItemHandler.
So a quick pro/con for using Json::Value would be:
Pros:
-easier to get to the data (no casting or including)
-easier to expose to lua?
-no need for a custom copy constructor and assignment op in cItem
Cons:
-adds more overhead (slower, more memory use)
-data structure is unclear
-no type checking
I must be blind - I don't see any duplication code, and cItem uses a unique_ptr, which cannot be copied.
More pros:
- direct correspondence with parsed commands (In the future if we add the same processing as vanilla does for commands, such as "/give ... {detaildata}" )
- future-proof for additional data
As for your cons, I don't think they are that much of a problem. cItem handling is definitely not a bottleneck, so if it uses a bit more CPU, I don't care. Data structure is unclear, which might be a pro as well, since we can add more data without any trouble for other items and for future versions.
ok, I'll rework it with Json::Value, shouldn't be too much work.
And look at the copy constructor and assignment op ->
Code:
if (a_CopyFrom.GetItemMeta()) {
cItemMeta * meta = GetHandler()->MakeItemMeta();
if (meta) {
meta->FromCopy(a_CopyFrom.GetItemMeta());
SetItemMeta(meta);
}
}
I reworked the code to use Json::Value as m_Metadata, everything works as intended. Currently implementing the crafting side and was wondering how I can prevent a certain ingredient to be consumed during crafting.
I don't think there is a way as of now, you'll need to come up with something.
actually there has to be a way because the crafting recipe for cake is working correctly and it doesn't consume the bucket
Would removing the ingredient from a_Recipe->m_Ingredients work?
---
Everything works as far as I can tell (apart from retaining one item in one recipe), but there is a weird issue when crafting banners. I have to click on the result for it to become visible for some reason.
Tomorrow I will check all the recipes to make sure they all work.
I would advise against storing anything as JSON in memory, considering saves are in NBT format.
(05-24-2016, 08:00 AM)tigerw Wrote: [ -> ]I would advise against storing anything as JSON in memory, considering saves are in NBT format.
Well I didn't until xoft convinced me it would be better if I did. As for the NBT saving, that part is covered. Might I suggest a vote then? I can see the merit of both approaches, but would like to have consensus on this matter before I make any pull requests.