Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add an NBT tree structure #4179

Open
wants to merge 20 commits into
base: master
Choose a base branch
from
Open

Conversation

peterbell10
Copy link
Member

This is the first step in fixing #3682 and adding support for storing arbitrary NBT in cItem.

@bibo38
Copy link
Contributor

bibo38 commented Feb 9, 2018

Just for the naming I would prefer NBTTree, because I thought first, that you would add NBT data to a tree 🤣 .

@peterbell10
Copy link
Member Author

The name is meant to complement cFastNBT and cParsedNBT.

src/WorldStorage/TreeNBT.cpp Outdated Show resolved Hide resolved
src/WorldStorage/TreeNBT.h Outdated Show resolved Hide resolved
@12xx12
Copy link
Member

12xx12 commented Aug 9, 2020

@peterbell10 are you still working on this? May I help you?

I'd like to use the NBT tags with my banner pull request

@peterbell10
Copy link
Member Author

As far as I remember this works as-is. It just never got reviewed.

@bearbin
Copy link
Member

bearbin commented Aug 10, 2020

Time for a review then, I'll try rebasing and have a go this evening.

@12xx12
Copy link
Member

12xx12 commented Oct 18, 2020

So I rebased on master.

There were two words which required attention in MVSC.
The tests are not done with the new test "engine"

I fixed this but i'm not quite shure if I should commit.

In all I think the code should be merged

12xx12
12xx12 previously requested changes Oct 18, 2020

void clear() { m_Tags.clear(); }

void swap(cList & a_Other) NOEXCEPT
Copy link
Member

@12xx12 12xx12 Oct 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
void swap(cList & a_Other) NOEXCEPT
void swap(cList & a_Other) noexcept

MSVC doesn't like this. Didn't check other. Yeah GCC doesn't like this either

m_Tags.swap(a_Other.m_Tags);
}

friend void swap(cList & a_Lhs, cList & a_Rhs) NOEXCEPT
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
friend void swap(cList & a_Lhs, cList & a_Rhs) NOEXCEPT
friend void swap(cList & a_Lhs, cList & a_Rhs) noexcept

same here

tests/WorldStorage/TreeNBTTest.cpp Show resolved Hide resolved
@12xx12
Copy link
Member

12xx12 commented Oct 18, 2020

Another thing: we should enable the Packet writer and the NBT write to write those tags into themselves for easy transimission and saving

@tigerw
Copy link
Member

tigerw commented Oct 18, 2020

Bearbin and I took a look at this and we came to the conclusion that this was a reimplementation of std::variant.

Something like this may suffice.

#include <cassert>
#include <iostream>
#include <map>
#include <variant>
#include <vector>

class NBT
{
public:

	using List = std::vector<NBT>;
	using Compound = std::map<std::string, NBT>;

	using NubbyTree = std::variant<
		int,
		bool,
		List,
		Compound
	>;

	NBT(NubbyTree canopy)
	{
		top = canopy;
	}

	auto & Expose() const
	{
		return top;
	}

	void Poosh(NBT nubby)
	{
		assert(std::holds_alternative<List>(top));

		auto & lIsT = std::get<List>(top);
		assert(
			lIsT.empty() ||
			lIsT.back().top.index() == nubby.top.index()
		);

		lIsT.emplace_back(std::move(nubby));
	}

private:  // its privates

	NubbyTree top;
};

void MakeNubby()
{
	NBT Root{ NBT::List() };
	Root.Poosh({ 1 });
	Root.Poosh({ 2 });

	for (const auto & Item : std::get<NBT::List>(Root.Expose()))
	{
		std::cout << std::get<int>(Item.Expose()) << '\n';
	}
}

int main()
{
    MakeNubby();
}

@12xx12
Copy link
Member

12xx12 commented Oct 18, 2020

Yeah I just noticed that also that you use a variant and make a nice interface

@12xx12
Copy link
Member

12xx12 commented Oct 18, 2020

So what do we do now? I'd like to have the NBT for items added for other projects (Banner, Potions,...)

@12xx12 12xx12 dismissed their stale review October 18, 2020 10:01

standart did provide better option

@dImrich dImrich mentioned this pull request Feb 6, 2021
9 tasks
@PureTryOut
Copy link
Contributor

Has there been any progress on this recently?

@12xx12
Copy link
Member

12xx12 commented Aug 17, 2021

Check the discussion in #5015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants