Should AssetEvent::Modified {...} be triggered even if there was no change in bevy-inspector-egui?

[stinkytoe]

Greetings!

I have noticed what I believe to be a bug in the AssetEvent event system.

I am not sure if it's a bug in bevy or in bevy-inspector-egui, or even if it's just a misunderstanding on my behalf. Please let me know if I should bring this up on the bevy-inspector-egui project instead.

When I create a custom asset and inspect it using the WorldInspectorPlugin tool, whether in an entity or in the assets section, simply having the asset open in the inspector will cause an AssetEvent::Modified event to trigger every loop, even if nothing is changed in the asset!

Here is a minimum example:

use bevy::asset::{AssetLoader, AsyncReadExt};
use bevy::prelude::*;
use bevy_inspector_egui::quick::WorldInspectorPlugin;
use serde::Deserialize;
use thiserror::Error;

fn main() {
    App::new()
        // plugins
        .add_plugins((DefaultPlugins, WorldInspectorPlugin::default()))
        // asset stuff
        .init_asset::<TestAsset>()
        .register_asset_reflect::<TestAsset>()
        .register_asset_loader(TestAssetLoader)
        // systems
        .add_systems(Startup, setup)
        .add_systems(Update, asset_event_handler)
        .run();
}

fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
    commands.spawn(Camera2dBundle::default());

    let handle: Handle<TestAsset> = asset_server.load("test.ron");

    commands.spawn((Name::from("Test entity!".to_string()), handle));
}

fn asset_event_handler(
    mut events: EventReader<AssetEvent<TestAsset>>,
    test_assets: Res<Assets<TestAsset>>,
) {
    for event in events.read() {
        match event {
            AssetEvent::Added { id } => {
                let test_asset = test_assets.get(*id).unwrap();
                info!("Added: {test_asset:?}");
            }
            AssetEvent::Modified { id } => {
                let test_asset = test_assets.get(*id).unwrap();
                info!("Modified: {test_asset:?}");
            }
            AssetEvent::Removed { id: _ }
            | AssetEvent::Unused { id: _ }
            | AssetEvent::LoadedWithDependencies { id: _ } => {}
        }
    }
}

#[derive(Asset, Debug, Deserialize, Reflect)]
struct TestAsset {
    test_field: String,
}

#[derive(Debug, Error)]
enum TestAssetLoaderError {
    #[error(transparent)]
    Io(#[from] std::io::Error),
    #[error(transparent)]
    RonSpannedError(#[from] ron::error::SpannedError),
}

struct TestAssetLoader;

impl AssetLoader for TestAssetLoader {
    type Asset = TestAsset;

    type Settings = ();

    type Error = TestAssetLoaderError;

    fn load<'a>(
        &'a self,
        reader: &'a mut bevy::asset::io::Reader,
        settings: &'a Self::Settings,
        load_context: &'a mut bevy::asset::LoadContext,
    ) -> bevy::utils::BoxedFuture<'a, Result<Self::Asset, Self::Error>> {
        Box::pin(async move {
            let mut bytes = Vec::new();
            reader.read_to_end(&mut bytes).await?;
            let ron: TestAsset = ron::de::from_bytes(&bytes)?;

            Ok(ron)
        })
    }

    fn extensions(&self) -> &[&str] {
        &["test.ron"]
    }
}

And here's a repo containing the example: example

[alice-i-cecile]

Definitely a nuisance. I suspect bevy_inspector_egui isn't early-returning correctly, but we may not give them the tools to do so. Can you open an issue on their repo and we can investigate further there?

[stinkytoe]

issue

[alexparlett]

I've seen this before with egui - its how they handle non reference values, the string will be getting changed by the text edit every frame even if it didn't change.

[alexparlett]

@alice-i-cecile Double checked this. Whilst the above is also true due to immediate mode in egui bevy also causes this.

    #[inline]
    pub fn get_mut(&mut self, id: impl Into<AssetId<A>>) -> Option<&mut A> {
        let id: AssetId<A> = id.into();
        let result = match id {
            AssetId::Index { index, .. } => self.dense_storage.get_mut(index),
            AssetId::Uuid { uuid } => self.hash_map.get_mut(&uuid),
        };
        if result.is_some() {
            self.queued_events.push(AssetEvent::Modified { id });
        }
        result
    }

It's because we don't do actual change detection, we just assume it has changed whenever a mutable value is retrieved from assets.