search

Low-Drag-MC / Shimmer

A mod that integrates my passion for rendering
MIT License
71 stars 22 forks source link

Forge API Events aren't fired on startup and improvements to the shimmer api/format #48

Closed justliliandev closed 1 year ago

justliliandev commented 1 year ago

Minecraft Version

1.19.2

ModLoader

Forge

Shimmer version

Shimmer-1.19.2-0.1.14

Modpack info or mod list(please reduce mod range)

shimmer with any mod that tries to receive the events

The latest.log file and hs_err_pidXXX if exists

not required

optifine , Rubidium , flywheel or any rendering related mods. mod version is required.

none

Issue description

The ForgeShimmerLoadConfigEvent and the ForgeShimmerReloadEvent is fired during the startup of the game. The ForgeEventBus only starts to fire events after the game has full loaded. To get the events fired the client has to force a resource Reload after the game has started (using F3+T or other means). Changing the EventBus to the ModEventBus or a shimmer specific EventBus is a solution to this problem.

Steps to reproduce

  1. Create Forge Mod Project containing shimmer as a dependency
  2. Listen to the event
  3. Check if you receive the event via debugger or print statement or something else

Other information

I would also recommend following improvements to the api for a simpler usage:

  1. automatic file finding for all mods under modid/assets/shimmer.json so that mods don't have to use a codedepency at all for simple static effects
  2. add a helper to the event so that the event can also accept just a resourcelocation to read from, so that mods don't have to add the code to read the file and put it in a string themselves, as that is just copied code from already existing code in the Configuration class
  3. add a DataGenerator to create those json files
  4. the documentation says to add java coded lights during the FMLClientSetup Event, however at that point the reloadlisteners haven't run yet and the Maps in LightManager will be cleared. The Documentation should be changed to add them during the ShimmerReloadEvent as that's after those maps have been cleared

I'm willing to make those changes and create a PR, but I need a decision on the EventBus that's used

zomb-676 commented 1 year ago

the pr #49 has be reviewed If you think the final change is OK than I will merge that pr and close this issue

justliliandev commented 1 year ago

Yep, those changes are good, thanks