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

Fix shader import hot reloading on windows #10502

Merged
merged 3 commits into from
Nov 11, 2023

Conversation

cart
Copy link
Member

@cart cart commented Nov 11, 2023

Objective

Hot reloading shader imports on windows is currently broken due to inconsistent / and \ usage ('/is used in the user facing APIs and` is produced by notify-rs (and likely other OS apis).

Fixes #10500

Solution

Standardize import paths when loading a Shader. The correct long term fix is to standardize AssetPath on /-only, but this is the right scope of fix for a patch release.

@cart cart added this to the 0.12.1 milestone Nov 11, 2023
@cart cart added A-Rendering Drawing game state to the screen A-Assets Load files from disk to use for things like images, models, and sounds C-Bug An unexpected or incorrect behavior labels Nov 11, 2023
@cart
Copy link
Member Author

cart commented Nov 11, 2023

@DGriffin91 has confirmed that this fixes the issue for them.

Copy link
Contributor

@hymm hymm left a comment

Choose a reason for hiding this comment

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

Code looks good to me and I trust DGriffin's testing.

Co-authored-by: François <mockersf@gmail.com>
@cart cart enabled auto-merge November 11, 2023 22:51
@cart cart added this pull request to the merge queue Nov 11, 2023
Merged via the queue into bevyengine:main with commit 0eeb8f9 Nov 11, 2023
21 checks passed
cart added a commit that referenced this pull request Nov 30, 2023
# Objective

Hot reloading shader imports on windows is currently broken due to
inconsistent `/` and `\` usage ('/` is used in the user facing APIs and
`\` is produced by notify-rs (and likely other OS apis).

Fixes #10500

## Solution

Standardize import paths when loading a `Shader`. The correct long term
fix is to standardize AssetPath on `/`-only, but this is the right scope
of fix for a patch release.

---------

Co-authored-by: François <mockersf@gmail.com>
rdrpenguin04 pushed a commit to rdrpenguin04/bevy that referenced this pull request Jan 9, 2024
# Objective

Hot reloading shader imports on windows is currently broken due to
inconsistent `/` and `\` usage ('/` is used in the user facing APIs and
`\` is produced by notify-rs (and likely other OS apis).

Fixes bevyengine#10500

## Solution

Standardize import paths when loading a `Shader`. The correct long term
fix is to standardize AssetPath on `/`-only, but this is the right scope
of fix for a patch release.

---------

Co-authored-by: François <mockersf@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Assets Load files from disk to use for things like images, models, and sounds A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Shader hot reloading no longer works for locally imported shaders
4 participants