Skip to content

Conversation

@chyzman
Copy link
Contributor

@chyzman chyzman commented Dec 30, 2025

No description provided.

@chyzman chyzman changed the title add RESOURCES_LOADED event add resource reload start/end events Dec 31, 2025
/**
* Called before the client begins loading resources.
*/
public static final Event<StartResourceReload> START_RESOURCE_RELOAD = EventFactory.createArrayBacked(StartResourceReload.class, callbacks -> (client, isFirst) -> {
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be helpful to explain the isFirst parameter in the javadoc

Copy link
Member

@LambdAurora LambdAurora left a comment

Choose a reason for hiding this comment

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

The events should be in resource-loader-v1, in a new class (ClientResourceLoadingEvents), and in a future PR most likely move the data-pack events to resource loader too.

Aside from that, the events are missing important context: the resource manager.
And in the case of the end event, it's also missing an Optional<Throwable> in the case of errors, (and the event should trigger even if the resource load fails).

It's the kind of low-bandwidth event where I'd recommend creating a context object just to ensure a stable API while allowing to add more context if needs be.

@cputnam-a11y
Copy link
Contributor

cputnam-a11y commented Jan 5, 2026

Could it also include the store on success?
And yeah, context probably makes sense here

@LambdAurora
Copy link
Member

Including the store is not a bad idea, yes.

@chyzman
Copy link
Contributor Author

chyzman commented Jan 5, 2026

The events should be in resource-loader-v1, in a new class (ClientResourceLoadingEvents), and in a future PR most likely move the data-pack events to resource loader too.

im not sure exactly what package to put it in in the resource loader module, do i mimic the package structure from a typical event module?

Could it also include the store on success?

what does this mean exactly?

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.

5 participants