`Map::insert` returned value is ambiguous (and using `Result` types)

[Athosvk]

Problem The Map::insert API currently returns an Option<Box<dyn Reflect>>, but if this returns None it can mean there was no previous value, the key type did not match or the value type did not match. Currently there is no way to tell.

Additionally it's currently not possible to infer context of operations not succeeding when using other API functions too. All conversions from dyn Reflect to T may fail, but it's not clear where the fail happens. E.g.

#[derive(Default, Reflect)]
struct Foo {
    bar: i32,
    foobar: u64
}

#[derive(Reflect)]
struct Bar {
    values: BTreeMap<i32, Foo>;
}

[...]

fn test() {
  Bar::from_reflect(StructValue::default().with_field("values", 42);
  Bar::from_reflect(StructValue::default().with_field("test", BTreeMap::default());

  let mut map = BTreeMap::default();
  map.insert("invalid_key", Foo::default());
  Bar::from_reflect(StructValue::default().with_field("values", map));
}

All three will return None, but all for very different reasons.

Potential Solution For Map::insert specifically (maybe there's other similar instances that I missed), it would help if it would return something like a Result<Option<Box<&dyn Reflect>>> so that there is a distinguishment between containing a previous value at that key or failing to convert the input types.

Ideally the Result would also make use of an error type that has some kind of trail? Maybe adding the "path" as part of the error type would make it easier to spot errors in source paths? E.g.

  let mut map = BTreeMap::default();
  map.insert("invalid_key", Foo::default());
  Bar::from_reflect(StructValue::default().with_field("values", map));

Error: Could not construct Bar::values from Map { "invalid_key", Struct { ... } } 

Even if the path aspect isn't done, I think returning a Result instead of Option where conversions can fail would already help quite a bit.

[davidpdrsn]

I agree that makes sense to fix! Added it to the 0.2 milestone since its a breaking change.

[davidpdrsn]

This was fixed in https://github.com/EmbarkStudios/mirror-mirror/pull/136. Will be released as part of 0.2. Not sure when that'll be but probably not too long 🤞