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

MixinExtras: Add handler signature support for expressions. #2244

Conversation

LlamaLad7
Copy link
Contributor

@LlamaLad7 LlamaLad7 commented Feb 25, 2024

There are still 2 things to address, but you'd probably be better placed to implement them than me.

  1. It doesn't handle int-like types. Ideally the user would be presented with a dropdown of the suitable types so they can choose. All int-like types within the handler should match (at least with the current possible targets). If that is too much to ask then it should probably just treat them as int types, but I do worry the users would get confused when they expected e.g. a char and have to change it themselves.
  2. Not a direct result of this PR but the name suggestion code suggests duplicate names for parameters of the same type, which causes issues. This is noticeable e.g. when wrapping a comparison.

(I don't want this to target dev but I don't seem to have a choice. It can be manually merged. Review the latest commit only.)

@github-actions github-actions bot changed the base branch from mixinextras-expression to dev February 25, 2024 18:03
Copy link

New PRs should target the dev branch. The base branch of this PR has been automatically changed to dev.
Please check that there are no merge conflicts.

@LlamaLad7 LlamaLad7 changed the base branch from dev to mixinextras-expression February 25, 2024 18:03
@github-actions github-actions bot changed the base branch from mixinextras-expression to dev February 25, 2024 18:04
Copy link

New PRs should target the dev branch. The base branch of this PR has been automatically changed to dev.
Please check that there are no merge conflicts.

@LlamaLad7
Copy link
Contributor Author

...

@RedNesto
Copy link
Contributor

Erm I guess this should only trigger when targeting a release branch

@Earthcomputer Earthcomputer changed the base branch from dev to mixinextras-expression March 7, 2024 20:00
@LlamaLad7 LlamaLad7 force-pushed the feature/mixinextras-expression-handler-signatures branch from 7ed79bf to b0d0fcc Compare March 7, 2024 20:08
@LlamaLad7 LlamaLad7 force-pushed the feature/mixinextras-expression-handler-signatures branch from b0d0fcc to 11bad2c Compare March 7, 2024 20:08
@Earthcomputer Earthcomputer merged commit ba6b6b5 into minecraft-dev:mixinextras-expression Mar 7, 2024
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.

None yet

3 participants