Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

show these 'fully qualified paths' for bevy_remote's rpc #16944

Merged
merged 11 commits into from
Dec 31, 2024

Conversation

alphastrata
Copy link
Contributor

@alphastrata alphastrata commented Dec 23, 2024

Objective

It is not obvious that one would know these:

{
  "id": 1,
  "jsonrpc": "2.0",
  "result": [
    "bevy_animation::AnimationPlayer",
    "bevy_animation::AnimationTarget",
    "bevy_animation::graph::AnimationGraphHandle",
    "bevy_animation::transition::AnimationTransitions",
    "bevy_audio::audio::PlaybackSettings",
    "bevy_audio::audio::SpatialListener",
    "bevy_core_pipeline::bloom::settings::Bloom",
    "bevy_core_pipeline::contrast_adaptive_sharpening::ContrastAdaptiveSharpening",
   **... snipping for brevity ...**
    "bevy_ui::ui_node::Node",
    "bevy_ui::ui_node::Outline",
    "bevy_ui::ui_node::ScrollPosition",
    "bevy_ui::ui_node::TargetCamera",
    "bevy_ui::ui_node::UiAntiAlias",
    "bevy_ui::ui_node::ZIndex",
    "bevy_ui::widget::button::Button",
    "bevy_window::monitor::Monitor",
    "bevy_window::window::PrimaryWindow",
    "bevy_window::window::Window",
    "bevy_winit::cursor::CursorIcon",
    "server::Cube"
  ]
}

Especially if you for example, are reading the GH examples because:
image
If you for example expand these things, due to the number of places bevy re-exports things you'll find it difficult to find the true path of something.
i.e you'd probably be forgiven for writing a query (using the client.rs example):

$ cargo run --example client --  bevy_pbr::mesh_material::MeshMaterial3d | jq
{
  "error": {
    "code": -23402,
    "message": "Unknown component type: `bevy_pbr::mesh_material::MeshMaterial3d`"
  },
  "id": 1,
  "jsonrpc": "2.0"
}

which is where

pub struct MeshMaterial3d<M: Material>(pub Handle<M>);
would lead you to believe it is...

I've worked with bevy a lot, so it's no issue for me, but for others... ?

Solution

  • Add some more docs, including a sample request (json and rust)

Testing

N/A


Showcase

N/A

…posed to know despite they're not what you'd think...
Copy link
Contributor

The generated examples/README.md is out of sync with the example metadata in Cargo.toml or the example readme template. Please run cargo run -p build-templated-pages -- update examples to update it, and commit the file change.

@alphastrata alphastrata changed the title wip: example to show these 'fully qualified paths' you're somehow sup… show these 'fully qualified paths' for bevy_remote's rpc Dec 23, 2024
@alphastrata
Copy link
Contributor Author

👍🏽

@BenjaminBrienen BenjaminBrienen added C-Feature A new feature, making something new possible A-Networking Sending data between clients, servers and machines D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Dec 24, 2024
Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. I can see why this would be useful, but I think it's probably better suited as a doc test in the module docs.

@alphastrata
Copy link
Contributor Author

Hmm. I can see why this would be useful, but I think it's probably better suited as a doc test in the module docs.

Am I correct in thinking that if they are here [in examples] they're scraped for the docs, but not the other way around?

Happy to move them though. @alice-i-cecile whatever you want to do is fine with me.

@alice-i-cecile
Copy link
Member

alice-i-cecile commented Dec 26, 2024

Examples getting scraped is a good argument, but Bevy's examples are already rather numerous. I'd like to reserve them for teaching things that must be interactive / rendered, so let's move this information into the docs.

@BenjaminBrienen
Copy link
Contributor

BenjaminBrienen commented Dec 26, 2024

Examples being visual examples only will help delineate that and it makes sense, since examples are shown on the website as a "look at this". Although, many crates don't have anything to do with visuals and might benefit from bigger examples that aren't well suited to a doc test.

@alice-i-cecile
Copy link
Member

Although, many crates don't have anything to do with visuals and might benefit from bigger examples that aren't well suited to a doc test.

Yeah, things that demonstrate end-to-end usage patterns can be valuable to teach in examples too! But small snippets / notes like this should just go in the docs.

@alphastrata
Copy link
Contributor Author

Although, many crates don't have anything to do with visuals and might benefit from bigger examples that aren't well suited to a doc test.

Yeah, things that demonstrate end-to-end usage patterns can be valuable to teach in examples too! But small snippets / notes like this should just go in the docs.

I will move them over the weekend. --standby

@alphastrata
Copy link
Contributor Author

We thinking something like this:
image
i.e have them on the BrpRequest docs? or something more... prominent?

@alice-i-cecile
Copy link
Member

On the BrpRequest docs makes sense to me :)

why doesn't cargo fmt just fix this... ><
spaces, for real this time.
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Dec 31, 2024
@alice-i-cecile alice-i-cecile added C-Docs An addition or correction to our documentation A-Dev-Tools Tools used to debug Bevy applications. S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed C-Feature A new feature, making something new possible A-Networking Sending data between clients, servers and machines S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Dec 31, 2024
Merged via the queue into bevyengine:main with commit 33afd38 Dec 31, 2024
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Dev-Tools Tools used to debug Bevy applications. C-Docs An addition or correction to our documentation D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants