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

Restore MTEHatchEnergy inventory size + description constructor #4000

Merged
merged 1 commit into from
Feb 27, 2025

Conversation

@serenibyss serenibyss requested a review from a team February 26, 2025 19:14
@serenibyss serenibyss added the chore Buildscript update, dep update, adaptation to trivial breaking changes, etc. label Feb 26, 2025
@boubou19
Copy link
Member

Can't twist update its code to use other constructors? GT5U is NH only, so if it has no use in NH...

@serenibyss
Copy link
Member Author

serenibyss commented Feb 26, 2025

Can't twist update its code to use other constructors? GT5U is NH only, so if it has no use in NH...

It can't, the PR removing this constructor removed the only way for energy hatches to set their inventory size, and since it is passed up to set a final array (not wrapped with a getter), I'm not aware of any way to do this even with reflection since afaik on newer java VMs, using things to modify a final field with reflection doesn't work well. So the only option would be mixin or asm, which to me feels unnecessary when the change is so trivial on our end

@boubou19
Copy link
Member

Can't twist update its code to use other constructors? GT5U is NH only, so if it has no use in NH...

It can't, the PR removing this constructor removed the only way for energy hatches to set their inventory size, and since it is passed up to set a final array (not wrapped with a getter), I'm not aware of any way to do this even with reflection since afaik on newer java VMs, using things to modify a final field with reflection doesn't work well. So the only option would be mixin or asm, which to me feels unnecessary when the change is so trivial on our end

It seems weird to me that we managed to remove the whole need for this constructor in NH but Twist can't... Anyways, explicit the comment about why and where it's needed, needed in an addon definitely will not age well. After this, let's merge it

@Dream-Master Dream-Master merged commit 27f57bf into master Feb 27, 2025
5 of 6 checks passed
@Dream-Master Dream-Master deleted the mte-hatch-energy-ctor branch February 27, 2025 05:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Buildscript update, dep update, adaptation to trivial breaking changes, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants