Trades with merchant_predicate are incorrectly removed from pool

We have encountered a regression in version 26.2 related to the fix for MC-307833. When using a custom villager trade set with "allow_duplicates": true, trades that rely on a merchant_predicate to check the villager's existing Offers list are being incorrectly removed from the trade generation pool on the first failed attempt. This breaks custom trade sets that functioned correctly in version 26.1.

Detailed Analysis

In version 26.1, the trade generation logic would loop through the pool and retry evaluating trades. If the predicate required certain trades to already exist, it would be re-evaluated after other trades were generated.

However, in version 26.2 (which includes the fix for MC-307833), the function addOffersFromItemListings was modified to remove a trade from the leftoverOffers pool if villagerTrade.value().getOffer(lootContext) returned null. For trades with a merchant_predicate, the getOffer method may return null on the first evaluation attempt if the predicate conditions are not yet satisfied. Once null is returned, the trade is permanently removed from the pool and never considered again, even if the predicate would become true after other trades are generated (e.g., once the villager has accumulated a sufficient number of existing offers).

This creates a scenario where certain custom trade sets that functioned correctly in 26.1 are now broken, as the trades are prematurely discarded, preventing the villager from receiving them.

Steps to Reproduce:

1. Create a new creative world in Minecraft 26.2 with the linked data pack.

2. Restart the game, spawn a villager, and convert it into an armorer by placing a blast furnace nearby.

3. Open the villager's trade interface and observe the available trades.

Expected Result: All the trades should appear among the armorer's level 1 offers, as they did in version 26.1.

Actual Result: Most trades do not appear. The merchant_predicate in each trade file fails on the first evaluation because the villager's Offers list does not yet meet the required conditions (i.e., does not have the exact number of existing recipes). The trades are then permanently removed from the pool before other trades can be generated to satisfy the predicates.

Code Review (Speculative):

The regression appears to be caused by the fix for MC-307833, which introduced logic that removes a trade from the pool on any null return from getOffer. The current implementation does not distinguish between:

• Permanent failures (e.g., map trades in worlds with no structures, where the trade can never generate)

• Temporary failures (e.g., merchant_predicate conditions that are not yet met but could be met after other trades are generated)

The previous behavior did not remove trades on predicate failure, allowing them to be re-evaluated.

Suggested Fix:

The current behavior of removing a trade immediately when getOffer returns null is incorrect. Instead, the generation logic should follow this approach:

1. Traverse all available trades in the pool once, in random order.

2. If a trade's getOffer returns null due to a predicate failure, keep it in the pool and continue to the next trade.

3. Only after completing one full pass through all trades, if no new offers were successfully generated (i.e., all trades failed their predicates), then stop the generation process entirely.

4. If at least one offer was generated during the pass, repeat the process for the remaining trades in the pool.

Proposed Implementation:

private static void addOffersFromItemListings(LootContext lootContext, MerchantOffers merchantOffers, HolderSet<VillagerTrade> potentialOffers, int numberOfOffers) {
    ArrayList<Holder<VillagerTrade>> leftoverOffers = Lists.newArrayList(potentialOffers);
    int offersFound = 0;
    
    // Continue until we have enough offers or no more trades are available
    while (offersFound < numberOfOffers && !leftoverOffers.isEmpty()) {
        boolean anyOfferGenerated = false;
        
        // Shuffle the remaining trades to maintain randomness for this pass
        Collections.shuffle(leftoverOffers, lootContext.getRandom());
        
        // Iterate through all trades in this pass
        Iterator<Holder<VillagerTrade>> iterator = leftoverOffers.iterator();
        while (iterator.hasNext()) {
            Holder<VillagerTrade> villagerTrade = iterator.next();
            MerchantOffer offer = villagerTrade.value().getOffer(lootContext);
            
            if (offer == null) {
                // Predicate failed - keep the trade in the pool for the next pass
                // It may succeed after other trades are generated
                continue;
            }
            
            // Offer successfully generated
            merchantOffers.add(offer);
            offersFound++;
            anyOfferGenerated = true;
            break;
        }
        
        // If no offers were generated during this full pass, all remaining trades
        // have predicates that cannot be satisfied - stop the generation process
        if (!anyOfferGenerated) {
            break;
        }
    }
}