Implementing banner
#11
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.
Reply
Thanks given by:
#12
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
Reply
Thanks given by:
#13
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.
Reply
Thanks given by:
#14
Smile 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);
        }
    }
Reply
Thanks given by:
#15
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.
Reply
Thanks given by:
#16
I don't think there is a way as of now, you'll need to come up with something.
Reply
Thanks given by:
#17
@Seyaku actually there has to be a way because the crafting recipe for cake is working correctly and it doesn't consume the bucket
Reply
Thanks given by:
#18
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.
Reply
Thanks given by:
#19
I would advise against storing anything as JSON in memory, considering saves are in NBT format.
Reply
Thanks given by:
#20
(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.
Reply
Thanks given by:




Users browsing this thread: 2 Guest(s)