Skip to content

PlayerEvent.PLAYER_ADVANCEMENT works differently on Fabric and NeoForge #654

@teddyxlandlee

Description

@teddyxlandlee

What happened

I tried to listen a hidden advancement (whose display is absent, e.g. minecraft:recipes/transportation/acacia_boat) using PlayerEvent.PLAYER_ADVANCEMENT.

Expected behavior

It can be listened on both Fabric and NeoForge.

Actual behavior

It can be listened on Fabric, but not on NeoForge.

Platform

  • Minecraft: 1.21.8
  • Architectury: 17.0.8
  • Fabric: Latest Fabric API/Loaderr
  • NeoForge: 21.8.36

Code analysis

While architectury-fabric mixins the listener on its own, architectury-neoforge, however, uses the wheel provided by NeoForge API, causing different effects between platforms.

This is PlayerAdvancements.award(AdvancementHolder, String) (mojmaps):

	public boolean award(AdvancementHolder advancement, String criterionKey) {
		boolean bl = false;
		AdvancementProgress advancementProgress = this.getOrStartProgress(advancement);
		boolean bl2 = advancementProgress.isDone();
		if (advancementProgress.grantProgress(criterionKey)) {
			this.unregisterListeners(advancement);
			this.progressChanged.add(advancement);
			bl = true;
			if (!bl2 && advancementProgress.isDone()) {
				advancement.value().rewards().grant(this.player);
				// Where architectury-fabric plugs in its mixin
				advancement.value().display().ifPresent(displayInfo -> {
					if (displayInfo.shouldAnnounceChat() && this.player.level().getGameRules().getBoolean(GameRules.RULE_ANNOUNCE_ADVANCEMENTS)) {
						this.playerList.broadcastSystemMessage(displayInfo.getType().createAnnouncement(advancement, this.player), false); AdvancementEvent.AdvancementEarnEvent
					}
					// Where NeoForge API listens
				});
			}
		}

		if (!bl2 && advancementProgress.isDone()) {
			this.markForVisibilityUpdate(advancement);
		}

		return bl;
	}

How to resolve

Since hidden advancements also deserve listening, it's suggested that architectury-neoforge abandons AdvancementEvent.AdvancementEarnEvent and uses the mixin as well, to match Fabric.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions