[ecs] Entry API for components and try_insert command

[NathanSWard]

Objective

• It can be desired to add a component to an entity, if (and only if) that entity doesn't contain the component.
• Partially addresses Non-overwriting insert commands #2054

Solution

• Adds a component function to EntityMut
• Adds a try_insert function to EntityCommands.

TODO:

• Add examples of entry API

[MinerSebas]

What is this doing differently compared to #2061?

[NathanSWard]

What is this doing differently compared to #2061?

The primary discourse around #2061 and #2054 was the try_insert_bundle and what its purpose was.
IMO, try_insert for Components does not carry the same baggage and this PR limits the scope of the issues.

[mockersf]

I think the name try_insert is wrong. In Rust, try_ methods are variant of a panicking method that can't panic. Here it doesn't change anything to the panicking possibilities...

But I don't have any meaningful suggestion for better naming.

[NathanSWard]

I think the name try_insert is wrong.

Yep, I had this thought as well.
I think ideally we'd want some like the HashMap::entry API, however that feels like a little too much right now.
std::HashMap does have a try_insert method (albeit unstable) here.

[Nilirad]

insert_checked
insert_if_absent
insert_unique
insert_dedup

[mockersf]

for completion in IDE, it would be nice to start the name with insert

[NathanSWard]

Would we want something similar to the HashMap::entry API?

enum ComponentEntry {
    Vacant(..),
    Occupied(..),
}

world.entity_mut(e).component::<T>()

[wilk10]

insert_checked
insert_if_absent
insert_unique
insert_dedup

i really like insert_if_absent, it's the most literal / unambiguous, imo

[Nilirad]

i really like insert_if_absent, it's the most literal / unambiguous, imo

I have to admit though that having the word if in the identifier does not look the best.

[NathanSWard]

@mockersf @Nilirad @wilk10
I updated the current implement to have a HashMap::entry like API.
I'm curious to see what your thoughts are?

Note: EntityCommands still has a try_insert function.
However, EntityMut has a new entry API.

[NathanSWard]

In Rust, try_ methods are variant of a panicking method that can't panic

The more I think about this, the more I don't think its a problem.
Just because a method has try_ in front of it doesn't necessarily mean the non try_ variant will panic.

Especially in the context of inserting something for an already valid entity, IMO, this is quite clear.

I think the best example of try_ functions where the non-try variant doesn't panic is the proposed

• std::HashMap::try_insert here
• hashbrowh::HashMap::try_insert here

World and entity->component relationship ois very similar to the HashMap key->value relationship.

[wilk10]

Thanks a lot for your work!

I can get behind this reasoning and the two other examples linked make a good case for it. I also don't think it changes the meaning of insert. It conveys very well that the component will be inserted unless some condition is true.

insert_if_absent goes further and specifies the reason why it won't insert it, but i agree that the presence of if is not great, and maybe users will use it for all insertions, also the first one?
The other proposals were not immediately super obvious to me.

We do lose out on IDE completion aid, but i guess it's an acceptable trade-off.

[NathanSWard]

We do lose out on IDE completion aid, but i guess it's an acceptable trade-off.

This isn't quite true (at least as a VSCode/Rust Analyzer user.
If you type in insert things that start with insert will pop up first, but also, functions that contain that contain the name insert will also pop up.
So the function doesn't necessarily have to start with insert.

[mockersf]

The try_* that you linked (andtry_from) returns a Result which is not possible here as the command execution is asynchronous.

It bothers me to use this word when neither the function name or the return type can explain why it could fail / if it failed

[NathanSWard]

Closing for now since there is probably a design discussion to be had to better this API