NeoForge icon indicating copy to clipboard operation
NeoForge copied to clipboard

Making entires field public for loot pool modifcation purposes

Open yajatkaul opened this issue 8 months ago • 8 comments

Im making a neoforge mod and i m coming across an issue where i cant modify the loot pool because certain fields are private so i made a mixin for that

@Mixin(LootPool.class)
public interface LootPoolAccessor {
    @Accessor("entries")
    List<LootPoolEntryContainer> getEntries();
}

But doing it like this makes it so that i need to have multiple accessors How i add items to the loot pool right now is very messy

    @SubscribeEvent
    public static void onLootTableLoad(LootTableLoadEvent event) {
        ResourceLocation lootTableId = event.getName();
        ResourceLocation desertPyramidLootTable = ResourceLocation.fromNamespaceAndPath("minecraft", "archaeology/desert_pyramid");
        ResourceLocation desertWellLootTable = ResourceLocation.fromNamespaceAndPath("minecraft", "archaeology/desert_well");

        LootTable table = event.getTable();
        LootPool mainPool = table.getPool("main");
        LootPoolAccessor poolAccessor = (LootPoolAccessor) mainPool;

        if (mainPool != null && !mainPool.isFrozen()) {
            LootPool.Builder newPoolBuilder = LootPool.lootPool()
                    .setRolls(mainPool.getRolls())
                    .setBonusRolls(mainPool.getBonusRolls());

            poolAccessor.getEntries().forEach(entry ->
                    newPoolBuilder.add(new LootPoolEntryContainer.Builder() {
                        @Override
                        protected LootPoolEntryContainer.Builder getThis() {
                            return null;
                        }

                        @Override
                        public LootPoolEntryContainer build() {
                            return entry;
                        }
                    })
            );

            if (desertPyramidLootTable.equals(lootTableId) || desertWellLootTable.equals(lootTableId)) {
                newPoolBuilder.add(LootItem.lootTableItem(MegaStones.RED_ORB)
                        .setWeight(1));

                // Preserve the name
                if (mainPool.getName() != null) {
                    newPoolBuilder.name(mainPool.getName());
                }

                // Replace the old pool with the new one
                table.removePool("main");
                table.addPool(newPoolBuilder.build());
            }
        }
    }

So i hope this pull request can be accepted so i can modify the loot pools properly

yajatkaul avatar Apr 13 '25 09:04 yajatkaul

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Apr 13 '25 09:04 CLAassistant

  • [ ] Publish PR to GitHub Packages

This is a very common AT for mods to make, so it makes sense for this to be in Neo as its very useful when used with Neo's LootTableLoadEvent API. Github has 50 results for this AT on older obffed versions: https://github.com/search?q=path:Aaccesstransformer.cfg "net.minecraft.world.level.storage.loot.LootPool f_79023_"&type=code And 17 for unobffed: https://github.com/search?q=path:Aaccesstransformer.cfg "net.minecraft.world.level.storage.loot.LootPool entries"&type=code

ChiefArug avatar Apr 13 '25 09:04 ChiefArug

Also please implement this in 1.21.1 as well

yajatkaul avatar Apr 13 '25 10:04 yajatkaul

I haven't worked much with newer 1.21 versions, is that list mutable? And is it still final? If it's immutable, a definal might be appropriate to allow adding entries.

KnightMiner avatar Apr 15 '25 14:04 KnightMiner

It is still final and always mutable (even if you pass in an immutable list to the constructor it gets copied into a new ArrayList... leading to interesting bugs with the empty loot table having pools added).

ChiefArug avatar Apr 15 '25 22:04 ChiefArug

Those bugs make me wonder if an AT is the best approach over a method that protects against itself being the empty loot table.

Could be an add method, or something as simple as a getter that checks if it is equal to the empty instance, upon which it returns either List.of() or a new array list, based on whether you want to crash or ignore adding to empty.

KnightMiner avatar Apr 16 '25 17:04 KnightMiner

I'm wondering whether it makes more sense to add a method to the builder that's like copyFrom, where it inherits all the elements from the previous pool to make it mutable again.

ChampionAsh5357 avatar Aug 04 '25 23:08 ChampionAsh5357