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

Double nested repeat support #8

Open
akuseru opened this issue May 25, 2020 · 6 comments
Open

Double nested repeat support #8

akuseru opened this issue May 25, 2020 · 6 comments

Comments

@akuseru
Copy link
Contributor

akuseru commented May 25, 2020

https://github.com/ufx/SaintCoinach/blob/master/SaintCoinach/Definitions/Quest.json#L341-L355

more crazy definitions that are missing.

@NotAdam
Copy link
Owner

NotAdam commented May 25, 2020

Yep, and there can be groups in these or groups of these too, it's a huge mess. Fixing the groups would require them to have a name at least, and then it could be cleaned up a bit but it's pretty disgusting.

I'm not really sure on a good approach for handling these as its all kind of everywhere

@goaaats
Copy link
Collaborator

goaaats commented May 25, 2020

How about just parsing it into a fixed-size array, I think that's what SC does

@NotAdam
Copy link
Owner

NotAdam commented May 26, 2020

You can't for groups, because a group might repeat say 20 times and have several members, which would look more like

struct BaseParam
{
    uint kind;
    uint value;
} params[6];

Which isn't something you can do sequentially as is without sacrificing what the data actually is

@NotAdam
Copy link
Owner

NotAdam commented May 26, 2020

I think it's not too much of a problem if the groups were named, because then you could generate structures for all of them, but as is, it's kind of just meaningless

@NotAdam
Copy link
Owner

NotAdam commented Jun 8, 2020

Double nested repeat isn't done, nor is crazy shit like this:

{
  "sheet": "GCSupplyDuty",
  "definitions": [
    {
      "type": "repeat",
      "count": 11,
      "definition": {
        "type": "repeat",
        "count": 3,
        "definition": {
          "type": "group",
          "members": [
            {
              "name": "Item",
              "converter": {
                "type": "link",
                "target": "Item"
              }
            },
            {
              "name": "ItemCount"
            }
          ]
        }
      }
    }
  ]
}

But the generator has been completely rewritten and correctly supports:

  • Arrays
  • Groups (needs more work but what's there is a decent start)
  • Sheet references, lazily loaded
  • Any combinations of the above, except groups with sheet references
  • bools without returning junk

bitsets are gone and are just read by their columns now instead, easier that way

this is currently on develop, will merge into master and publish to nuget probably tomorrow or the day after once I fix a couple weird things

main thing to note: generated structs have moved to their own nuget package, lumina.generated, potentially changing namespace too but not 100% committed to that yet

@NotAdam
Copy link
Owner

NotAdam commented Jun 9, 2020

fyi: all of these changes outlined above are available on nuget now

I'm still conflicted on whether double repeat and whatnot is even worth supporting, it's probably going to be more reasonable to manually implement those sheets as required

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

3 participants