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

Fix a Specific Case Issue of Generics in Injected Interfaces (when more than one type parameter are provided by the Target Class) #1177

Merged
merged 2 commits into from
Sep 24, 2024

Conversation

FirstMegaGame4
Copy link
Contributor

@FirstMegaGame4 FirstMegaGame4 commented Sep 24, 2024

I discovered while wanting to use the second type parameter of net.minecraft.state.State<O,S> in a injected interface, that the second check I made in #1050 was not properly working. In fact, only the first type parameter of the target class was properly read, meaning that if an injected interface wanted to use another type parameter than the first one the check would fail.

This was entirely my fault, I wrongly used SignatureVisitor#visitEnd as I thought this one would be called once SignatureReader would finish to read the signature of the target class.

I also added a test case clearly showing that the issue was solved in the pull request.

@FirstMegaGame4 FirstMegaGame4 changed the title Fix a Specific Case Issue of Generics in Injected Interfaces (more than one type parameter provided by the Target Class) Fix a Specific Case Issue of Generics in Injected Interfaces (case when more than one type parameter is provided by the Target Class) Sep 24, 2024
@FirstMegaGame4 FirstMegaGame4 changed the title Fix a Specific Case Issue of Generics in Injected Interfaces (case when more than one type parameter is provided by the Target Class) Fix a Specific Case Issue of Generics in Injected Interfaces (when more than one type parameter are provided by the Target Class) Sep 24, 2024
@modmuss50 modmuss50 merged commit 5b44b25 into FabricMC:exp/1.8 Sep 24, 2024
89 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