Port of the Forestry compatibility from 1.12.2#4940
Conversation
|
|
If there is code in this PR, it isn't showing up because the mass of 4000 removed files is hiding it. |
|
Just like you said, the code was hiding behind the JSONs. It's fixed now, and I’ve added a new exclusiveContent block |
screret
left a comment
There was a problem hiding this comment.
All of the model files for comds should be generated at runtime like tag prefix items do.
| patchouli = { module = "vazkii.patchouli:Patchouli", version.ref = "patchouli" } | ||
| forestry = { module = "maven.modrinth:forestry-community-edition", version.ref = "forestry" } | ||
| modkit = { module = "com.github.thedarkcolour:ModKit", version.ref = "modkit" } |
There was a problem hiding this comment.
could you match the indents with the other entries above this?
| try { | ||
| for (Field field : GTBeesSpecies.class.getDeclaredFields()) { | ||
| if (field.getType() == ResourceLocation.class) { | ||
| ResourceLocation location = (ResourceLocation) field.get(null); | ||
| String speciesName = location.getPath(); | ||
| String speciesNameCap = speciesName.substring(0, 1).toUpperCase() + speciesName.substring(1); | ||
| provider.add("allele.forestry.bee_species.gtceu." + speciesName, speciesNameCap); | ||
| } | ||
| } | ||
| } catch (IllegalAccessException e) { | ||
| e.printStackTrace(); | ||
| } |
There was a problem hiding this comment.
WTF?? no this is absolutely not how this should be done.
also, if at all possible, don't duplicate the translations for the materials.
There was a problem hiding this comment.
maybe don't add the class at all if it's empty
| @FeatureProvider | ||
| public class GTApicultureItems { | ||
|
|
||
| public static final IFeatureRegistry REGISTRY = ModFeatureRegistry.get(new ResourceLocation(GTCEu.MOD_ID, "core")); |
There was a problem hiding this comment.
You should use GT's registrate instead of whatever this is. you'll get the same end result but without having to duplicate sot much stuff and/or make it jank.
There was a problem hiding this comment.
Seeing as how these are (almost) all based on materials, maybe make "comb" a material property?
There was a problem hiding this comment.
- rename to
GTBeeDefinitions - Make this use a material property instead of hardcoding all of it, please.
There was a problem hiding this comment.
if you end up getting rid of the GTBeeSpecies class with all the IDs, this should be renamed GTBeeSpecies and not GTBeeDefinition because IBeeDefinition no longer exists in Forestry CE
There was a problem hiding this comment.
you're probably referring to the reflection done in GTForestryLangProvider, and they shouldn't use this class for that purpose. however, a class with a list of species IDs is what Forestry does to avoid littering the code with string literals you have to hunt down if the IDs ever change.
There was a problem hiding this comment.
if you do end up dropping this class, GTBeeDefinition should be GTBeeSpecies since there's no more IBeeDefinition in Forestry CE
There was a problem hiding this comment.
IMO the ids shouldn't be stored as constants outside the bee species' constructors/registration
| public static final String GENUS_GT_ORGANIC = "gt_organic"; | ||
| public static final String GENUS_GT_INDUSTRIAL = "gt_industrial"; | ||
| public static final String GENUS_GT_ALLOY = "gt_alloy"; | ||
| public static final String GENUS_GT_GEM = "gt_gem"; | ||
| public static final String GENUS_GT_METAL = "gt_metal"; | ||
| public static final String GENUS_GT_RAREMETAL = "gt_raremetal"; | ||
| public static final String GENUS_GT_RADIOACTIVE= "gt_radioactive"; | ||
| public static final String GENUS_GT_NOBLEGAS = "gt_noblegas"; |
There was a problem hiding this comment.
seconded, why not something more creative?
|
|
||
| // CREDIT GOES TO: christopherwalkerml | | ||
| // https://github.com/christopherwalkerml/MoBees/blob/main/src/main/java/com/noodlepfp/mobees/item/MoreBeesItemHiveFrame.java | ||
| public class GTItemHiveFrame extends ItemForestry implements IHiveFrame { |
There was a problem hiding this comment.
| public class GTItemHiveFrame extends ItemForestry implements IHiveFrame { | |
| public class GTHiveFrameItem extends ItemForestry implements IHiveFrame { |
| .setMutationMult(1.2f) | ||
| .setSpeedMult(1.8f) | ||
| .setAgeMult(0.9f) | ||
| .build(), "frame_accelerated"); |
There was a problem hiding this comment.
Make the items' IDs match their language keys, e.g. this should be accelerated_frame, FRAME_MUTAGENIC's id should be mutagenic_frame, etc.
There was a problem hiding this comment.
agreed. it's a bad pattern that Forestry has that i plan on abandoning in 1.21.1
There was a problem hiding this comment.
you're probably referring to the reflection done in GTForestryLangProvider, and they shouldn't use this class for that purpose. however, a class with a list of species IDs is what Forestry does to avoid littering the code with string literals you have to hunt down if the IDs ever change.
There was a problem hiding this comment.
if you do end up dropping this class, GTBeeDefinition should be GTBeeSpecies since there's no more IBeeDefinition in Forestry CE
| public static final String GENUS_GT_ORGANIC = "gt_organic"; | ||
| public static final String GENUS_GT_INDUSTRIAL = "gt_industrial"; | ||
| public static final String GENUS_GT_ALLOY = "gt_alloy"; | ||
| public static final String GENUS_GT_GEM = "gt_gem"; | ||
| public static final String GENUS_GT_METAL = "gt_metal"; | ||
| public static final String GENUS_GT_RAREMETAL = "gt_raremetal"; | ||
| public static final String GENUS_GT_RADIOACTIVE= "gt_radioactive"; | ||
| public static final String GENUS_GT_NOBLEGAS = "gt_noblegas"; |
There was a problem hiding this comment.
seconded, why not something more creative?
| modBus.addListener(ForestryClientHandler::registerItemColors); | ||
|
|
||
|
|
||
| ModuleUtil.getModBus(ForestryConstants.MOD_ID).addListener(EventPriority.HIGHEST, ((ForestryClientApiImpl) IForestryClientApi.INSTANCE)::initializeTextureManager); |
There was a problem hiding this comment.
there's no reason. AI tends to do this, i've also seen it in a vibecoded AI slop Forestry addon
| .setMutationMult(1.2f) | ||
| .setSpeedMult(1.8f) | ||
| .setAgeMult(0.9f) | ||
| .build(), "frame_accelerated"); |
There was a problem hiding this comment.
agreed. it's a bad pattern that Forestry has that i plan on abandoning in 1.21.1
| import java.text.DecimalFormat; | ||
| import java.util.List; | ||
|
|
||
| import javax.annotation.Nullable; |
There was a problem hiding this comment.
a holdover from old Forestry. they should have paid more attention to the GT code style.
| import java.util.function.Consumer; | ||
|
|
||
| @ForestryModule | ||
| public class GTForestryModule implements IForestryModule { |
There was a problem hiding this comment.
why are you reregistering Forestry's client code?? get rid of this.
| GTBeeDefinition.register(apiculture); | ||
| } | ||
|
|
||
|
|
There was a problem hiding this comment.
this big gap probably needs reformatting too
| minecraft = "1.20.1" | ||
| forge = "47.3.0" | ||
| minecraftForge = "1.20.1-47.3.0" | ||
| forge = "47.4.10" |
There was a problem hiding this comment.
is this safe to bump?
There was a problem hiding this comment.
I didn’t encounter any issues with the version bump. That’s why I specified in my original PR that the version was bumped. I did this because Forestry forces you to use a newer version of Forge, but I would expect that this PR will need further checks
There was a problem hiding this comment.
And for ForestryClientHandler I didn’t use AI because I didn’t find any documentation about Java add-ons. I used Mo' Bees as a base to see how it should be implemented. But yeah, I will probably get down to fix it between today and tomorrow. If there is any documentation that I missed, don’t hesitate to share it.
There was a problem hiding this comment.
If you'd like some help with this, come ask in the Forestry CE Discord. I haven't written documentation for everything yet, but I do have an addon template: https://github.com/thedarkcolour/ForestryAddonTemplate/tree/main
There was a problem hiding this comment.
if you end up getting rid of the GTBeeSpecies class with all the IDs, this should be renamed GTBeeSpecies and not GTBeeDefinition because IBeeDefinition no longer exists in Forestry CE
Co-authored-by: screret <68943070+screret@users.noreply.github.com>
What
Port of the Forestry compatibility from 1.12.2
Implementation Details
All bees, combs, and frames from the 1.12.2 version have been ported.
The datagen related to these objects has been left disconnected for now, as I was unsure of the best way to integrate it into the current setup.
A service file was added because Forestry CE uses Java's ServiceLoader to discover and load addons.
Recipes for comb processing and frames still need to be implemented.
Forge was updated to a newer version to ensure compatibility with the latest Forestry CE update, as the previously used version was missing content required by the port.
[] No AI driven tools were used for this pull request.
Yes AI driven tools were used for this pull request.
Agent Used
ClaudeCode Opus 4.7
AI was used to write the tedious parts, such as the ResourceLocation entries for GTBeesSpecies
How Was This Tested
The mod was built and tested in a clean Minecraft instance, both with and without Forestry installed, and worked as intended