Api improvements and footguns

[ten3roberts]

Making this an umbrella issue to make it easier to add things and not have dozens of separate loose issues with fragmented conversations.

Discovered during modjam:

• [ ] input::get and input::get_delta are inconsistent in their number of return values
• [x] with_default is a footgun, for example using sphere_collider #690
• [ ] physics_controlled is misleading in that it says it only needs that one component, while is needs a lot of components to work. #480
• [ ] mouse_position does not make it clear if it is clip or screen space, while the ray functions use the screen_/clip_ naming.
• [ ] Ray is not a component
• [x] Multiple identical functions (and modules), e.g sphere
• [ ] Message declaration is quite complicated, though this is mostly because of toml and inline/non-inline tables that may be difficult to parse due to their non structured format. In general, this is because toml is not designed for nested lists, which message declaration uses
• [ ] #695

[philpax]

Making this an umbrella issue to make it easier to add things and not have dozens of separate loose issues with fragmented conversations.

They're not really related, so I'm not sure grouping them is the right call. That being said,

input::get and input::get_delta are inconsistent in their number of return values

This is partially intentional - get_delta gets the previous and current inputs, and returns both the delta and the current input to allow you not to call the current one again. Maybe this should be a single return value with a struct of { current, previous, delta }?

with_default is a footgun, for example using sphere_collider ban .with_default #690

Yes, that seems to be the consensus. I'll get it done.

physics_controlled is misleading in that it says it only needs that one component, while is needs a lot of components to work.

I don't think it's ever said that it needs just that one component, but I agree that it's not the most obvious what else is required. Will update.

mouse_position does not make it clear if it is clip or screen space, while the ray functions use the screen_/clip_ naming.

Agreed.

Ray is not a component

I don't want to bake too many types into the component values because the enum will continue to grow, but I agree that this would be nice. Maybe one for struct type definition in ambient.toml, whenever we get around to doing that.

Multiple identical functions (and modules), e.g sphere

Aye. Have been trying to clear these out with renaming the tag components to is_, but I think the components that convert their entity to something else (like the sphere component) should have their own prefix. Something like convert_to_sphere maybe?

Message declaration is quite complicated, though this is mostly because of toml and inline/non-inline tables that may be difficult to parse due to their non structured format. In general, this is because toml is not designed for nested lists, which message declaration uses

Mm, I know. It's not so bad if your fields fit in one inline table:

[messages.Input]
description = "Player input."
fields = { direction = "Vec2", mouse_delta_x = "F32" }

but it's not ideal if it doesn't. Maybe, in the future, we'll consider our own schema definition language, but that probably won't be for a while. We could also consider switching to another language like KDL, but we probably don't want to break the symmetry with Rust / it's even less visibly structured.

Setting linear_velocity on an entity does not affect its initial velocity. Appears to be zeroed out when spawned using the inspector.

I would suggest opening a separate issue for this.

[ten3roberts]

Yeah, I'll shell out the less related ones to their own issues, and keep this as an umbrella one just to keep them under the topic of "issues encountered by a new user within 20 minutes"

[ten3roberts]

Mm, I know. It's not so bad if your fields fit in one inline table:

[messages.Input] description = "Player input." fields = { direction = "Vec2", mouse_delta_x = "F32" }

Yes, though the issue with toml is that you have these two different ways of doing things, and you start with one, and then by just additive changes you have to go back and change to the other format. In general, whenever I design apis I tend to adhere to the principle of "adding things should not make you change things declared above", which toml doesn't uphold.

This is the manifest I ended up using for the modjam

[ember]
id = "asteroid_storm"
name = "Asteroid Storm"
version = "0.0.1"

[components]
camera_ref = { type = "EntityId", attributes = ["Debuggable"] }

[messages]

[messages.Input]
description = "The ray from the camera to the mouse cursor."

[messages.Input.fields]
origin = "Vec3"
dir = "Vec3"

[chaosprint]

Is it possible that once physics_controlled() is given, a default controller height and radius will also be set.

[ten3roberts]

Is it possible that once physics_controlled() is given, a default controller height and radius will also be set.

Yes

I do think, given our rather unique goals of what we want that we should aim towards a "sensible defaults" oriented workflow as much as possible. Compared to Bevy, which tailors to the more in-depth development we tailor for a bit more casual, where people create games and end user products to a larger extent rather than building out a whole collection of systems.

As such, having things easier to set up, such as the number of components for a physics enabled ball should be as low as possible. We are after all a runtime which handles all the dirty nitty gritty details for our users, whereas Bevy is lower level and leaves the decision to the user

[chaosprint]

100% agree

[philpax]

Is it possible that once physics_controlled() is given, a default controller height and radius will also be set.

Hm, I don't think so - physics_controlled can be used for things that aren't character controllers, so we don't want to attach components unnecessarily (see @FredrikNoren's comment here https://github.com/AmbientRun/Ambient/issues/696#issuecomment-1678819182)

It's more likely that we'll clean up the components and remove redundancies, and use concepts, so that you can just do with_merge(make_character_controller()) (or, something I'm playing around with in my head: with_merge(CharacterController { radius: 2.0, ..Default::default() }.into_entity())), so that you can put together what you need with prefilled bundles of components.

[ten3roberts]

Yes, perhaps physics_controller and character_controller is a bad example. What I meant is to more broadly focus on making not all components needed and be assumed with a good default. Concepts are a way to solve this issue, but does not fix the underlying issue of the number of components which are needed to make an entity work for the thing you want it. This is because concepts only aid in creation of entities, but still but the same pressure on networking, memory, and bookkeeping compared to having fewer components.

For a previous game I made I got into the same corner, and I ended up assuming component values with good defaults and it did solve my problem permanently. I first went a similar route to concepts, but it ended up just masking the issue and causing the problems mentioned above, so in the end resolved to using the defaults approach, but still used concepts as well, as they were a good tool either way.