-
Notifications
You must be signed in to change notification settings - Fork 306
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 ore and worldgen #3662
base: master
Are you sure you want to change the base?
Conversation
Changing to use dimension name will probably break FE's multiworld since you can name a dimension. This would effectively defeat the purpose of not using hardcoded dims since you would not be able to create a multiworld with GT ores in it again... |
@spacebuilder2020 I've never used forge essentials, but it looks like it'll be fine as long as the multiworld uses the proper provider. It determines what ores to generate based on the provider's dimension name, so if you make a multiworld with the nether provider it'll generate everything normally. |
ok, I thought it was based on the world name. If it is the provider dim name then it is probably fine, but I want to know what your reasoning for changing it to the name? |
End asteroids: |
These are incompatible with NEID and can cause problems with blocks whose metadata can go over 127.
this must not be merged until VP is made compatible again |
Just to make sure, does this require a sister PR of some sort for dreamcraft or any other mod which has compat for GT ores? |
@YannickMG There are a few outstanding problems, but they're just a matter of moving a few lines over to the new API. They'll still crash the game though. I'm working on VP currently. https://github.com/GTNewHorizons/Draconic-Evolution/blob/2835b7a54eb902dd274e25de7f2a4a994a1f77dd/src/main/java/com/brandon3055/draconicevolution/integration/ModHelper.java#L113 Plus the handful of references to sBlockOres1. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an amazing improvement that the pack codebase sorely needs if we want to keep being able to evolve the pack. Due to the size of this PR (each bullet point in your PR description could have been it's own PR) I cannot meaningfully review every change to the logic, but thankfully this system is critical enough that we should find any hidden issue soon enough.
What I can do is review changes to existing APIs as well as review the usability and readability of new APIs. I also pointed changes that seemed odd, inconsistent or confusing.
I hope to help ensure new APIs are as usable as possible and make our codebase as clear and maintainable as can be so that if/when a bug is found in the code that was added, it is as simple as possible to resolve it.
src/main/java/bartworks/system/material/BWItemMetaGeneratedOre.java
Outdated
Show resolved
Hide resolved
src/main/java/gtneioreplugin/plugin/gregtech5/PluginGT5VeinStat.java
Outdated
Show resolved
Hide resolved
- Misc minor clean up - Removed several unused or useless methods and parameters - Encapsulated several fields behind getters - Made several public fields private where possible - Used generic interface for collections where possible - Used new (Immutable)BlockMeta types and removed old gcgreg classes that the new types replace - Removed replaceable block system in gcgreg to prevent duplication of responsibility, since that's handled by the new stone type system now - Fixed mehen belt asteroid gen (dim name was wrong) - Changed asteroid gen to use murmurhash to prevent ore banding (old bespoke hash algorithm was terrible) - Couldn't use google hash lib because those objects can't be reset - this code is in a hot-path, so allocations need to be as low as possible - Added custom pair type for detectPrefix - Added ice and clay stones for Seth to increase oregen (not needed due to void miners, but it looks nicer now) - Added @Nullable/@nonnull for several methods - Added oregen event for visual prospecting, to remove its mixin (along with a new GT event bus) - Added lots of comments to critical methods - Changed singleton enums to proper final class instance singletons - Changed ore hardness/explosion resistance to reflect backing stone hardness - Changed GT++ ores to use the adapter drop logic (I forgot to change this before) - Removed old ambiguous BaseOreComponent constructor
Can you add EFR's deepslate as another stone type for OW ores? |
If we're doing that I think I also saw the ore generate in tuff? Not sure if it should. |
Summary:
Todo:
Changes: