Default and/or Insert, onboarding.

[cheako]

The first time an application is run, using a default value appears clunky. Catching the NotFound Error is natural enough to have specific helpers for that case. There is also an argument that this is not an error and get should return an Result<Option<_>... sounds interesting.

Notice the examples are bad(delete those b4 someone copies them), they just roll through any errors by treating any error as the NotFound condition.

Similar to: hwchen/keyring-rs#103

It would be nice if there were rust conventions surrounding these APIs, if you look at std they almost always returns an Option... noting the difference that IO errors are not a thing for HashMap.

    let h: PkvStore;

    let _limits: Option<Limits> =
        h.get("limits")
            .or_else(|x| -> Result<_, Box<dyn std::error::Error>> {
                if matches!(x, bevy_pkv::GetError::NotFound) {
                    h.set("limits", &Option::<Limits>::None)?;
                    Ok(None) /// Indicating the user has not seen the wizard for this.
                } else {
                    Err(x)?
                }
            })?;

    let _old_boot_id: Option<u64> = h.get("boot_id").map(Option::Some).or_else(
        |x| -> Result<_, Box<dyn std::error::Error>> {
            if matches!(x, bevy_pkv::GetError::NotFound) {
                Ok(None) /// Value set in next block.
            } else {
                Err(x)?
            }
        },
    )?;

    let boot_id: u64 = rand::thread_rng().gen();
    h.set("boot_id", &boot_id)?;

I think there are better ways to write this, suggestions welcome.

[landhb]

@cheako Similar to what's discussed here: hwchen/keyring-rs#103

You can create a helper function:

/// Helper to obtain the value, returning None when not
/// yet set.
pub fn get_optional<T: DeserializeOwned>(
    store: &PkvStore,
    value: &str,
) -> Result<Option<T>, Box<dyn Error>> {
    match store.get(value) {
        Ok(s) => Ok(Some(s)),
        Err(bevy_pkv::GetError::NotFound) => Ok(None),
        Err(e) => Err(e),
    }
}

let h: PkvStore;
let _limits: Option<Limits> = get_optional(&h, "limits")?;
let _old_boot_id: Option<u64> = get_optional(&h, "boot_id")?;

[cheako]

Looks good, what if that was a pub function in the crate? That would prevent almost every client from having something similar.

[johanhelsing]

I'd like to try to keep simple usage as ergonomic as possible, but that what's simple may depend on your intention/expectation when using the store. Sometimes you really do expect the thing you query for to be there, so having a Result<Option<T>> then is a bit annoying.

Adding a such get_optional method in addition would be fine, though.

For reference, in bevy APIs, the approach to this is having some methods often come in two flavors, for instance:

• World::entity - panics if there is no matching entity
• World::get_entity - returns None if there is no entity
• Query::single - panics if there is not exactly one matching entity
• Query::get_single - returns an Err if there is not exactly one matching entity