Skip to content

Rename Core Render Graph Labels#11882

Merged
alice-i-cecile merged 1 commit intobevyengine:mainfrom
cart:rename-labels
Feb 15, 2024
Merged

Rename Core Render Graph Labels#11882
alice-i-cecile merged 1 commit intobevyengine:mainfrom
cart:rename-labels

Conversation

@cart
Copy link
Copy Markdown
Member

@cart cart commented Feb 15, 2024

Objective

#10644 introduced nice "statically typed" labels that replace the old strings. I would like to propose some changes to the names introduced:

  • SubGraph2d -> Core2d and SubGraph3d -> Core3d. The names of these graphs have been / should continue to be the "core 2d" graph not the "sub graph 2d" graph. The crate is called bevy_core_pipeline, the modules are still core_2d and core_3d, etc.
  • Labels2d and Labels3d, at the very least, should not be plural to follow naming conventions. A Label enum is not a "collection of labels", it is a specific Label. However I think Label2d and Label3d is significantly less clear than Node2d and Node3d, so I propose those changes here. I've done the same for LabelsPbr -> NodePbr and LabelsUi -> NodeUi

Additionally, #10644 accidentally made one of the Camera2dBundle constructors use the 3D graph instead of the 2D graph. I've fixed that here.


Changelog

  • Renamed SubGraph2d -> Core2d, SubGraph3d -> Core3d, Labels2d -> Node2d, Labels3d -> Node3d, LabelsUi -> NodeUi, LabelsPbr -> NodePbr

@cart cart added this to the 0.13 milestone Feb 15, 2024
@alice-i-cecile alice-i-cecile added C-Bug An unexpected or incorrect behavior A-Rendering Drawing game state to the screen C-Usability A targeted quality-of-life change that makes Bevy easier to use S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it labels Feb 15, 2024
Copy link
Copy Markdown
Contributor

@superdump superdump left a comment

Choose a reason for hiding this comment

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

No complaints from me

Copy link
Copy Markdown
Contributor

@IceSentry IceSentry left a comment

Choose a reason for hiding this comment

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

I haven't looked at the code but the renames in the description LGTM

@alice-i-cecile alice-i-cecile added this pull request to the merge queue Feb 15, 2024
Merged via the queue into bevyengine:main with commit f83de49 Feb 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior C-Usability A targeted quality-of-life change that makes Bevy easier to use 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.

5 participants