Skip to content

More Mixin Fixes #2471

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

Merged
merged 2 commits into from
May 25, 2025
Merged

Conversation

LlamaLad7
Copy link
Contributor

@LlamaLad7 LlamaLad7 commented May 25, 2025

Fixes mixin shadow autocomplete results often being prioritized over regular java ones
Also adds much better support for injection point specifiers.
The latter required some decently big reworks to the CollectVisitor system.
During that I greatly improved the (previously nonexistent due to bugs) message about which filters were responsible for the failure.
Happy to debate any of the refactors if you disagree with them.

Also as a side note, I keep my commit history clean, so it would be preferable for my PRs not to be squashed.

Adding our elements one by one lets some of them get frozen above the normal suggestions. Instead, we should add them all in a batch.
@Earthcomputer
Copy link
Member

The InjectionPoint code is API, so we need to make sure our API consumers, notably MixinSquared, are aware of these changes

@LlamaLad7
Copy link
Contributor Author

As in I should inform them / make a PR there, or try to maintain bin compat?

@Earthcomputer
Copy link
Member

Earthcomputer commented May 25, 2025

Trying to maintain binary compat is the best, but not at the expense of keeping our own codebase maintainable. We don't have many API consumers so we can get away with breaking binary compat as long as we inform the major ones (I can only think of MixinSquared tbh)

@LlamaLad7
Copy link
Contributor Author

I can inform them yeah, they don't have any injection points anyway so should be minor

@LlamaLad7 LlamaLad7 force-pushed the fix/mixin-fixes-2 branch from 258c2ee to 3881882 Compare May 25, 2025 15:32
@LlamaLad7 LlamaLad7 requested a review from Earthcomputer May 25, 2025 15:32
This includes several related things like:
- Rework `CollectVisitor`s to work with `Sequence`s rather than having a special case for only wanting the first result
- Rework `CollectResultFilter`s to filter based on the whole sequence (this is needed to support `:LAST`)
- Offer proper completion options for specifiers in the right context
- Fix `At#value` completions disappearing after typing `:`
@LlamaLad7 LlamaLad7 force-pushed the fix/mixin-fixes-2 branch from 3881882 to a9c3fab Compare May 25, 2025 16:40
@LlamaLad7
Copy link
Contributor Author

The MixinSquared plugin needs no changes.

@Earthcomputer Earthcomputer merged commit 8f2c118 into minecraft-dev:dev May 25, 2025
4 checks passed
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