Skip to content
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

Enable BlockStatePredicate list sizes of 1 #935

Merged
merged 1 commit into from
Jan 21, 2025

Conversation

johanfriis
Copy link
Contributor

@johanfriis johanfriis commented Jan 4, 2025

Description

This fixes an issue with calling BlockStatePredicate.wrap with a list of size 1.

Example Script

With existing implementation, b1 is an empty list, while b2 is not

const b1 = BlockStatePredicate.wrap(['minecraft:oak_log']).getBlockIds();
const b2 = BlockStatePredicate.wrap(['minecraft:oak_log', 'minecraft:birch_log']).getBlockIds();

console.log(b1);
console.log(b2);

I removed the check for list size > 1 completely, rather than change it to list size > 0, as there should never be a case where list size is < 0, and the empty case is handled in the other branch.

Let me know if you would prefer there to be an explicit check for size > 0 and I can update the PR

Other details

With existing implementation, `b1` is an empty list, while `b2` is not

```
const b1 = BlockStatePredicate.wrap(['minecraft:oak_log']).getBlockIds();
const b2 = BlockStatePredicate.wrap(['minecraft:oak_log', 'minecraft:birch_log']).getBlockIds();

console.log(b1);
console.log(b2);
```

I removed the check for list size > 1 completely, rather than change it to list size > 0, as there should never be a case where list size is < 0, and the empty case is handled in the other branch.

Let me know if you would prefer there to be an explicit check for size > 0 and I can update the PR
@LatvianModder LatvianModder merged commit e8beb89 into KubeJS-Mods:main Jan 21, 2025
@johanfriis johanfriis deleted the patch-1 branch January 21, 2025 23:34
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.

2 participants