add v10 support

[dt-12345]

added support for the main v7 features and reading for up to v10 (I didn't add the v10 non-container root nodes though)
wasn't entirely sure how to handle the file nodes (0xa2) so I ended up creating a new struct (also renamed byml::Hash to byml::Dictionary as we have proper hash nodes now)
tested on several files from totk and everything seemed to be fine

---

This change is 

[dt-12345]

added updated strings for totk aamp files (I figured it's at least somewhat relevant)

[leoetlino]

ooh nice! thanks for the pull request, I never got around to doing this

I'll review this later this week

[leoetlino]

Reviewed 4 of 6 files at r1, 2 of 2 files at r2, 2 of 2 files at r3.
Reviewable status: all files reviewed (commit messages unreviewed), 7 unresolved discussions (waiting on @dt-12345)

---

readme.rst line 15 at r3 (raw file):

* `AAMP <https://zeldamods.org/wiki/AAMP>`_ (binary parameter archive): Only version 2 is supported.
* `BYML <https://zeldamods.org/wiki/BYML>`_ (binary YAML): Versions 1-4 are fully supported, version 5-10 have partial support.

could you document what is or isn't supported?

---

src/byml.cpp line 63 at r3 (raw file):

  String = 0xa0,
  Binary = 0xa1,
  File = 0xa2,

can we call this BinaryWithAlignment to keep this consistent with 0xa1?

---

src/byml.cpp line 254 at r3 (raw file):

      const auto type = m_reader.Read<NodeType>(types_offset + i);
      const auto hash = m_reader.Read<u32>(offset + 4 + 8 * i);
      result.emplace(*hash, ParseContainerChildNode(offset + 8 + 8 * i, type.value()));

Suggestion:

hash.value()

---

src/byml.cpp line 381 at r3 (raw file):

      return;
    case Byml::Type::File:
      writer.Seek(util::AlignUp(data.GetFile().align, writer.Tell() + 8) - 8);

hmm, are you sure this is correct? intuitively it feels more natural to write the size and alignment first, then align, and then finally write the data, but I haven't looked at the code that reads aligned binary data so what I'm suggesting might not be possible

---

src/byml_text.cpp line 126 at r3 (raw file):

      auto hash = Byml::Hash32{};
      for (const auto& child : node) {
        u32 key = std::stoul(child.key().data(), nullptr, 16);

stoul/stoull aren't safe to use here because the string data isn't null-terminated. can you convert the key string view into a string first?

---

src/byml_text.cpp line 134 at r3 (raw file):

      auto hash = Byml::Hash64{};
      for (const auto& child : node) {
        u64 key = std::stoull(child.key().data(), nullptr, 16);

ditto

---

src/byml_text.cpp line 184 at r3 (raw file):

        [&](const std::vector<u8>& v) {
          const std::string encoded =
              absl::Base64Escape(absl::string_view((const char*)v.data(), v.size()));

reinterpret_cast<const char*>(v.data())

[dt-12345]

src/byml.cpp line 381 at r3 (raw file):

      return;
    case Byml::Type::File:
      writer.Seek(util::AlignUp(data.GetFile().align, writer.Tell() + 8) - 8);

hmm, are you sure this is correct? intuitively it feels more natural to write the size and alignment first, then align, and then finally write the data, but I haven't looked at the code that reads aligned binary data so what I'm suggesting might not be possible

the parsing code for getting the data is at 0x7101213a3c for totk 1.2.1 and it appears to expect the data to follow directly after the alignment value (this can be confirmed by looking at any of the files in the Effect folder that use this node type)

[dt-12345]

Reviewable status: 3 of 8 files reviewed, 7 unresolved discussions (waiting on @leoetlino)

---

readme.rst line 15 at r3 (raw file):

Previously, leoetlino (Léo Lam) wrote…

could you document what is or isn't supported?

Done.

---

src/byml.cpp line 63 at r3 (raw file):

Previously, leoetlino (Léo Lam) wrote…

can we call this BinaryWithAlignment to keep this consistent with 0xa1?

Done.

---

src/byml.cpp line 254 at r3 (raw file):

      const auto type = m_reader.Read<NodeType>(types_offset + i);
      const auto hash = m_reader.Read<u32>(offset + 4 + 8 * i);
      result.emplace(*hash, ParseContainerChildNode(offset + 8 + 8 * i, type.value()));

Done.

---

src/byml_text.cpp line 126 at r3 (raw file):

Previously, leoetlino (Léo Lam) wrote…

stoul/stoull aren't safe to use here because the string data isn't null-terminated. can you convert the key string view into a string first?

Done.

---

src/byml_text.cpp line 134 at r3 (raw file):

Previously, leoetlino (Léo Lam) wrote…

ditto

Done.

---

src/byml_text.cpp line 184 at r3 (raw file):

Previously, leoetlino (Léo Lam) wrote…

reinterpret_cast<const char*>(v.data())

Done.

[leoetlino]

mostly lgtm, just some minor nits. please run clang-format on the files that have been modified in this pull request too

Reviewed 5 of 5 files at r4, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @dt-12345)

---

src/byml_text.cpp line 126 at r3 (raw file):

Previously, dt-12345 wrote…

Done.

nit: you can use RymlSubstrToStrView to turn the rapidyaml substring into a std::string_view, and then pass that to std::string (which has a constructor overload that takes a string_view).

---

src/byml_text.cpp line 188 at r4 (raw file):

        },
        [&](const BinaryWithAlignment& v) {
          yml::LibyamlEmitter::MappingScope scope{emitter, "!file", YAML_BLOCK_MAPPING_STYLE};

maybe we should use !aligned_binary or something like that

[dt-12345]

Reviewable status: 4 of 8 files reviewed, 2 unresolved discussions (waiting on @leoetlino)

---

src/byml_text.cpp line 126 at r3 (raw file):

Previously, leoetlino (Léo Lam) wrote…

nit: you can use RymlSubstrToStrView to turn the rapidyaml substring into a std::string_view, and then pass that to std::string (which has a constructor overload that takes a string_view).

Done.

---

src/byml_text.cpp line 188 at r4 (raw file):

Previously, leoetlino (Léo Lam) wrote…

maybe we should use !aligned_binary or something like that

Done.
I also added a check for !!file since I noticed that's what other tools used for this node

[leoetlino]

Reviewed 4 of 4 files at r5, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @dt-12345)

---

src/byml_text.cpp line 188 at r4 (raw file):

Previously, dt-12345 wrote…

Done.
I also added a check for !!file since I noticed that's what other tools used for this node

!file or !!file?

[dt-12345]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @leoetlino)

---

src/byml_text.cpp line 188 at r4 (raw file):

Previously, leoetlino (Léo Lam) wrote…

!file or !!file?

roead and nx-editor's BymlLibrary both use !!file with two !'s

[leoetlino]

I'm not quite sure why this fails to build on macOS: https://github.com/zeldamods/oead/actions/runs/9832406488/job/27186862601?pr=36#step:6:717

It looks like adding Hash32 and Hash64 to the variant suddenly makes the compiler think that std::pair<u32/u64/const std::string, oead::Byml> aren't copy constructible even though Byml should be copy constructible :|

[leoetlino]

lgtm, thanks again for implementing this!

I'm not sure what to do about the macOS build failure - I don't see an obvious explanation for the compile error (and the fact it builds fine with MSVC and Ubuntu's GCC kinda suggests it might be a compiler bug)

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @dt-12345)

---

src/byml_text.cpp line 188 at r4 (raw file):

Previously, dt-12345 wrote…

roead and nx-editor's BymlLibrary both use !!file with two !'s

oh ok, was just wondering because a previous version of this PR used !file

[dt-12345]

hmm that's odd, I don't know much about macOS so I'm not sure how to resolve that

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @dt-12345)