-
Notifications
You must be signed in to change notification settings - Fork 31
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
- Update geometry component #219
Conversation
} | ||
} | ||
"type": "string", | ||
"enum": ["minecraft:geometry.full_block", "minecraft:geometry.cross"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are they really enum, or can people use their own geometry 🤔
Think it might be better to use a pattern matching with regex in the case that its not an enum
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
People can use their own geometry. I'd just copied what was already there, from stirante I believe, and moved it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the enum isn't restricted to just those values I figured it was fine and it was nice to have those show up in autocompletes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"oneOf": [
{
"enum": [
"minecraft:geometry.full_block",
"minecraft:geometry.cross"
]
},
{
"pattern": "minecraft:geometry\\.(full_block|cross)|geometry\\.[a-zA-Z0-9_]+"
}
]
```
Do you think something like this would be good?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Enum are restricted to those values, so they if someone specifies something else it would error 🤔
Might have to revisit some other parts then later.
think a [a-zA-Z0-9_\.:\=]
would be good enough 😄
Now it shouldn't count under "matched multiple schemas" and the default values show when you use the string