xwingtmg / xwing-data2

An easy-to-use collection of data and images from X-Wing: The Miniatures Game (Second Edition) by Fantasy Flight Games.
MIT License
98 stars 67 forks source link

value of "???" for cost fix. #605

Closed pakirby1 closed 1 year ago

pakirby1 commented 1 year ago

I've noticed that some of the cost fields are set to "???" for the value key. The cost should be an Int type but it is set to "???". Why is this and what should the Int value for these fields be set to?

{ "name": "Delta-7B", "limited": 0, "xws": "delta7b", "sides": [ { "title": "Delta-7B", "type": "Configuration", "slots": ["Configuration"], "grants": [ { "type": "stat", "value": "agility", "amount": -1 }, { "type": "stat", "value": "shields", "amount": 2 }, { "type": "stat", "value": "attack", "arc": "Front Arc", "amount": 1 } ], "ffg": 548, "image": "https://squadbuilder.fantasyflightgames.com/card_images/en/515903e04a0d06a9200860698326896d.png", "artwork": "https://squadbuilder.fantasyflightgames.com/card_art/6530f18639b7efff5a5a659589e5d65c.jpg", "text": "The Delta-7B was designed as a heavier variant of the Delta-7 Aethersprite-class Interceptor, identifiable by the repositioned astromech slot. Many Jedi Generals favor this craft's greater firepower and durability." } ], "cost": { "value": "???" }, "restrictions": [{ "ships": ["delta7aethersprite"] }], "standard": false, "extended": false, "epic": false },

danrs commented 1 year ago

In the case of D7B there's no valid cost as the card is banned, so I guess -1 would make sense. Feel free to raise a PR with this change

pakirby1 commented 1 year ago

Maybe something other than -1, otherwise people would gain an extra point possibly. This isn't a big deal, since the squad builders prevent people from using this card.

danrs commented 1 year ago

Good point - how about 999?

chrisalleng commented 1 year ago

Seems like a good argument for null as the value? I have the same question for adding the starter set and endor pilots that are in the process of being spoiled.

pakirby1 commented 1 year ago

The problem is that it should be an int value (not null).

On Tue, May 2, 2023 at 11:46 AM chrisalleng @.***> wrote:

Seems like a good argument for null as the value? I have the same question for adding the starter set and endor pilots that are in the process of being spoiled.

— Reply to this email directly, view it on GitHub https://github.com/guidokessels/xwing-data2/issues/605#issuecomment-1531976645, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABNXW4PRE7AFKES3HKBB3XLXEFI7HANCNFSM6AAAAAAVE75QRQ . You are receiving this because you authored the thread.Message ID: @.***>

chrisalleng commented 1 year ago

I've been trying to find some best practices for JSON for situations like this that I can, and unsurprisingly haven't found any good references. I don't think using a sentinel value here is a good idea, as we'd need to find some other way to represent that the value is effectively illegitimate somewhere else in the project.

As an example, if I were building a card browser, and used XWS to populate all of my data, every time I sorted by cost I'd see Delta 7B at the top (or the bottom), because it would have either some negative cost, or some maximal cost higher than any ship's loadout value. The developer obviously doesn't want to display this data that is incorrect, so now they have to manually filter that card out anyways.

A cleaner solution is probably just not having the cost name on Delta-7B entirely, just as the standard loadout upgrades do not have any cost? The goal of XWS is to be a reference specification of the game data itself, and Delta 7B doesn't have a cost.

pakirby1 commented 1 year ago

For my app, the pilot cost is a required Int. The upgrade cost is a nullable Int. I think if we make the pilot cost to be nullable that should fix the issue. Consumers of the xws would need to update their apps to support the nullable field.

On Tue, May 2, 2023 at 2:20 PM chrisalleng @.***> wrote:

I've been trying to find some best practices for JSON for situations like this that I can, and unsurprisingly haven't found any good references. I don't think using a sentinel value here is a good idea, as we'd need to find some other way to represent that the value is effectively illegitimate somewhere else in the project.

As an example, if I were building a card browser, and used XWS to populate all of my data, every time I sorted by cost I'd see Delta 7B at the top (or the bottom), because it would have either some negative cost, or some maximal cost higher than any ship's loadout value. The developer obviously doesn't want to display this data that is incorrect, so now they have to manually filter that card out anyways.

A cleaner solution is probably just not having the cost name on Delta-7B entirely? The goal of XWS is to be a reference specification of the game data itself, and Delta 7B doesn't have a cost.

— Reply to this email directly, view it on GitHub https://github.com/guidokessels/xwing-data2/issues/605#issuecomment-1532167872, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABNXW4LTC7JVBKHYXWDJUYLXEF3DPANCNFSM6AAAAAAVE75QRQ . You are receiving this because you authored the thread.Message ID: @.***>

s1n commented 1 year ago

Cost would be an optional property in that case.

As an alternative, you could separate cost, slots, restrictions, and keywords to separate files denoting the rules of the game format. That is, standard would lack this pilot, even if they have a definition, thereby making them banned in that format. Currently, you have format membership defined as a binary properties. That would separate the information vut give you the ability to define formats, format specification, and rule sets similar to the old FFG squad builder.

pakirby1 commented 1 year ago

I agree on the optional property.

Is minimizing the impact on the squad builder devs a concern? Or are they fairly adept at making changes to their apps based on the XWS definition?

On Tue, May 2, 2023 at 3:00 PM Jason Switzer @.***> wrote:

Cost would be an optional property in that case.

As an alternative, you could separate cost, slots, restrictions, and keywords to separate files denoting the rules of the game format. That is, standard would lack this pilot, even if they have a definition, thereby making them banned in that format. Currently, you have format membership defined as a binary properties. That would separate the information vut give you the ability to define formats, format specification, and rule sets similar to the old FFG squad builder.

— Reply to this email directly, view it on GitHub https://github.com/guidokessels/xwing-data2/issues/605#issuecomment-1532208857, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABNXW4JOLEK2CKG3VRZZAYDXEF7WHANCNFSM6AAAAAAVE75QRQ . You are receiving this because you authored the thread.Message ID: @.***>

chrisalleng commented 1 year ago

I don't think there are any squadbuilders currently using XWS for anything other than import/export functionality, both YASB and LBN have different internals.

@danrs if you're cool with it, I can go ahead and remove the cost on 7B on the giant open PR to save you a versioning update as well

danrs commented 1 year ago

Sure, go for it

On Mon, 8 May 2023 at 11:26 am, chrisalleng @.***> wrote:

I don't think there are any squadbuilders currently using XWS for anything other than import/export functionality, both YASB and LBN have different internals.

@danrs https://github.com/danrs if you're cool with it, I can go ahead and remove the cost on 7B on the giant open PR to save you a versioning update as well

— Reply to this email directly, view it on GitHub https://github.com/guidokessels/xwing-data2/issues/605#issuecomment-1537686751, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABLOVLN4E5JVWNFNYJQUF23XFBRU7ANCNFSM6AAAAAAVE75QRQ . You are receiving this because you were mentioned.Message ID: @.***>

danrs commented 1 year ago

Fixed in #610