A lot of miscellaneous tweaks (#329)

* - Fix filter property diagnosis
* - Update list of hardcoded vanilla animations
* - Account for entity add/remove component groups being strings
* - Add diagnosis for set_home_position
* - Fixed entity based components not being diagnosed
* - Added diagnosis for input_air_controlled
* - Add error for old minecraft:fall_damage component
* - Added info for when a state is never transitioned into
* - Added check for particle type in durability sensor
* - Add duplicate id check

src/Lib/Diagnostics/BehaviorPack/Animation Controllers/entry.ts

@@ -1,5 +1,5 @@
 import { Internal, SMap } from "bc-minecraft-bedrock-project";
-import { DocumentDiagnosticsBuilder } from "../../../Types";
+import { DiagnosticSeverity, DocumentDiagnosticsBuilder } from "../../../Types";
 import { Json } from "../../Json/Json";
 import { general_animation_controllers } from "../../Minecraft/Animation Controllers";
 import { diagnose_molang } from "../../Molang/diagnostics";
@@ -20,7 +20,17 @@ export function Diagnose(diagnoser: DocumentDiagnosticsBuilder): void {
   general_animation_controllers(controllers, diagnoser);
 
   //foreach animation,
-  SMap.forEach(controllers.animation_controllers, (controller) => {
+  SMap.forEach(controllers.animation_controllers, (controller, id) => {
+
+    diagnoser.context.getCache().behaviorPacks.animation_controllers.forEach(animation_controller => {
+    if (animation_controller.id === id && animation_controller.location.uri !== diagnoser.document.uri) diagnoser.add(
+      id,
+      `Duplicate identifier "${id}" found.`,
+      DiagnosticSeverity.warning,
+      "behaviorpack.animation_controller.duplicate_id"
+      );
+    })
+
     SMap.forEach(controller.states, (state) => {
       state.on_entry?.forEach((item) => json_commandsCheck(item, diagnoser));
       state.on_exit?.forEach((item) => json_commandsCheck(item, diagnoser));

src/Lib/Diagnostics/BehaviorPack/Animation/entry.ts

@@ -19,6 +19,15 @@ export function Diagnose(diagnoser: DocumentDiagnosticsBuilder): void {
   SMap.forEach(anims.animations, (anim, id) => {
     const length = anim.animation_length;
 
+    diagnoser.context.getCache().behaviorPacks.animations.forEach(animation => {
+    if (animation.id === id && animation.location.uri !== diagnoser.document.uri) diagnoser.add(
+      id,
+      `Duplicate identifier "${id}" found.`,
+      DiagnosticSeverity.warning,
+      "behaviorpack.animation.duplicate_id"
+      );
+    })
+
     //foreach time
     SMap.forEach(anim.timeline, (data, time) => {
       json_commandsCheck(data, diagnoser);

src/Lib/Diagnostics/BehaviorPack/Block/entry.ts

@@ -17,6 +17,16 @@ export function Diagnose(diagnoser: DocumentDiagnosticsBuilder): void {
   const block = Json.LoadReport<Internal.BehaviorPack.Block>(diagnoser);
   if (!Internal.BehaviorPack.Block.is(block)) return;
 
+  const identifier = block['minecraft:block'].description.identifier
+  diagnoser.context.getCache().behaviorPacks.blocks.forEach(block => {
+    if (block.id === identifier && block.location.uri !== diagnoser.document.uri) diagnoser.add(
+      `minecraft:block/description/identifier`,
+      `Duplicate identifier "${identifier}" found.`,
+      DiagnosticSeverity.warning,
+      "behaviorpack.block.duplicate_id"
+    );
+  })
+
   //check components
   const context: Context<Internal.BehaviorPack.Block> = {
     source: block,

src/Lib/Diagnostics/BehaviorPack/Entity/components/diagnose.ts

@@ -1,11 +1,12 @@
 import { ComponentBehavior } from "bc-minecraft-bedrock-types/lib/minecraft/components";
-import { DocumentDiagnosticsBuilder } from "../../../../Types";
+import { DiagnosticSeverity, DocumentDiagnosticsBuilder } from "../../../../Types";
 import { Context } from "../../../../utility/components";
 import { ComponentCheck, components_check } from "../../../../utility/components/checks";
 import { behaviorpack_entity_components_filters } from './filters';
 import { check_loot_table } from "./loot";
 import { check_trade_table } from "./trade";
 import { Internal } from 'bc-minecraft-bedrock-project';
+import { FormatVersion } from 'bc-minecraft-bedrock-types/lib/minecraft';
 
 /**
  *
@@ -28,4 +29,23 @@ const component_test: Record<string, ComponentCheck<Internal.BehaviorPack.Entity
   "minecraft:equipment": check_loot_table,
   "minecraft:loot": check_loot_table,
   "minecraft:trade_table": check_trade_table,
+  "minecraft:input_air_controlled": (name, component, context, diagnoser) => {
+    if (!(context.source as any).use_beta_features) diagnoser.add(name, 
+      `This component requires "use_beta_features" to be set to true`,
+      DiagnosticSeverity.error,
+      `behaviorpack.entity.component.requires_beta_features`
+    )
+  },
+  "minecraft:fall_damage": (name, component, context, diagnoser) => {
+    try {
+    const version = FormatVersion.parse(context.source.format_version);
+    if (version[0] > 1 || (version[0] === 1 && version[1] > 10) || (version[0] === 1 && version[1] === 10 && version[2] > 0)) diagnoser.add(name,
+      `To use "minecraft:fall_damage", you need a "format_version" of 1.10.0 or lesser`,
+      DiagnosticSeverity.error,
+      'behaviorpack.entity.component.fall_damage')
+    // eslint-disable-next-line @typescript-eslint/no-unused-vars
+    } catch (err) {
+      // Leaving this empty as the base diagnoser should already flag an invalid format version
+    }
+  }
 };

src/Lib/Diagnostics/BehaviorPack/Entity/entry.ts

@@ -28,6 +28,16 @@ export function Diagnose(diagnoser: DocumentDiagnosticsBuilder): void {
   if (!Internal.BehaviorPack.Entity.is(entity)) return;
 
   //No resource-pack check, entities can exist without their rp side
+
+  const identifier = entity['minecraft:entity'].description.identifier
+  diagnoser.context.getCache().behaviorPacks.entities.forEach(entity => {
+    if (entity.id === identifier && entity.location.uri !== diagnoser.document.uri) diagnoser.add(
+      `minecraft:entity/description/identifier`,
+      `Duplicate identifier "${identifier}" found.`,
+      DiagnosticSeverity.warning,
+      "behaviorpack.entity.duplicate_id"
+    );
+  })
 
   //check components
   const context: Context<Internal.BehaviorPack.Entity> = {
@@ -40,10 +50,9 @@ export function Diagnose(diagnoser: DocumentDiagnosticsBuilder): void {
 
   const container = entity["minecraft:entity"];
   const MolangData = Molang.MolangFullSet.harvest(container);
-  const id = container.description.identifier;
 
   const owner = {
-    id: id,
+    id: identifier,
     molang: MolangData,
     animations: DefinedUsing.create<string>(),
   };

src/Lib/Diagnostics/BehaviorPack/Entity/events.ts

@@ -36,8 +36,8 @@ export function behaviorpack_entity_check_event(
   properties: EntityProperty[],
   component_groups?: ComponentGroups
 ): void {
-  has_groups(diagnoser, event_id, event.add?.component_groups, component_groups);
-  has_groups(diagnoser, event_id, event.remove?.component_groups, component_groups);
+  has_groups(diagnoser, event_id, typeof event.add?.component_groups == 'string' ? [event.add?.component_groups] : event.add?.component_groups, component_groups);
+  has_groups(diagnoser, event_id, typeof event.remove?.component_groups == 'string' ? [event.remove?.component_groups] : event.remove?.component_groups, component_groups);
 
   event.randomize?.forEach((item) => {
     behaviorpack_entity_check_event(item, event_id, diagnoser, properties, component_groups);
@@ -64,6 +64,15 @@ export function behaviorpack_entity_check_event(
     );
   }
 
+  if ((event as any)["set_home_position"] && !diagnoser.document.getText().includes('minecraft:home')) {
+    diagnoser.add(
+      `events/${event_id}`,
+      `To use set_home_position, \`minecraft:home\` is required.`,
+      DiagnosticSeverity.error,
+      "behaviorpack.entity.event.set_home_position"
+    );
+  }
+
   if (event.queue_command) {
     const c = event.queue_command.command;
     const command = typeof c === "string" ? [c] : c;

src/Lib/Diagnostics/BehaviorPack/Item/components/diagnose.ts

@@ -8,6 +8,7 @@ import { behaviorpack_check_blockid } from "../../Block";
 import { behaviorpack_entityid_diagnose } from "../../Entity";
 import { behaviorpack_item_diagnose } from "../diagnose";
 import { FormatVersion } from 'bc-minecraft-bedrock-types/lib/minecraft';
+import { resourcepack_particle_diagnose } from '../../../ResourcePack/Particle';
 
 /**
  *
@@ -69,7 +70,7 @@ const component_test: Record<string, ComponentCheck<Internal.BehaviorPack.Item>>
     if (component.replace_block_item && context.source['minecraft:item'].description.identifier != component.block) diagnoser.add(`minecraft:block_placer/block/${component.block}`,
       `${component.replace_block_item} and ${context.source['minecraft:item'].description.identifier} need to match when trying to replace the block item`,
       DiagnosticSeverity.error,
-      'behaviorpack.item.components.replace_block_ids_dont_match') 
+      'behaviorpack.item.components.replace_block_ids_dont_match')
     if (component.block) {
       if (typeof component.block == 'object' && 'name' in component.block) behaviorpack_check_blockid((component.block as { name: string }).name, diagnoser)
       else if (typeof component.block == 'string') behaviorpack_check_blockid(component.block, diagnoser);
@@ -107,16 +108,20 @@ const component_test: Record<string, ComponentCheck<Internal.BehaviorPack.Item>>
     }
   },
   "minecraft:custom_components": (name, component, context, diagnoser) => {
-  try {
-    const version = FormatVersion.parse(context.source.format_version);
-    if (version[0] < 1 || version[1] < 21 || (version[2] < 10 && version[1] <= 21)) diagnoser.add(context.source.format_version,
-      `To use custom components, a minimum format version of 1.21.10 is required`,
-      DiagnosticSeverity.error,
-      'behaviorpack.item.components.custom_components_min_version')
-    // eslint-disable-next-line @typescript-eslint/no-unused-vars
-  } catch (err) {
-    // Leaving this empty as the base diagnoser should already flag an invalid format version
-  }
+    try {
+      const version = FormatVersion.parse(context.source.format_version);
+      if (version[0] < 1 || version[1] < 21 || (version[2] < 10 && version[1] <= 21)) diagnoser.add(context.source.format_version,
+        `To use custom components, a minimum format version of 1.21.10 is required`,
+        DiagnosticSeverity.error,
+        'behaviorpack.item.components.custom_components_min_version')
+      // eslint-disable-next-line @typescript-eslint/no-unused-vars
+    } catch (err) {
+      // Leaving this empty as the base diagnoser should already flag an invalid format version
+    }
+  },
+  "minecraft:durability_sensor": (name, component, context, diagnoser) => {
+    if (!component.particle_type || !(typeof component.particle_type == 'string')) return;
+    resourcepack_particle_diagnose(component.particle_type, diagnoser)
   }
 };
 

src/Lib/Diagnostics/BehaviorPack/Item/entry.ts

@@ -13,6 +13,16 @@ export function Diagnose(diagnoser: DocumentDiagnosticsBuilder): void {
   const item = Json.LoadReport<Internal.BehaviorPack.Item>(diagnoser);
   if (!Internal.BehaviorPack.Item.is(item)) return;
 
+  const identifier = item['minecraft:item'].description.identifier
+  diagnoser.context.getCache().behaviorPacks.items.forEach(item => {
+    if (item.id === identifier && item.location.uri !== diagnoser.document.uri) diagnoser.add(
+      `minecraft:item/description/identifier`,
+      `Duplicate identifier "${identifier}" found.`,
+      DiagnosticSeverity.warning,
+      "behaviorpack.item.duplicate_id"
+    );
+  })
+
   //Check components
   const context: Context<Internal.BehaviorPack.Item> = {
     source: item,

src/Lib/Diagnostics/Minecraft/Animation Controllers.ts

@@ -57,11 +57,26 @@ export function general_animation_controller(
     }
   }
 
+  const states = Object.keys(controller.states);
+  const transitionedStates = new Set()
+
   //Check states
   SMap.forEach(controller.states, (state) => {
     //Check transitions
-    if (state.transitions) CheckTransition(controller_id, state.transitions, controller.states, diagnoser);
+    if (state.transitions) {
+      CheckTransition(controller_id, state.transitions, controller.states, diagnoser).forEach(transition => transitionedStates.add(transition))
+    }
   });
+
+  states.forEach(state => {
+    if (!transitionedStates.has(state) && controller.initial_state != state) diagnoser.add(
+      `${controller_id}/${state}`,
+      `"${state}" state is never reached.`,
+      DiagnosticSeverity.info,
+      "minecraft.animation_controller.state.never_reached"
+    );
+  })  
+
 }
 
 /**
@@ -76,7 +91,8 @@ function CheckTransition(
   Transitions: Types.Conditional[],
   States: SMap<State>,
   diagnoser: DiagnosticsBuilder
-): void {
+): string[] {
+  const transitionedStates: string[] = []
   //Loop over the transitions
   for (let I = 0; I < Transitions.length; I++) {
     const trans = Transitions[I];
@@ -91,8 +107,9 @@ function CheckTransition(
         DiagnosticSeverity.error,
         "minecraft.animation_controller.state.missing"
       );
-    }
+    } else transitionedStates.push(state)
   }
+  return transitionedStates
 }
 
 export type Controller =

src/Lib/Diagnostics/Minecraft/Animation.ts

@@ -10,6 +10,9 @@ const whitelist = [
   "controller.animation.player.hudplayer",
   "animation.player.look_at_target.inverted",
   "controller.animation.persona.blink",
+  "animation.humanoid.fishing_rod",
+  "animation.player.first_person.attack_rotation_item",
+  "animation.player.first_person.crossbow_hold"
 ];
 
 export interface AnimationUsage {

src/Lib/Diagnostics/Minecraft/Filter/Filters/property.ts

@@ -1,17 +1,31 @@
 import { Minecraft } from "bc-minecraft-bedrock-types";
-import { DiagnosticsBuilder } from "../../../../Types";
+import { DiagnosticsBuilder, DiagnosticSeverity } from "../../../../Types";
 import { diagnose_entity_property_usage } from "../../../BehaviorPack/Entity/properties";
 
 export function diagnose_filter_property(filter: Minecraft.Filter.Filter, diagnoser: DiagnosticsBuilder) {
-  const { test, domain, value } = filter;
+  const { domain, value } = filter;
+
+  if (!domain) return;
 
   const entities = diagnoser.context.getCache().behaviorPacks.entities;
+
+  let diagnosed = false;
+
   entities.forEach((entity) => {
     if (entity.properties) {
       const property = entity.properties.find((property) => property.name === domain);
       if (property) {
-        diagnose_entity_property_usage([property], test, value as string | number | boolean, "filter", diagnoser);
+        diagnose_entity_property_usage([property], domain, value as string | number | boolean, "filter", diagnoser);
+        diagnosed = true;
       }
     }
   });
+
+  // If no definition for it is found in any entity
+  if (!diagnosed) diagnoser.add(
+    `$filters/${domain}`,
+    `Entity property definition for "${domain}" not found`,
+    DiagnosticSeverity.error,
+    "behaviorpack.entity.property.unknown_property"
+  )
 }

src/Lib/Diagnostics/ResourcePack/anim or controller.ts

@@ -7,6 +7,10 @@ import { education_enabled } from "../Definitions";
 import { animation_controller_diagnose_implementation } from "./Animation Controllers/diagnostics";
 import { animation_diagnose_implementation } from "./Animation/diagnostics";
 
+const whiteList = [
+  'animation.humanoid.fishing_rod'
+]
+
 export function animation_or_controller_diagnose_implementation(
   id: string,
   user: EntityAnimationMolangCarrier,
@@ -23,6 +27,7 @@ export function animation_or_controller_diagnose_implementation(
       return animation_controller_diagnose_implementation(id, user, ownerType, diagnoser, { particles, sounds });
 
     case anim_or_contr.neither:
+      if (whiteList.includes(id)) return;
       diagnoser.add(
         id,
         `Cannot find animation / animation controller: ${id}`,

src/Lib/utility/components/checks.ts

@@ -30,6 +30,7 @@ export function components_check<T>(
   if (data === undefined) return;
 
   compContainerCheck(data.components, context, diagnoser, component_test);
+  compContainerCheck(data, context, diagnoser, component_test);
 
   if (data.component_groups) {
     Object.entries(data.component_groups).forEach(([, group]) => {