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

Let's Talk: Constructor Injection #26

Open
LemmaEOF opened this issue Nov 21, 2019 · 8 comments
Open

Let's Talk: Constructor Injection #26

LemmaEOF opened this issue Nov 21, 2019 · 8 comments

Comments

@LemmaEOF
Copy link

The new 1.15 snapshots move almost all of MinecraftClient's setup stuff into the constructor. This is a massive hassle for any mod that wants to add its own resource pack stuff, which includes Fabric API. Fabric API has resorted to a @Redirect, but this means that nobody else can use that point, since @Redirect is collidable. The reason we have this problem is because @Inject only allows injection to the RETURN of constructors. Do we want to open that up to any other injection points? The main worry is preventing anything from being put before a super call.

@liach
Copy link

liach commented Nov 21, 2019

fyi mixin has a check to allow certain types of injections to be done after super calls; it generally groups injections to performable before super, after super, only in regular method body (these 3 types)

@Chocohead
Copy link

Injecting before super is called is perfectly possible if the callback is forced to be static. In fact it can be quite useful want wanting to modify a parameter passed up to the superclass as it avoids needing to catch every instance of the constructor being used. Not sure if Mixin properly supports this for all injection types though (similar to it allowing redirecting <init> which leaves uninitialised references about).

For what it's worth Rift always allowed you to tinker in constructors as you could in any other method and it never had problems with people doing stupid stuff. Mixin either lets you and is careful enough to avoid problems or crashes immediately (which is as much down to bugs within Mixin as it is safety features).

@LemmaEOF
Copy link
Author

yeah I'm super in favor of more opened init injections tbh, it'd make things a lot easier just in general

@liach
Copy link

liach commented May 2, 2020

@Boundarybreaker #30 has been made and works well. Feel free to peek over there.

@Devan-Kerman
Copy link

Devan-Kerman commented Jul 12, 2020

if you want to touch go nuts
I haven't tried, because in my opinion there's not enough reason to stray from "you can redirect, etc. but you can only @Inject at RETURN or you can make a custom injection point to circumvent that"
that imo is enough, because it forces you to think before you break the rule, but still makes the rule fundamentally breakable if you are determined to play with fire

the enforcement of a static mixin will need a PR though, idk if liach's thing already does it or not

@liach
Copy link

liach commented Jul 12, 2020

My pr only supports regular injects after super call, not static injects passed to super calls

@valoeghese
Copy link

did 536170a fix this

@liach
Copy link

liach commented Oct 14, 2021

probably

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 a pull request may close this issue.

5 participants