Merge branch 'evo-tree-fumble' into 'main'

Evo tree now throws a warning instead of crashing when failing to build a mon's evo tree

See merge request cable-mc/cobblemon!2481

CHANGELOG.md

@@ -23,6 +23,11 @@
 ### Changes
 
 ### Fixes
+- Fixed datapacks with item interaction evolutions that don't specify an item crashing the world on load.
+- Fixed the Pokédex evolution tree crashing when a datapack's dex entry is missing an evolution's form.
+- Fixed the Pokédex evolution tree crashing with a stack overflow on evolutions that loop back on themselves.
+- Fixed form-changing evolutions sometimes resolving to the source form in the Pokédex evolution tree.
+- Broken datapack evolutions are now logged and skipped in the Pokédex evolution tree instead of crashing.
 
 ### Developer
 

common/src/main/kotlin/com/cobblemon/mod/common/api/pokedex/entry/DexEntries.kt

@@ -8,6 +8,7 @@
 
 package com.cobblemon.mod.common.api.pokedex.entry
 
+import com.cobblemon.mod.common.Cobblemon
 import com.cobblemon.mod.common.CobblemonItems
 import com.cobblemon.mod.common.api.data.JsonDataRegistry
 import com.cobblemon.mod.common.api.molang.ExpressionLike
@@ -72,8 +73,11 @@ object DexEntries : JsonDataRegistry<PokedexEntry> {
     fun computeEvolutionDisplays() {
         evolutionDisplays.clear()
         PokemonSpecies.species.forEach {
-            evolutionDisplays.addAll(buildDisplays(it.standardForm))
-            evolutionDisplays.addAll(it.forms.flatMap { form -> buildDisplays(form) })
+            try {
+                evolutionDisplays.addAll(buildDisplays(it.standardForm) + it.forms.flatMap { form -> buildDisplays(form) })
+            } catch (e: Exception) {
+                Cobblemon.LOGGER.error("Failed to build dex evolution displays for ${it.resourceIdentifier}", e)
+            }
         }
     }
 
@@ -93,8 +97,10 @@ object DexEntries : JsonDataRegistry<PokedexEntry> {
                     ?.let { PokemonSpecies.getByIdentifier(it.asIdentifierDefaultingNamespace()) }
                     ?: return@resultLoop
 
-                // Carry the source form's aspects over like a real evolution does (Alola-Bias Pichu -> Alola-Bias Pikachu)
-                val resultForm = resultSpecies.getForm(form.aspects.toSet() + resultProperties.aspects)
+                // The result's own aspects win, otherwise carry the source form's aspects over
+                // like a real evolution does (Alola-Bias Pichu -> Alola-Bias Pikachu)
+                val resultForm = resultSpecies.getForm(resultProperties.aspects).takeIf { it != resultSpecies.standardForm }
+                    ?: resultSpecies.getForm(form.aspects.toSet() + resultProperties.aspects)
                 val toAspects = resultProperties.aspects + resultForm.aspects
                 val resultRequirements = if (resultProperties === evolution.shedder) {
                     requirements + CobblemonDexEvolutionRequirementDisplay(CobblemonItems.POKE_BALL, lang("ui.pokedex.evolution.shedder.tooltip"))
@@ -115,4 +121,4 @@ object DexEntries : JsonDataRegistry<PokedexEntry> {
         }
         return result
     }
-}
+}

common/src/main/kotlin/com/cobblemon/mod/common/api/pokedex/entry/DexEvolutionDisplay.kt

@@ -77,8 +77,8 @@ fun getToolTipHeaderIcon(evolution: Evolution): DexEvolutionRequirementDisplay?
 }
 
 
-fun getToolTipHeaderIcon(itemInteractionEvolution: ItemInteractionEvolution): CobblemonDexEvolutionRequirementDisplay {
-    val item = itemInteractionEvolution.requiredContext.items.get().first().value()
+fun getToolTipHeaderIcon(itemInteractionEvolution: ItemInteractionEvolution): CobblemonDexEvolutionRequirementDisplay? {
+    val item = itemInteractionEvolution.requiredContext.items.orElse(null)?.firstOrNull()?.value() ?: return null
     return CobblemonDexEvolutionRequirementDisplay(item, lang("ui.pokedex.evolution.item_interact.tooltip", item.description.plainCopy().green()))
 }
 
@@ -87,7 +87,9 @@ fun getToolTipHeaderIcon(tradeEvolution: TradeEvolution): CobblemonDexEvolutionR
     val tooltip = if (species == null) {
         lang("ui.pokedex.evolution.trade.tooltip.with_any_species")
     } else {
-        lang("ui.pokedex.evolution.trade.tooltip.with_species", PokemonSpecies.getByIdentifier(species.asIdentifierDefaultingNamespace())!!.translatedName.green())
+        PokemonSpecies.getByIdentifier(species.asIdentifierDefaultingNamespace())
+            ?.let { lang("ui.pokedex.evolution.trade.tooltip.with_species", it.translatedName.green()) }
+            ?: lang("ui.pokedex.evolution.trade.tooltip.with_any_species")
     }
     return CobblemonDexEvolutionRequirementDisplay(CobblemonItems.LINK_CABLE, tooltip)
 }
@@ -322,8 +324,8 @@ fun buildMoveTypeRequirement(moveTypeRequirement: MoveTypeRequirement): DexEvolu
     return CobblemonDexEvolutionRequirementDisplay(icon, tooltip)
 }
 
-fun buildOwnerHoldsItemRequirement(ownerHoldsItemRequirement: OwnerHoldsItemRequirement): DexEvolutionRequirementDisplay {
-    val item = ownerHoldsItemRequirement.itemCondition.items.get().first().value()
+fun buildOwnerHoldsItemRequirement(ownerHoldsItemRequirement: OwnerHoldsItemRequirement): DexEvolutionRequirementDisplay? {
+    val item = ownerHoldsItemRequirement.itemCondition.items.orElse(null)?.firstOrNull()?.value() ?: return null
     val tooltip = lang("ui.pokedex.requirement.owner_held_item.tooltip", item.description.plainCopy().green())
 
     return CobblemonDexEvolutionRequirementDisplay(item, tooltip)

common/src/main/kotlin/com/cobblemon/mod/common/client/gui/pokedex/widgets/EvolutionTreeWidget.kt

@@ -48,6 +48,7 @@ import net.minecraft.client.Minecraft
 import net.minecraft.client.gui.GuiGraphics
 import net.minecraft.core.registries.Registries
 import net.minecraft.locale.Language
+import net.minecraft.resources.ResourceLocation
 import net.minecraft.network.chat.Component
 import net.minecraft.network.chat.MutableComponent
 import net.minecraft.tags.TagKey
@@ -164,9 +165,14 @@ class EvolutionTreeWidget(
         var rootSpecies = pokedexEntry
         var rootForm = pokedexForm
         var preEvolution = findPreEvolution(rootSpecies, rootForm)
+        // Stop at whatever the dex can't resolve, and at cycles (datapacks)
+        val visited = mutableSetOf(rootSpecies.speciesId to rootForm.displayForm)
         while (preEvolution != null) {
-            rootSpecies = Dexes.findDexEntryBySpeciesId(preEvolution.fromSpecies)!!
-            rootForm = rootSpecies.forms.first { it.displayForm.equals(preEvolution.fromForm, ignoreCase = true) }
+            val preEvolutionSpecies = Dexes.findDexEntryBySpeciesId(preEvolution.fromSpecies) ?: break
+            val preEvolutionForm = preEvolutionSpecies.forms.firstOrNull { it.displayForm.equals(preEvolution.fromForm, ignoreCase = true) } ?: break
+            if (!visited.add(preEvolutionSpecies.speciesId to preEvolutionForm.displayForm)) break
+            rootSpecies = preEvolutionSpecies
+            rootForm = preEvolutionForm
             preEvolution = findPreEvolution(rootSpecies, rootForm)
         }
 
@@ -230,7 +236,10 @@ class EvolutionTreeWidget(
         pokedexEntry: PokedexEntry,
         pokedexForm: PokedexForm,
         aspects: Set<String> = emptySet(),
+        ancestors: Set<Pair<ResourceLocation, String>> = emptySet(),
     ): EvolutionNode {
+        // Datapack evolutions can loop back (A -> B -> A), so stop at anything already on this branch
+        val path = ancestors + (pokedexEntry.speciesId to pokedexForm.displayForm.lowercase())
         val discoveryLevel = CobblemonClient.clientPokedexData.getHighestKnowledgeFor(pokedexEntry, pokedexForm, aspects.toMutableSet())
 
         val formEquivalents = Dexes.getFormEquivalents(pokedexForm.displayForm)
@@ -243,8 +252,16 @@ class EvolutionTreeWidget(
                 return@mapNotNull null
             }
             val formEquivalents = Dexes.getFormEquivalents(it.toForm.lowercase())
-            val childPokedexForm = childPokedexEntry.forms.first { formEquivalents.contains(it.displayForm.lowercase()) }
-            buildEvolutionTree(childPokedexEntry, childPokedexForm, it.toAspects)
+            val childPokedexForm = childPokedexEntry.forms.firstOrNull { formEquivalents.contains(it.displayForm.lowercase()) }
+            if (childPokedexForm == null) {
+                Cobblemon.LOGGER.warn("Dex form ${it.toForm} not found for species: ${it.toSpecies}")
+                return@mapNotNull null
+            }
+            if ((childPokedexEntry.speciesId to childPokedexForm.displayForm.lowercase()) in path) {
+                Cobblemon.LOGGER.warn("Evolution loop found at species: ${it.toSpecies} form: ${it.toForm}")
+                return@mapNotNull null
+            }
+            buildEvolutionTree(childPokedexEntry, childPokedexForm, it.toAspects, path)
         }
 
         return EvolutionNode(