Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add LootTableEvents.LOADED event #3352

Merged
merged 4 commits into from
Oct 8, 2023
Merged

Add LootTableEvents.LOADED event #3352

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7ac89dc
Implement `LootTableEvents.LOADED` event
LLytho Sep 30, 2023
9897c54
Update for checkstyle
LLytho Sep 30, 2023
1a8fdd9
rename event
LLytho Oct 3, 2023
3c566a0
Update fabric-loot-api-v2/src/main/java/net/fabricmc/fabric/api/loot/…
modmuss50 Oct 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions fabric-loot-api-v2/src/main/java/net/fabricmc/fabric/api/loot/v2/LootTableEvents.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,15 @@ private LootTableEvents() {
}
});

/**
* This event can be used for post-processing after all loot tables have been loaded and modified by fabric.
modmuss50 marked this conversation as resolved.
Show resolved Hide resolved
*/
public static final Event<Loaded> LOADED = EventFactory.createArrayBacked(Loaded.class, listeners -> (resourceManager, lootManager) -> {
for (Loaded listener : listeners) {
listener.onLootTableLoaded(resourceManager, lootManager);
}
});

public interface Replace {
/**
* Replaces loot tables.
Expand Down Expand Up @@ -116,4 +125,14 @@ public interface Modify {
*/
void modifyLootTable(ResourceManager resourceManager, LootManager lootManager, Identifier id, LootTable.Builder tableBuilder, LootTableSource source);
}

public interface Loaded {
/**
* Called when all loot tables have been loaded and {@link LootTableEvents#REPLACE} and {@link LootTableEvents#MODIFY} have been invoked.
*
* @param resourceManager the server resource manager
* @param lootManager the loot manager
*/
void onLootTableLoaded(ResourceManager resourceManager, LootManager lootManager);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
void onLootTableLoaded(ResourceManager resourceManager, LootManager lootManager);
void onLootTablesLoaded(ResourceManager resourceManager, LootManager lootManager);

technically there's multiple of them? :P

Copy link
Contributor

@maityyy maityyy Oct 1, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Maybe then this event should be called ALL_LOADED? Previous events are called per loot table, so the new event may be confusing.

Also I think this event should be after the MODIFY/REPLACE events in the code, but that does not matter, it is about checkstyle.

Btw, this event is very similar to CommonLifecycleEvents.TAGS_LOADED

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also I think this event should be after the MODIFY/REPLACE events in the code, but that does not matter, it is about checkstyle.

wdym by that?

Copy link
Contributor

Choose a reason for hiding this comment

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

wdym by that?

Ah, sorry, nevermind. I am inattentive

}
}
1 change: 1 addition & 0 deletions fabric-loot-api-v2/src/main/java/net/fabricmc/fabric/mixin/loot/LootManagerMixin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,5 +95,6 @@ private void applyLootTableEvents(ResourceManager resourceManager, LootManager l
});

this.keyToValue = newTables.build();
LootTableEvents.LOADED.invoker().onLootTableLoaded(resourceManager, lootManager);
}
}
7 changes: 7 additions & 0 deletions fabric-loot-api-v2/src/testmod/java/net/fabricmc/fabric/test/loot/LootTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,5 +92,12 @@
tableBuilder.modifyPools(poolBuilder -> poolBuilder.with(ItemEntry.builder(Items.EMERALD)));
}
});

LootTableEvents.LOADED.register((resourceManager, lootManager) -> {
LootTable blackWoolTable = lootManager.getLootTable(Blocks.BLACK_WOOL.getLootTableId());

Check failure on line 97 in fabric-loot-api-v2/src/testmod/java/net/fabricmc/fabric/test/loot/LootTest.java

View workflow job for this annotation

GitHub Actions / build (20-jdk) 

missing blank line before block at same indentation level
if(blackWoolTable == LootTable.EMPTY) {

Check failure on line 98 in fabric-loot-api-v2/src/testmod/java/net/fabricmc/fabric/test/loot/LootTest.java

View workflow job for this annotation

GitHub Actions / build (20-jdk) 

'if' is not followed by whitespace.

Check failure on line 98 in fabric-loot-api-v2/src/testmod/java/net/fabricmc/fabric/test/loot/LootTest.java

View workflow job for this annotation

GitHub Actions / build (20-jdk) 

'if' is not followed by whitespace.
throw new AssertionError("black wool loot table should not be empty");
}
});
}
}
Loading