Skip to content

Conversation

@silasary
Copy link
Contributor

No description provided.

@silasary silasary mentioned this pull request Oct 16, 2024
@FuzzyGamesOn
Copy link
Contributor

FuzzyGamesOn commented Oct 18, 2024

Easy enough. We'll also need a related PR in Builder to support users importing this, since it's one of the fields that Builder does currently support.

@nicopop
Copy link
Contributor

nicopop commented Nov 18, 2024

Don't forget to add that to the json schema

nicopop
nicopop previously approved these changes Dec 10, 2024
@FuzzyGamesOn
Copy link
Contributor

I can't approve this until there's a Builder PR to support the classification field, since using it instead of the existing classification keys would cause worlds to not import correctly to Builder.

@silasary
Copy link
Contributor Author

silasary commented Jan 2, 2025

Weirdly enough, the builder already supports the classification field.

Internally, the builder uses item["classification"] instead of all the bools. And the way it loads the items, if that field is prepopulated, it'll just roll with the value.

A round trip through the builder converts the item to the old format, but it works well enough for my liking.

@FuzzyGamesOn
Copy link
Contributor

It's awesome that it sorta supports it, though I somewhat disagree on the quality bar. But that's kinda irrelevant because I realized...

#105 added support for multiiple item classifications, which I believe was started and completed after this PR was started. So this PR would need to be updated to similarly support multiple item classifications to ensure that it can have the same information as the separate fields.

Also realized we didn't get #105 to update Builder to support it. So no reason I should require any level of support for this one, either. Just feature parity in core is fine.


I swear I'm not trying to find things to hold this PR up. Just unfortunate timing on this one 😅

@FuzzyGamesOn FuzzyGamesOn added the blocked by feedback This PR is blocked by feedback in the PR label Apr 17, 2025
@FuzzyGamesOn
Copy link
Contributor

Ran back through this because of the label. Looks like it's good to run with once it supports multiple values, to be consistent with the current multiple classification support in separate keywords.

@FuzzyGamesOn FuzzyGamesOn removed the blocked by feedback This PR is blocked by feedback in the PR label Oct 16, 2025
@FuzzyGamesOn FuzzyGamesOn requested a review from nicopop December 3, 2025 23:01
nicopop

This comment was marked as outdated.

@silasary silasary requested a review from nicopop December 6, 2025 09:00
nicopop

This comment was marked as resolved.

@silasary silasary requested a review from nicopop January 2, 2026 04:54
@nicopop

This comment was marked as resolved.

nicopop

This comment was marked as resolved.

@silasary silasary requested a review from nicopop January 19, 2026 03:28
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

Successfully merging this pull request may close these issues.

3 participants