Skip to content

Make grs setFormula method behave in the same way as ceu ones #2772

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 3 commits into from
Apr 27, 2025

Conversation

MCTian-mi
Copy link
Contributor

@MCTian-mi MCTian-mi commented Apr 21, 2025

What

Make the setFormula method expansion from grs behave in the same way as ceu's. (i.e. returns the material instance instead of void return.

Implementation Details

Simply changes method returns to the material instead of nothing.

Outcome

In be4 this kind of code works in java but not in a grs script (since it considers groovy mixin'd methods first):

LEU235UraniumDioxide = new Material.Builder(8661, SuSyUtility.susyId('leu_235_uranium_dioxide'))
                .dust()
                .iconSet(SAND)
                .components(LEU235, Oxygen * 2)
                .color(0x1d2618)
                .build()
                .setFormula("UO2", true) // This retuens the material in Java, but null in grs

instead one needs to use:

LEU235UraniumDioxide = new Material.Builder(8661, SuSyUtility.susyId('leu_235_uranium_dioxide'))
                .dust()
                .iconSet(SAND)
                .components(LEU235, Oxygen * 2)
                .color(0x1d2618)
                .build()

LEU235UraniumDioxide.setFormula("UO2", true)

This PR makes the former form of code possible via grs.

Potential Compatibility Issues

None that I could think of?

@MCTian-mi MCTian-mi requested a review from a team as a code owner April 21, 2025 07:45
@Zorbatron Zorbatron added the type: bug Something isn't working label Apr 21, 2025
Copy link
Contributor

@brachy84 brachy84 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Im wondering why these exist at all when they just delegate to gt

Copy link
Member

@bruberu bruberu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since there is probably plenty of time before this would actually go in-game, there are a couple other methods that could be added to this PR, such as setIconSet, setHarvestLevel, setBurnTime, addToolEnchantment, addScaledToolEnchantment, setToolStats (all), setOverrideToolStats, and setBlastTemp.

@bruberu
Copy link
Member

bruberu commented Apr 21, 2025

OK, I've done precisely that.

@ALongStringOfNumbers ALongStringOfNumbers merged commit 1f69eb4 into GregTechCEu:master Apr 27, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants