sandstone-mc / sandstone

Sandstone | Next Generation Framework for Minecraft
https://sandstone.dev/
MIT License
174 stars 15 forks source link

Incorrect `Advancement` generic type inference #150

Closed GrantGryczan closed 1 year ago

GrantGryczan commented 2 years ago

Example:

Advancement('test', {
    criteria: {
        a: {
            trigger: 'minecraft:tick'
        },
        b: {
            trigger: 'minecraft:tick'
        }
    },
    requirements: ['a']
});

This should be inferred as Advancement<'a' | 'b'>, not Advancement<'a'>. Otherwise it thinks b is not a valid criterion.

image

Worst case, this should be fixed by removing this type parameter and accepting any string[] in requirements.

MulverineX commented 1 year ago

A: I remove autocomplete from requirements and allow people to add criteria that aren't used (when requirements is in use)

B: I keep it as is (autocompletes, errors if a criterion isn't used, I wish I could make it warn instead)

C: Continue to wrangle TS and maybe give auto-complete to A somehow I'm gonna go with B for now. especially since you can get around it easily now with RawResource

GrantGryczan commented 1 year ago

(Worth noting I think a type assertion on requirements would probably be a better way around it than RawResource.)

I can't think of any use case for not specifying all criteria in requirements currently, so if it's complicated to fix this then it's not worth it unless someone finds a real use case in the future.