Going back to get returning a Result

[leudz]

In 0.4 I made get panic if a component isn't found and added try_get to return a Result.
Both 0.3 and 0.4 Get traits have their pros and cons. It's also possible to personalize the library locally so the question is whether 0.3 or 0.4 way should be the default.

get returning Result has a big advantage, it's what the Rust ecosystem does and is less surprising for all users (especially new ones).
get panicking is useful for prototyping when you don't want to think about errors and it fits shipyard's try_* alternatives.

Previous discussions on the subject:
zulip
github

#80 will be implemented regardless of this issue.

👍 to go back to get returning a Result, no more try_get
👎 to keep get and try_get the way they are

[colelawrence]

I've been pretty vocal up to this point about this issue along with @almetica, because I think both of our applications have pretty serious consequences if they hit panics (that could have been avoided like using try_get instead of get).

I lean towards moving towards get returning a Result because it makes a lot of sense to me that a storage may not have a value for the entity. I think this both follows my intuition and reinforces the way that an ECS works. For large chunks of my application whether a component exists or not for an Entity tells me something important that I will make a choice to act on.

Any other function that can panic is one that is usually called at the beginning or the end of the life of the application, or it is something unrecoverable like not being able to borrow a storage. Therefore it is not as big of a deal to me to panic because of a borrowing issue, since there wasn't much I could do anyway.

[dakom]

I've been staying out of the convo mostly because it's been a crazy week for me personally, haven't even had time to look at the changes in 0.4 yet - but also because there's simply good arguments on both sides...

At the end of the day though, I'm going to give it a 👍 because the tradeoff seems to be verbosity for safety, and I think that's generally more in line with what people expect from Rust.

Having _unchecked() alternatives would be great- if it gives a performance boost too, that's icing on the cake.

[dakom]

So, uhhh, I didn't read this carefully or play with it enough. Changed my vote to 👎 since having try_get() is absolutely fine for codebases that want to work with Result instead of panic, and it seems to me getting rid of this makes it more tricky for other codebases (as mentioned - they can use an extension trait to automatically unwrap or whatever.. but, meh).

However, I don't think my vote should swing anything - obviously everyone else has thought more carefully about this than me 😅 ... so please count my vote as like half a vote ;) Just changing it because I felt not doing so would be disingenuous since I have changed my stance, fwiw.