Skip to content

Comments

add setOreBlock() to API#2480

Closed
Minepolz320 wants to merge 2 commits intoGTNewHorizons:masterfrom
Minepolz320:master
Closed

add setOreBlock() to API#2480
Minepolz320 wants to merge 2 commits intoGTNewHorizons:masterfrom
Minepolz320:master

Conversation

@Minepolz320
Copy link
Contributor

@Dream-Master
Copy link
Member

@Minepolz320 you need to run spotless

@Minepolz320
Copy link
Contributor Author

@Dream-Master Whoops 😅

@Dream-Master Dream-Master requested a review from a team January 28, 2024 17:29
@YannickMG
Copy link
Contributor

Isn't GT_TileEntity_Ores::setOreBlock already public static and thus usable directly in Twilight Forest? Is the idea that you'd prefer to use a method that's explicitly part of the API?

@Minepolz320
Copy link
Contributor Author

@YannickMG damn it's actually even easier

@Minepolz320
Copy link
Contributor Author

@YannickMG yes but how can you allow the mod to run without gregtech
just prevent the method from being called or so that the jvm does not load the class

I'm a bit of a noob when it comes to mod integration.

@Minepolz320
Copy link
Contributor Author

and yet it seems to me more obvious to have this method in the API
but maybe that's just me

@Caedis
Copy link
Member

Caedis commented Jan 29, 2024

Any mod specific code needs to be guarded with Loader.isModLoaded(modID).

The output of that method should be cached in a public static variable during preinit so that its not constantly checking if a mod is loaded.

@YannickMG
Copy link
Contributor

You could also consider not adding any dependency on gregtech.

Instead you could add an API to Twilight Forest to allow overriding the code used to set the ore for each stalactite and call that API from NewHorizonsCoreMod.

I can detail this tonight if you'd like.

@Minepolz320
Copy link
Contributor Author

@YannickMG

Isn't GT_TileEntity_Ores::setOreBlock already public static and thus usable directly in Twilight Forest? Is the idea that you'd prefer to use a method that's explicitly part of the API?

I think this is the best option for now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants