xivdev / EXDSchema

A collection of schemas to define Excel data.
15 stars 11 forks source link

Linking related fields in a sheet for improved API design #13

Closed WorkingRobot closed 1 month ago

WorkingRobot commented 6 months ago

The current work on EXDSchema is amazing, but it lacks the ability to relate different fields together, a feature that SaintCoinach definitions already have.

This problem is best explained with an example: https://github.com/xivdev/EXDSchema/blob/89394343970148396493b322755fcd3804a4c633/Schemas/2024.03.27.0000.0000/ItemFood.yml#L1-L24 ItemFood is defined as a list of 3 different BaseParam stats. With EXDSchema, there is no way to relate these fields together, and this is reflected in Lumina's GeneratedSheets2. In order to loop through each stat effectively, a for loop must be used to loop through every index over every array individually. On the other hand, SaintCoinach, by design, has this relation defined: https://github.com/xivapi/SaintCoinach/blob/1d43d4427797a86fe68a8815fca3e1c88a51ec04/SaintCoinach/Definitions/ItemFood.json#L11-L37

I propose the following optional relations field to all sheets:

relations:
  - Params:
      - name: BaseParam
      - name: IsRelative
      - name: Value
      - name: Max
      - name: ValueHQ
      - name: MaxHQ
  - other_relation_here:
      - name: and_so_on

The only requirement for these related fields is that they must be an array of the same size and must be siblings to one another. Parsers that see this field can choose to ignore it, or they can place the related fields into a separate class/struct/type, where they can easily be iterated over with a single array rather than several.

WorkingRobot commented 6 months ago

Proof of concept is here, where ItemFood and SpecialShop were given relations.

As an example, here's what was added to SpecialShop's Item array:

relations:
  ReceiveItems:
    - Item
    - ReceiveCount
    - ReceiveHq
  ItemCosts:
    - ItemCost
    - CurrencyCost
    - HqCost
    - CollectabilityCost

(also viewable here)

which results in the following C# being generated:

public class ItemStruct
{
    public ReceiveItemsStruct[] ReceiveItems { get; internal set; }
    public ItemCostsStruct[] ItemCosts { get; internal set; }
    public LazyRow<SpecialShopItemCategory>[] Category { get; internal set; }
    public LazyRow<Quest> Quest { get; internal set; }
    public int[] Unknown0 { get; internal set; }
    public LazyRow<Achievement> AchievementUnlock { get; internal set; }
    public int Unknown2 { get; internal set; }
    public ushort PatchNumber { get; internal set; }
    public byte[] Unknown1 { get; internal set; }
    public class ReceiveItemsStruct
    {
        public uint ReceiveCount { get; internal set; }
        public LazyRow<Item> Item { get; internal set; }
        public bool ReceiveHq { get; internal set; }
    }
    public class ItemCostsStruct
    {
        public uint CurrencyCost { get; internal set; }
        public LazyRow<Item> ItemCost { get; internal set; }
        public ushort CollectabilityCost { get; internal set; }
        public byte HqCost { get; internal set; }
    }
}
public ItemStruct[] Item { get; internal set; }

I don't know of any use case that would benefit from the non-related fields, so it may be beneficial to rename the fields. I can also add the ability to rename fields when they get put into relations if needed.

WorkingRobot commented 5 months ago

I'd really like to get this implemented before Dawntrail, or as soon as possible after that. Are there any steps I should take or improvements I should make?