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

Refactor salt disk feature to call vanilla placement method. #7968

Merged

Conversation

KdotJPG
Copy link
Contributor

@KdotJPG KdotJPG commented Dec 26, 2023

Changes proposed in this pull request:

The surface salt deposit placements currently use a copy of the vanilla code. This causes them not to pick up changes made by other mods that try to break up the monotony of (or otherwise change) the shapes of the vanilla disks:

image

By reworking the disk feature in Mekanism to call the vanilla code, the changes are now picked up:

image

Without the mods in question present, Mekanism behaves as it did before:

image


public final BlockState state;
public final RuleBasedBlockStateProvider stateProvider;
public final IntProvider radius;
public final IntSupplier halfHeight;
Copy link
Contributor Author

@KdotJPG KdotJPG Dec 26, 2023

Choose a reason for hiding this comment

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

This file changed to match the vanilla config as closely as possible. The only remaining difference is the IntSupplier since I am not aware of any clean approach towards making configured_feature/salt.json pull in the Mekanism config value for halfHeight.

}
}
return placed;
ResizableDiskConfig config = context.config();
Copy link
Contributor Author

@KdotJPG KdotJPG Dec 26, 2023

Choose a reason for hiding this comment

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

If my understanding of escape analysis is correct, the following instantiations should be stack-allocated if the function is called often enough to be optimized by the JVM.

@pupnewfster
Copy link
Member

I will try to look at this when I get a chance, out of curiosity though so when I do get to testing it, what mod are you testing it with?

@KdotJPG
Copy link
Contributor Author

KdotJPG commented Dec 26, 2023

Simply Improved Terrain, 1.20(.1) alpha
https://www.curseforge.com/minecraft/mc-mods/simply-improved-terrain

All The Mods 9 (with Mekanism already present, and the above mod added)

@pupnewfster pupnewfster added this to the 10.4.2 milestone Feb 21, 2024
@pupnewfster
Copy link
Member

Love this, it is great having less duplicate code as it makes porting easier. Did a couple minor cleaning related things to this PR so that more of the stuff can be overridden by changing the configured feature rather than just changing the config, and also fixed salt spawning outside of water as it seems MC moved it from being hardcoded like our copy had it to being a placement modifier

@pupnewfster pupnewfster merged commit 12892f6 into mekanism:1.20.x Feb 21, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants