[Bugfix] add_stage now checks Stage existence

[Telzhaak]

What's the bug?

add_stage never checked whether a given Stage-name already exists (which extends to with_stage())

bevy/crates/bevy_ecs/src/schedule/mod.rs

Lines 59 to 63 in cc9ed52

	pub fn add_stage<S: Stage>(&mut self, name: &str, stage: S) -> &mut Self {
	self.stage_order.push(name.to_string());
	self.stages.insert(name.to_string(), Box::new(stage));
	self
	}

instead simply

1. replacing the previously held Stage in Schedule.stages (effectively removing all systems/schedulers/... attached to that old Stage)
2. Appending the Stage-name to the vector Schedule.stage_order, while not removing the already existing one, creating a duplicate

How is it fixed?

This has been fixed using the same method the other two functions add_stage_after and add_stage_before use:

if self.stages.get(name).is_some() {
    panic!("Stage already exists: {}.", name);
}

This also raises the question:

• Should these functions even panic when a Stage-name is already taken, or should some sort of Error-flow be implemented to allow handling it?

---

Why does it matter?

Since this bug effectively changed system-execution, it could have caused some interesting and hard to debug problems.
Especially later on, when other Stages are added in relation to this one. add_stage is supposed to place a Stage at the end of execution-order at the time of calling (Vec::push()). Thus anything added via e.g. add_stage_before(this_broken_stage) would be expected to execute as e.g. second-last. However add_stage_before determines positioning via:

let target_index = self
  .stage_order
  .iter()
  .enumerate()
  .find(|(_i, stage_name)| *stage_name == target)
  .map(|(i, _)| i)
  .unwrap_or_else(|| panic!("Target stage does not exist: {}.", target))

This would return the index of the duplicate in Schedule.stage_order at the old position, earlier in the execution-order.

[Telzhaak]

As @cart elaborated in the resolved conversation, panicking is the way to go for now. Error-handling being a discussion to be held in a different issue.

@bjorn3 's changes have been merged and additionally been applied to add_stage_before and add_stage_after, removing one HashMap-lookup each, to improve startup-times