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

Find and SetValue #1

Open
xbelanch opened this issue Jul 17, 2011 · 12 comments
Open

Find and SetValue #1

xbelanch opened this issue Jul 17, 2011 · 12 comments

Comments

@xbelanch
Copy link

Hi,
It'll be great to see this two functions:

  • Find a tag by name and return a pointer to this tag
  • Change the value given a pointer to a tag

Great job!

@agrif
Copy link
Owner

agrif commented Jul 17, 2011

Hello! Those are great ideas, and I'm already looking into ways to implement them. I've also got to write the in-place region writing I've had sketched in my notebook for a few months, so now is probably a good time for both.

Just to clear some things up:

  • For Find: what should happen when there's more than one tag with that name? Should it return the first one? Or should these return some sort of iterator like rs_tag_compound_iterator_*? If both forms would be useful, I can write both; I'm just looking for input. Using an iterator instead of a simple find would look something like...
RSTagIterator it;
RSTag* tag;
rs_tag_find_init(root_tag, "TagName", &it);
while (rs_tag_find_next(&it, &tag))
{
    /* do something with found tag */
}
  • The easiest way I can think of doing SetValue would be to set the value based on another RSTag*. Writing setter functions for each of the types might get a little painful, and they're basically already there with the rs_tag_new-type functions. So, something like
rs_tag_set(dest_tag, rs_tag_new(RS_TAG_STRING, "Hello, world!"));
/* or */
rs_tag_set(dest_tag, some_other_tag_you_made);

@xbelanch
Copy link
Author

Hello!

And thanks for your fast answer and amazing news if you were thinking about something similar to implement. Ok, when I was thinking in a find function given a tag name, I was simply thinking in really basic tags like "Time", "Pos" or "rainTime". The need of return a tag pointer is, basically, for read/write/print operations:

RSTag* rs_nbt_find_tag_by_name("TagName", RSTag* root);
void rs_nbt_print_tag(RSTag* tag);
void rs_nbt_set_tag(RSTag* tag, void* value);

I suppose it's really difficult to do :-( and reading your last lines of your response I can't imagine how is the easy way to merge the new/another RSTag* with the rest of the root tag...

Sorry if I can't help you :(

@agrif
Copy link
Owner

agrif commented Jul 17, 2011

Getting the "Time" or "rainTime" tags from level.dat is pretty simple right now:

RSNBT* nbt = rs_nbt_parse_from_file("level.dat");
RSTag* timetag = rs_tag_compound_get(rs_nbt_get_root(nbt), "Time");

The documentation isn't very good right now so it's possible you missed this. Still, I agree that a find function that searches the whole tree is a good idea, especially for things like "Pos" which are nested inside other tags.

As for the SetValue you were talking about, are you looking for a way to completely replace what value an RSTag* has? Or are you looking for a way to merge two tags? Replacing would be pretty easy to add (with the rs_tag_set I mentioned before), but merging is a different matter entirely.

Oh! Also, I'll look into moving the tag printing code from tools/nbttool.c into a library function.

@xbelanch
Copy link
Author

Ops! I missed it but thanks for your help! On the other hand, the function SetValue (in my opinion) is about to replace only what RSTag* has not merge (complicated, isn't?). I'll write an example.c trying to show/explain how it works (think it can help other people to use your code!).

@xbelanch
Copy link
Author

Just do it! :-D

Print the value of "Time" tag of "level.dat":

RSTag* data = rs_tag_compound_get(rs_nbt_get_root(nbt), "Data");
RSTag* timeT = rs_tag_compound_get(data, "Time");
printf("%Li\n",rs_tag_get_integer(timeT));

You must transverse the full path of "level.dat" tree to get the value of a Tag :-S (the find function claims to be recursirve :-D)

Change the value of "Time" tag of "level.dat":

rs_tag_set_integer(timeTag, 0);

Wow!

@agrif
Copy link
Owner

agrif commented Jul 20, 2011

I've just pushed some changes, mostly from ideas talked about here. I'll go over them briefly.

There's now a rs_tag_compound_get_chain that takes any number of names, letting you get to deeply-nested named tags quicker:

RSTag* timeT = rs_tag_compound_get_chain(rs_nbt_get_root(nbt), "Data", "Time", NULL);

The last argument is always NULL, to signal the end of the name list.

rs_tag_print takes two arguments: the tag to print and a FILE* to print to. It prints out the simplest representation of the tag it can; for integers, floats, and strings, it's like printing the value with %Li, %f, and %s. For byte arrays, it writes the data raw.

rs_tag_print(timeT, stdout);

The rs_tag_pretty_print function takes the same arguments, but instead it prints out a pretty, indented tree like Notch has at the end of his NBT notes. This is the same sort of output nbttool prints.

The rs_tag_find function searches a tag (recursively) for the first tag with the given name, and returns it. If no such tag exists, it returns NULL.

RSTag* timeT = rs_tag_find(rs_nbt_get_root(nbt), "Time");

For each of these, there is a similar function starting with rs_nbt_ that works on the root tag of an NBT file. So, the following are equivalent:

rs_tag_compound_get_chain(rs_nbt_get_root(nbt), "Data", "Time", NULL);
rs_tag_find(rs_nbt_get_root(nbt), "Time");
rs_nbt_get_chain(nbt, "Data", "Time", NULL);
rs_nbt_find(nbt, "Time");

Hopefully this makes it a bit more convenient to read NBT files with libredstone. :) Proper documentation for these, and the rest of libredstone, is coming.

I'm still a little confused by what you mean by SetValue being about to replace only what has not been merged... In my mind a SetValue(a, b) function would completely replace a with b.

@xbelanch
Copy link
Author

Great job!

I've played around with them and works fine! Only I've noticed one missed feature in rs_tag_compound_get_chain: what's happens when I try to read the three values of "Pos" Tag? Following the logical of function, it'll be:

RSTag* posT = rs_tag_compound_get_chain(rs_nbt_get_root(nbt), "Data", "Player", "Pos", 0,  NULL);

I know this will be a little more complex to handle and I suppose that this will produce an error because the types of values (float against RSTag*). So, which way it'll be better to handle this? I write a stupid function:

    RSTagIterator it;
    RSTag* subtag;
    float* posPlayer =  malloc(3*sizeof(float));
    rs_tag_list_iterator_init(posTag, &it);
    int i = 0;
    while (rs_tag_list_iterator_next(&it, &subtag))
    {
         posPlayer--;
        *posPlayer  = rs_tag_get_float(subtag); 
    }

and have an array of three floats, but I think it's a stupid code (I confess to you I'm not a programmer...) and I gues you'll do much better than I.

Another question is about SetValue... is much more simple than you'be been thinking of. Imagine I want to set the Time tag of level.dat to 0. Following your code is not necessary reinventing the wheel because it's straightforward do the next:

RSTag* timeT = rs_tag_compound_get_chain(rs_nbt_get_root(nbt), "Data", "Time", NULL);
rs_tag_set_integer(timeT, 0);

@xbelanch
Copy link
Author

Following the comment before, I've found this manner to acquire the three values of Pos tag:

RSTag* posT = rs_tag_compound_get_chain(rs_nbt_get_root(nbt), "Data", "Player", "Pos", NULL);
RSTag* pos_x = rs_tag_list_get(posT, 0);
RSTag* pos_y = rs_tag_list_get(posT, 1);
RSTag* pos_z = rs_tag_list_get(posT, 2);

Runs well only for the first element of the list, but fails with the other two... Is this the right way?

@xbelanch
Copy link
Author

Forget the two last messages: I was wrong (last message) because I haven't been using rs_tag_list_iterator_next to move the pointer to the next item and (the penultimate message) I was messed with pointers. The next code works fine:

float* get_pos_player(RSTag* self){
    RSTagIterator it;
    RSTag* subtag;
    float* posPlayer =  malloc(3*sizeof(float));
    float* pt = posPlayer;

    rs_tag_list_iterator_init(self, &it);
    while (rs_tag_list_iterator_next(&it, &subtag))
    {
        *posPlayer = rs_tag_get_float(subtag);
        posPlayer++;
    }
    return pt;
}

@agrif
Copy link
Owner

agrif commented Jul 24, 2011

Okay cool! You're most recent version of your posPlayer code was actually my suggested fix for your first. Your second example (with rs_tag_list_get) should have worked though.

I thought about letting rs_tag_compound_get_chain handle list indexes too, but that would make it impossible to get an entire list without indexing it. Since NULL is usually defined as zero, it's hard to tell if a 0 passed in means "end of list" or "zero index".

Wow, I was overthinking the SetValue. As long as you don't change what type of tag it is, you can already change the value of a tag (exactly like your example with rs_tag_set_integer). You can't change a tag type yet, though I'll keep that idea in the back of my mind in case it's ever needed.

@xbelanch
Copy link
Author

The question about handle list indexes will probably resolve with a rs_tag_compound_list_get_chain where the last argument is the index value of list. In this way, you pass the NULL to determine the end of the chain and, at last, the index value:

RSTag* posX = rs_tag_compound_list_get_chain(rs_nbt_get_root(nbt), "Data", "Player", "Pos", NULL, 0); // returns the first value of "Pos" tag list

If chain tag is not a list, the index value not cause any effect or, to do safer, add an assert to evaluate if the last tag is really a list. I think it's a bit ugly function... so, I don't know if it's really necessary if rs_tag_list_getworks fine.

On another hand... it's possible to know the biome value (arid, tropical, polar,...) where the player is walking around?

@agrif
Copy link
Owner

agrif commented Jul 25, 2011

Unfortunately, the biome info isn't stored with the level like the lighting and block data. This is actually a huge annoyance for mappers (such as another project I work on, Minecraft-Overviewer).

Right now if you want to get biome data you'll either need to use a bukkit plugin like JSONAPI, or a Java program that interfaces directly with Minecraft, such as Minecraft Biome Extractor.

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

No branches or pull requests

2 participants