amethyst_animation::util::get_animation_set panics if Entity is dead.

[Xaeroxe]

See this: https://github.com/amethyst/amethyst/pull/684/files/8696588889e44cc42f8b5e0e851d3ce9f8557df3#r185024799

for more detail.

[torkleyy]

I'm adding this issue for first-time contributors. Here's a more detailed description of the issue:

The following function is using an unwrap on the Result of entry. In case the Entity is dead, the result is an error and the unwrap will make the function panic.

amethyst/amethyst_animation/src/util.rs

Lines 15 to 27 in 6484af2

	pub fn get_animation_set<'a, I, T>(
	controls: &'a mut WriteStorage<AnimationControlSet<I, T>>,
	entity: Entity,
	) -> &'a mut AnimationControlSet<I, T>
	where
	I: Send + Sync + 'static,
	T: AnimationSampling,
	{
	controls
	.entry(entity)
	.unwrap()
	.or_insert_with(AnimationControlSet::default)
	}

Your task is to evaluate whether the function should return an Option or a Result and replace the unwrap accordingly. If you're interested in fixing this issue or you need help with something, please comment here.

There's a rough overview of the animation crate available in our book.

[torkleyy]

I'm adding this issue for first-time contributors. Here's a more detailed description of the issue:

The following function is using an unwrap on the Result of entry. In case the Entity is dead, the result is an error and the unwrap will make the function panic.

amethyst/amethyst_animation/src/util.rs

Lines 15 to 27 in 6484af2

	pub fn get_animation_set<'a, I, T>(
	controls: &'a mut WriteStorage<AnimationControlSet<I, T>>,
	entity: Entity,
	) -> &'a mut AnimationControlSet<I, T>
	where
	I: Send + Sync + 'static,
	T: AnimationSampling,
	{
	controls
	.entry(entity)
	.unwrap()
	.or_insert_with(AnimationControlSet::default)
	}

Your task is to evaluate whether the function should return an Option or a Result and replace the unwrap accordingly. If you're interested in fixing this issue or you need help with something, please comment here.

There's a rough overview of the animation crate available in our book.

[hashedone]

I don't think, that returning Result makes sense here, as long, as only information available in Err is actual_gen. Am I right?

[hashedone]

I don't think, that returning Result makes sense here, as long, as only information available in Err is actual_gen. Am I right?

[Xaeroxe]

It's the only information we have and I don't see much harm in returning a Result

[Xaeroxe]

It's the only information we have and I don't see much harm in returning a Result

[hashedone]

I made a correction and submitted pull request for this.

[hashedone]

I made a correction and submitted pull request for this.