Iron's Spells 'n Spellbooks

Iron's Spells 'n Spellbooks

Mods
24M Downloads

[Bug] Crash When Spell Filter has invalid but not empty return

AceTheEldritchKing opened this issue ยท 3 comments

commented

Observed behaviour

After applying a loot table to a custom entity to drop randomized scrolls for the Eldritch school, the game crashes upon the entity dying and provides this error: java.lang.NullPointerException: Cannot invoke "java.lang.Integer.intValue()" because the return value of "java.util.NavigableMap.floorKey(Object)" is null

Expected behaviour

Expected behavior is that when an entity dies, it drops randomized Eldritch scroll loot

Steps to reproduce

  1. Create a custom loot table with an entry as follows:
    { "type": "minecraft:item", "name": "irons_spellbooks:scroll", "functions": [ { "function": "irons_spellbooks:randomize_spell", "quality": { "min": 0.75, "max": 1.0 }, "spell_filter": { "school": "irons_spellbooks:eldritch" } } ] }
  2. Run the game
  3. Summon the entity and kill it to spawn in the loot

Server Type

Single Player

Crashlog

https://gist.github.com/AceTheEldritchKing/5926333ae108a1c6839ddf824c615ca0

Iron's Spells N Spellbooks version

1.21.1-1.10.1

Forge version

Neoforge 1.21.1

Other mods

I encountered this crash while in my dev environment for Discerning The Eldritch, I'm not sure if this is supposed to happen or not but I figured I'd report it here

Crashlog Check

  • I understand if this is a crashbug and I did not attach a crashlog, this will not be handled
commented

It seems the the error comes from floorKey being called on an empty map, which returns null.

I don't have access atm to my dev env so could you try this ?

@Override
protected ItemStack run(ItemStack itemStack, LootContext lootContext) {
    if (!(itemStack.getItem() instanceof Scroll) && !Utils.canImbue(itemStack)) {
        return itemStack;
    }
    
    var applicableSpells = this.applicableSpells.getApplicableSpells();
    if (applicableSpells.isEmpty()) {
        itemStack.setCount(0);
        return itemStack;
    }
    
    var spellList = getWeightedSpellList(applicableSpells);
    if (spellList.isEmpty()) {
        return handleEmptySpellList(itemStack);
    }
    
    Integer highestKey = spellList.floorKey(Integer.MAX_VALUE);
    if (highestKey == null) {
        return handleEmptySpellList(itemStack);
    }

    int total = highestKey;
    AbstractSpell abstractSpell = SpellRegistry.none();
    int randomValue = lootContext.getRandom().nextInt(total);

    Map.Entry<Integer, AbstractSpell> entry = spellList.higherEntry(randomValue);
    if (entry != null) {
        abstractSpell = entry.getValue();
    }
    
    if (abstractSpell.equals(SpellRegistry.none())) {
        return handleEmptySpellList(itemStack);
    }
    
    applySpellToScroll(abstractSpell, itemStack, lootContext);
    return itemStack;
}

private ItemStack handleEmptySpellList(ItemStack itemStack) {
    if (itemStack.getItem() instanceof Scroll) {
        return ItemStack.EMPTY;
    } else {
        return itemStack;
    }
}

private void applySpellToScroll(AbstractSpell spell, ItemStack itemStack, LootContext lootContext) {
    int maxLevel = spell.getMaxLevel();
    float quality = qualityRange.getFloat(lootContext);
    int spellLevel = 1 + Math.round(quality * (maxLevel - 1));
    
    if (itemStack.getItem() instanceof Scroll) {
        ISpellContainer.createScrollContainer(spell, spellLevel, itemStack);
    } else {
        ISpellContainer.createImbuedContainer(spell, spellLevel, itemStack);
    }
}
commented

the reason it is crashing is because the loot function only validity checks for empty, but the spell filter never returns an empty list, but rather a list containing the none spell. the fix is for spell filter to return an empty list.

However, the reason the filter returns nothing @AceTheEldritchKing is because none of the eldritch spells allow looting. there are a few workarounds to this

commented

fixed in 3.10.2. see changelog for new spell filter usage @AceTheEldritchKing