Quilted Fabric API (QFAPI) / Quilt Standard Libraries (QSL)

Quilted Fabric API (QFAPI) / Quilt Standard Libraries (QSL)

441k Downloads

Confusing behaviour of phase ordering

SquidDev opened this issue ยท 2 comments

commented

Attempting to add a phase ordering based on the default phase results in confusing behaviour.

Reproduction steps

Create a test mod with the following code

var phase = new ResourceLocation("example", "late");
ServerLifecycleEvents.SERVER_STARTED.addPhaseOrdering(Event.DEFAULT_PHASE, phase);
ServerLifecycleEvents.SERVER_STARTED.register(phase, server -> {
  System.out.println("Server started (late)");
});

ServerLifecycleEvents.SERVER_STARTED.register( server -> {
  System.out.println("Server started (default)");
});

One would expect to see the following output:

Server started (default)
Server started (late)

However, you instead see the opposite:

Server started (late)
Server started (default)

Additional notes

I believe this is caused by both Quilt and Fabric defining their own default phase ordering (quilt:default and fabric:default respectively).

When registering an event, it uses Quilt's default phase, but adding a phase ordering with Event.DEFAULT_PHASE uses, intuitively, Fabric's default phase. This means the resulting sorted phase list of the event looks like [fabric:default, example:late, quilt:default].

Some possible fixes I can see:

  • Remove the separate Fabric phase, just making it equal to Quilt's. This might break people hard coding fabric:default, though I'd hope folks aren't doing that!
  • Replace Fabric's DEFAULT_PHASE with Quilt's inside QuiltCompatEvent.addPhaseListener and QuiltCompatEvent.register.
commented

Thank you! I don't know if it's worth using quiltifyPhases in register too?, though I guess people calling register explicitly passing the default phase is less common.

commented

Oh wait, you do have a point; After all, here, Murphy's Law is pretty much active: If something can go wrong, it will go wrong; So yeah, if we can make sure that something is more compatible, then we should (impl details are an exception since they tend to be a deep rabbit hole that can lead to nowhere)