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.

Sign up for GitHub

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

Jump to bottom

[Merged by Bors] - Retain :: after >, ) or ] when shortening type names #7755

Closed
wants to merge 2 commits into from
Closed

[Merged by Bors] - Retain :: after >, ) or ] when shortening type names #7755

wants to merge 2 commits into from

Conversation

geieredgar
Copy link
Contributor

Objective

While working on #7442 i discovered that get_short_name does not work well with sub paths after closing brackets. It currently turns bevy_asset::assets::Assets<bevy_scene::dynamic_scene::DynamicScene>::asset_event_system into Assets<DynamicScene>asset_event_system. This PR fixes that.

Solution

  • Retain :: after a closing bracket like >, ) or ].
  • Add a test for all sub path after closing bracket cases.

@geieredgar geieredgar mentioned this pull request Feb 20, 2023
Closed
@alice-i-cecile alice-i-cecile added C-Bug An unexpected or incorrect behavior A-ECS Entities, components, systems, and events A-Utils Utility functions and types labels Feb 20, 2023
@alice-i-cecile
Copy link
Member

Many thanks; this short code parsing code was an endless nightmare of edge cases.

MrGVSV
MrGVSV approved these changes Feb 20, 2023
View reviewed changes
crates/bevy_utils/src/short_names.rs
Comment on lines +127 to +134
assert_eq!(
get_short_name("(String, String)::default"),
"(String, String)::default".to_string()
);
assert_eq!(
get_short_name("[i32; 16]::default"),
"[i32; 16]::default".to_string()
);
Copy link
Member

Choose a reason for hiding this comment

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

Nit: I don't know if these are technically possible. I'm pretty sure you're required to wrap them with angled brackets (i.e. <(String, String)>::default).

@alice-i-cecile alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Feb 20, 2023
@alice-i-cecile
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Feb 20, 2023
@geieredgar
Retain :: after >, ) or ] when shortening type names (#7755)
e1a8123 
# Objective

While working on #7442 i discovered that `get_short_name` does not work well with sub paths after closing brackets. It currently turns `bevy_asset::assets::Assets<bevy_scene::dynamic_scene::DynamicScene>::asset_event_system` into `Assets<DynamicScene>asset_event_system`. This PR fixes that.

## Solution
- Retain `::` after a closing bracket like `>`, `)` or `]`.
- Add a test for all sub path after closing bracket cases.
bors bot pushed a commit that referenced this pull request Feb 20, 2023
@geieredgar
Add report_sets option to ScheduleBuildSettings (#7756)
acff221 
# Objective

- Fixes #7442.

## Solution

- Added `report_sets` option to `ScheduleBuildSettings` like described in the linked issue.

The output of the `3d_scene` example when reporting ambiguities with `report_sets` and `use_shortnames` set to `true` (and with #7755 applied) now looks like this: 
```
82 pairs of systems with conflicting data access have indeterminate execution order. Consider adding `before`, `after`, or `ambiguous_with` relationships between these:
 -- filesystem_watcher_system (LoadAssets) and apply_system_buffers (FirstFlush)
    conflict on: bevy_ecs::world::World
 -- update_asset_storage_system<DynamicScene> (LoadAssets) and apply_system_buffers (FirstFlush)
    conflict on: bevy_ecs::world::World
 -- update_asset_storage_system<Scene> (LoadAssets) and apply_system_buffers (FirstFlush)
    conflict on: bevy_ecs::world::World
 -- update_asset_storage_system<Shader> (LoadAssets) and apply_system_buffers (FirstFlush)
    conflict on: bevy_ecs::world::World
 -- update_asset_storage_system<Mesh> (LoadAssets) and apply_system_buffers (FirstFlush)
    conflict on: bevy_ecs::world::World
 -- update_asset_storage_system<SkinnedMeshInverseBindposes> (LoadAssets) and apply_system_buffers (FirstFlush)
    conflict on: bevy_ecs::world::World
 -- update_asset_storage_system<Image> (LoadAssets) and apply_system_buffers (FirstFlush)
    conflict on: bevy_ecs::world::World
 -- update_asset_storage_system<TextureAtlas> (LoadAssets) and apply_system_buffers (FirstFlush)
    conflict on: bevy_ecs::world::World
 -- update_asset_storage_system<ColorMaterial> (LoadAssets) and apply_system_buffers (FirstFlush)
    conflict on: bevy_ecs::world::World
 -- update_asset_storage_system<Font> (LoadAssets) and apply_system_buffers (FirstFlush)
    conflict on: bevy_ecs::world::World
 -- update_asset_storage_system<FontAtlasSet> (LoadAssets) and apply_system_buffers (FirstFlush)
    conflict on: bevy_ecs::world::World
 -- update_asset_storage_system<StandardMaterial> (LoadAssets) and apply_system_buffers (FirstFlush)
    conflict on: bevy_ecs::world::World
 -- update_asset_storage_system<Gltf> (LoadAssets) and apply_system_buffers (FirstFlush)
    conflict on: bevy_ecs::world::World
 -- update_asset_storage_system<GltfNode> (LoadAssets) and apply_system_buffers (FirstFlush)
    conflict on: bevy_ecs::world::World
 -- update_asset_storage_system<GltfPrimitive> (LoadAssets) and apply_system_buffers (FirstFlush)
    conflict on: bevy_ecs::world::World
 -- update_asset_storage_system<GltfMesh> (LoadAssets) and apply_system_buffers (FirstFlush)
    conflict on: bevy_ecs::world::World
 -- update_asset_storage_system<AudioSource> (LoadAssets) and apply_system_buffers (FirstFlush)
    conflict on: bevy_ecs::world::World
 -- update_asset_storage_system<AudioSink> (LoadAssets) and apply_system_buffers (FirstFlush)
    conflict on: bevy_ecs::world::World
 -- update_asset_storage_system<AnimationClip> (LoadAssets) and apply_system_buffers (FirstFlush)
    conflict on: bevy_ecs::world::World
 -- scene_spawner_system (Update) and close_when_requested (Update)
    conflict on: bevy_ecs::world::World
 -- exit_on_all_closed (PostUpdate) and apply_system_buffers (CalculateBoundsFlush, PostUpdate)
    conflict on: bevy_ecs::world::World
 -- exit_on_all_closed (PostUpdate) and apply_system_buffers (AddClustersFlush, PostUpdate)
    conflict on: bevy_ecs::world::World
 -- camera_system<Projection> (CameraUpdateSystem, PostUpdate) and apply_system_buffers (CalculateBoundsFlush, PostUpdate)
    conflict on: bevy_ecs::world::World
 -- camera_system<Projection> (CameraUpdateSystem, PostUpdate) and apply_system_buffers (AddClustersFlush, PostUpdate)
    conflict on: bevy_ecs::world::World
 -- camera_system<OrthographicProjection> (CameraUpdateSystem, PostUpdate) and apply_system_buffers (CalculateBoundsFlush, PostUpdate)
    conflict on: bevy_ecs::world::World
 -- camera_system<OrthographicProjection> (CameraUpdateSystem, PostUpdate) and apply_system_buffers (AddClustersFlush, PostUpdate)
    conflict on: bevy_ecs::world::World
 -- camera_system<PerspectiveProjection> (CameraUpdateSystem, PostUpdate) and apply_system_buffers (CalculateBoundsFlush, PostUpdate)
    conflict on: bevy_ecs::world::World
 -- camera_system<PerspectiveProjection> (CameraUpdateSystem, PostUpdate) and apply_system_buffers (AddClustersFlush, PostUpdate)
    conflict on: bevy_ecs::world::World
 -- calculate_bounds (CalculateBounds, PostUpdate) and apply_system_buffers (AddClustersFlush, PostUpdate)
    conflict on: bevy_ecs::world::World
 -- apply_system_buffers (CalculateBoundsFlush, PostUpdate) and visibility_propagate_system (PostUpdate, VisibilityPropagate)
    conflict on: bevy_ecs::world::World
 -- apply_system_buffers (CalculateBoundsFlush, PostUpdate) and update_text2d_layout (PostUpdate)
    conflict on: bevy_ecs::world::World
 -- apply_system_buffers (CalculateBoundsFlush, PostUpdate) and ui_stack_system (PostUpdate, Stack)
    conflict on: bevy_ecs::world::World
 -- apply_system_buffers (CalculateBoundsFlush, PostUpdate) and text_system (PostUpdate)
    conflict on: bevy_ecs::world::World
 -- apply_system_buffers (CalculateBoundsFlush, PostUpdate) and update_image_calculated_size_system (PostUpdate)
    conflict on: bevy_ecs::world::World
 -- apply_system_buffers (CalculateBoundsFlush, PostUpdate) and flex_node_system (Flex, PostUpdate)
    conflict on: bevy_ecs::world::World
 -- apply_system_buffers (CalculateBoundsFlush, PostUpdate) and add_clusters (AddClusters, PostUpdate)
    conflict on: bevy_ecs::world::World
 -- apply_system_buffers (CalculateBoundsFlush, PostUpdate) and apply_system_buffers (AddClustersFlush, PostUpdate)
    conflict on: bevy_ecs::world::World
 -- apply_system_buffers (CalculateBoundsFlush, PostUpdate) and play_queued_audio_system<AudioSource> (PostUpdate)
    conflict on: bevy_ecs::world::World
 -- apply_system_buffers (CalculateBoundsFlush, PostUpdate) and animation_player (PostUpdate)
    conflict on: bevy_ecs::world::World
 -- apply_system_buffers (CalculateBoundsFlush, PostUpdate) and propagate_transforms (PostUpdate, TransformPropagate)
    conflict on: bevy_ecs::world::World
 -- apply_system_buffers (CalculateBoundsFlush, PostUpdate) and sync_simple_transforms (PostUpdate, TransformPropagate)
    conflict on: bevy_ecs::world::World
 -- apply_system_buffers (CalculateBoundsFlush, PostUpdate) and update_directional_light_cascades (PostUpdate, UpdateDirectionalLightCascades)
    conflict on: bevy_ecs::world::World
 -- apply_system_buffers (CalculateBoundsFlush, PostUpdate) and update_clipping_system (PostUpdate)
    conflict on: bevy_ecs::world::World
 -- apply_system_buffers (CalculateBoundsFlush, PostUpdate) and update_frusta<Projection> (PostUpdate, UpdateProjectionFrusta)
    conflict on: bevy_ecs::world::World
 -- apply_system_buffers (CalculateBoundsFlush, PostUpdate) and update_frusta<PerspectiveProjection> (PostUpdate, UpdatePerspectiveFrusta)
    conflict on: bevy_ecs::world::World
 -- apply_system_buffers (CalculateBoundsFlush, PostUpdate) and update_frusta<OrthographicProjection> (PostUpdate, UpdateOrthographicFrusta)
    conflict on: bevy_ecs::world::World
 -- visibility_propagate_system (PostUpdate, VisibilityPropagate) and apply_system_buffers (AddClustersFlush, PostUpdate)
    conflict on: bevy_ecs::world::World
 -- update_text2d_layout (PostUpdate) and apply_system_buffers (AddClustersFlush, PostUpdate)
    conflict on: bevy_ecs::world::World
 -- ui_stack_system (PostUpdate, Stack) and apply_system_buffers (AddClustersFlush, PostUpdate)
    conflict on: bevy_ecs::world::World
 -- text_system (PostUpdate) and apply_system_buffers (AddClustersFlush, PostUpdate)
    conflict on: bevy_ecs::world::World
 -- update_image_calculated_size_system (PostUpdate) and apply_system_buffers (AddClustersFlush, PostUpdate)
    conflict on: bevy_ecs::world::World
 -- flex_node_system (Flex, PostUpdate) and apply_system_buffers (AddClustersFlush, PostUpdate)
    conflict on: bevy_ecs::world::World
 -- flex_node_system (Flex, PostUpdate) and animation_player (PostUpdate)
    conflict on: ["bevy_transform::components::transform::Transform"]
 -- apply_system_buffers (AddClustersFlush, PostUpdate) and play_queued_audio_system<AudioSource> (PostUpdate)
    conflict on: bevy_ecs::world::World
 -- apply_system_buffers (AddClustersFlush, PostUpdate) and animation_player (PostUpdate)
    conflict on: bevy_ecs::world::World
 -- apply_system_buffers (AddClustersFlush, PostUpdate) and propagate_transforms (PostUpdate, TransformPropagate)
    conflict on: bevy_ecs::world::World
 -- apply_system_buffers (AddClustersFlush, PostUpdate) and sync_simple_transforms (PostUpdate, TransformPropagate)
    conflict on: bevy_ecs::world::World
 -- apply_system_buffers (AddClustersFlush, PostUpdate) and update_directional_light_cascades (PostUpdate, UpdateDirectionalLightCascades)
    conflict on: bevy_ecs::world::World
 -- apply_system_buffers (AddClustersFlush, PostUpdate) and update_clipping_system (PostUpdate)
    conflict on: bevy_ecs::world::World
 -- apply_system_buffers (AddClustersFlush, PostUpdate) and update_frusta<Projection> (PostUpdate, UpdateProjectionFrusta)
    conflict on: bevy_ecs::world::World
 -- apply_system_buffers (AddClustersFlush, PostUpdate) and update_frusta<PerspectiveProjection> (PostUpdate, UpdatePerspectiveFrusta)
    conflict on: bevy_ecs::world::World
 -- apply_system_buffers (AddClustersFlush, PostUpdate) and update_frusta<OrthographicProjection> (PostUpdate, UpdateOrthographicFrusta)
    conflict on: bevy_ecs::world::World
 -- apply_system_buffers (AddClustersFlush, PostUpdate) and check_visibility (CheckVisibility, PostUpdate)
    conflict on: bevy_ecs::world::World
 -- apply_system_buffers (AddClustersFlush, PostUpdate) and update_directional_light_frusta (PostUpdate, UpdateLightFrusta)
    conflict on: bevy_ecs::world::World
 -- Assets<Scene>::asset_event_system (AssetEvents) and apply_system_buffers (PostUpdateFlush)
    conflict on: bevy_ecs::world::World
 -- Assets<Shader>::asset_event_system (AssetEvents) and apply_system_buffers (PostUpdateFlush)
    conflict on: bevy_ecs::world::World
 -- Assets<Mesh>::asset_event_system (AssetEvents) and apply_system_buffers (PostUpdateFlush)
    conflict on: bevy_ecs::world::World
 -- Assets<SkinnedMeshInverseBindposes>::asset_event_system (AssetEvents) and apply_system_buffers (PostUpdateFlush)
    conflict on: bevy_ecs::world::World
 -- Assets<Image>::asset_event_system (AssetEvents) and apply_system_buffers (PostUpdateFlush)
    conflict on: bevy_ecs::world::World
 -- Assets<TextureAtlas>::asset_event_system (AssetEvents) and apply_system_buffers (PostUpdateFlush)
    conflict on: bevy_ecs::world::World
 -- Assets<ColorMaterial>::asset_event_system (AssetEvents) and apply_system_buffers (PostUpdateFlush)
    conflict on: bevy_ecs::world::World
 -- Assets<Font>::asset_event_system (AssetEvents) and apply_system_buffers (PostUpdateFlush)
    conflict on: bevy_ecs::world::World
 -- Assets<FontAtlasSet>::asset_event_system (AssetEvents) and apply_system_buffers (PostUpdateFlush)
    conflict on: bevy_ecs::world::World
 -- Assets<StandardMaterial>::asset_event_system (AssetEvents) and apply_system_buffers (PostUpdateFlush)
    conflict on: bevy_ecs::world::World
 -- Assets<Gltf>::asset_event_system (AssetEvents) and apply_system_buffers (PostUpdateFlush)
    conflict on: bevy_ecs::world::World
 -- Assets<GltfNode>::asset_event_system (AssetEvents) and apply_system_buffers (PostUpdateFlush)
    conflict on: bevy_ecs::world::World
 -- Assets<GltfPrimitive>::asset_event_system (AssetEvents) and apply_system_buffers (PostUpdateFlush)
    conflict on: bevy_ecs::world::World
 -- Assets<GltfMesh>::asset_event_system (AssetEvents) and apply_system_buffers (PostUpdateFlush)
    conflict on: bevy_ecs::world::World
 -- Assets<AudioSource>::asset_event_system (AssetEvents) and apply_system_buffers (PostUpdateFlush)
    conflict on: bevy_ecs::world::World
 -- Assets<AudioSink>::asset_event_system (AssetEvents) and apply_system_buffers (PostUpdateFlush)
    conflict on: bevy_ecs::world::World
 -- Assets<AnimationClip>::asset_event_system (AssetEvents) and apply_system_buffers (PostUpdateFlush)
    conflict on: bevy_ecs::world::World
 -- Assets<DynamicScene>::asset_event_system (AssetEvents) and apply_system_buffers (PostUpdateFlush)
    conflict on: bevy_ecs::world::World
```

Co-authored-by: Edgar Geier <[email protected]>
@bors
Copy link
Contributor

bors bot commented Feb 20, 2023

Pull request successfully merged into main.

Build succeeded:

@bors bors bot changed the title Retain :: after >, ) or ] when shortening type names [Merged by Bors] - Retain :: after >, ) or ] when shortening type names Feb 20, 2023
@bors bors bot closed this Feb 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MrGVSV MrGVSV MrGVSV approved these changes

@alice-i-cecile alice-i-cecile alice-i-cecile approved these changes

Assignees
No one assigned
Labels
A-ECS Entities, components, systems, and events A-Utils Utility functions and types C-Bug An unexpected or incorrect behavior 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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@geieredgar @alice-i-cecile @MrGVSV