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

[Merged by Bors] - Change default ColorMaterial color to white #3981

Closed
wants to merge 1 commit into from
Closed

[Merged by Bors] - Change default ColorMaterial color to white #3981

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Feb 18, 2022

Context

I wanted to add a texture to my ColorMaterial without explicitly adding a color. To do this I used ..Default::default() which in turn gave me unexpected results. I was expecting that my texture would render without any color modifications, but to my surprise it got rendered in a purple tint (Color::rgb(1.0, 0.0, 1.0)). To fix this I had to explicitly define the color using color: Color::WHITE.

What I wanted to use

commands
    .spawn_bundle(MaterialMesh2dBundle {
        mesh: mesh_handle.clone().into(),
        transform: Transform::default().with_scale(Vec3::splat(8.)),
        material: materials.add(ColorMaterial {
            texture: Some(texture_handle.clone()),
            ..Default::default() // here
        }),
        ..Default::default()
    })

image

What I had to use instead

commands
    .spawn_bundle(MaterialMesh2dBundle {
        mesh: mesh_handle.clone().into(),
        transform: Transform::default().with_scale(Vec3::splat(8.)),
        material: materials.add(ColorMaterial {
            texture: Some(texture_handle.clone()),
            color: Color::WHITE, // here
        }),
        ..Default::default()
    })

image

@github-actions github-actions bot added the S-Needs-Triage This issue needs to be labelled label Feb 18, 2022
@ghost ghost added C-Code-Quality A section of code that is hard to understand or change A-Rendering Drawing game state to the screen C-Usability A simple quality-of-life change that makes Bevy easier to use and removed S-Needs-Triage This issue needs to be labelled labels Feb 18, 2022
@alice-i-cecile
Copy link
Member

Normally there's some value to having garish defaults: it makes it really obvious when you've forgot to set it.

But given the usage in ColorMaterial, I think this is the right call.

@mockersf
Copy link
Member

I think you should change the default impl, but not the untracked asset for the default handle. That would fix your case while still leaving the very visible material when not set

@mockersf mockersf 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 19, 2022
@cart
Copy link
Member

cart commented Feb 19, 2022

bors r+

bors bot pushed a commit that referenced this pull request Feb 19, 2022
# Context

I wanted to add a `texture` to my `ColorMaterial` without explicitly adding a `color`. To do this I used `..Default::default()` which in turn gave me unexpected results. I was expecting that my texture would render without any color modifications, but to my surprise it got rendered in a purple tint (`Color::rgb(1.0, 0.0, 1.0)`). To fix this I had to explicitly define the `color` using `color: Color::WHITE`.

## What I wanted to use

```rust
commands
    .spawn_bundle(MaterialMesh2dBundle {
        mesh: mesh_handle.clone().into(),
        transform: Transform::default().with_scale(Vec3::splat(8.)),
        material: materials.add(ColorMaterial {
            texture: Some(texture_handle.clone()),
            ..Default::default() // here
        }),
        ..Default::default()
    })
```

![image](https://user-images.githubusercontent.com/75334794/154765141-4a8161ce-4ec8-4687-b7d5-18ddf1b58660.png)

## What I had to use instead

```rust
commands
    .spawn_bundle(MaterialMesh2dBundle {
        mesh: mesh_handle.clone().into(),
        transform: Transform::default().with_scale(Vec3::splat(8.)),
        material: materials.add(ColorMaterial {
            texture: Some(texture_handle.clone()),
            color: Color::WHITE, // here
        }),
        ..Default::default()
    })
```

![image](https://user-images.githubusercontent.com/75334794/154765225-f1508b41-9d5b-4f0c-af7b-e89c1a82d85b.png)
@bors bors bot changed the title Change default ColorMaterial color to white [Merged by Bors] - Change default ColorMaterial color to white Feb 19, 2022
@bors bors bot closed this Feb 19, 2022
molikto pushed a commit to molikto/bevy that referenced this pull request Feb 20, 2022
# Context

I wanted to add a `texture` to my `ColorMaterial` without explicitly adding a `color`. To do this I used `..Default::default()` which in turn gave me unexpected results. I was expecting that my texture would render without any color modifications, but to my surprise it got rendered in a purple tint (`Color::rgb(1.0, 0.0, 1.0)`). To fix this I had to explicitly define the `color` using `color: Color::WHITE`.

## What I wanted to use

```rust
commands
    .spawn_bundle(MaterialMesh2dBundle {
        mesh: mesh_handle.clone().into(),
        transform: Transform::default().with_scale(Vec3::splat(8.)),
        material: materials.add(ColorMaterial {
            texture: Some(texture_handle.clone()),
            ..Default::default() // here
        }),
        ..Default::default()
    })
```

![image](https://user-images.githubusercontent.com/75334794/154765141-4a8161ce-4ec8-4687-b7d5-18ddf1b58660.png)

## What I had to use instead

```rust
commands
    .spawn_bundle(MaterialMesh2dBundle {
        mesh: mesh_handle.clone().into(),
        transform: Transform::default().with_scale(Vec3::splat(8.)),
        material: materials.add(ColorMaterial {
            texture: Some(texture_handle.clone()),
            color: Color::WHITE, // here
        }),
        ..Default::default()
    })
```

![image](https://user-images.githubusercontent.com/75334794/154765225-f1508b41-9d5b-4f0c-af7b-e89c1a82d85b.png)
kurtkuehnert pushed a commit to kurtkuehnert/bevy that referenced this pull request Mar 6, 2022
# Context

I wanted to add a `texture` to my `ColorMaterial` without explicitly adding a `color`. To do this I used `..Default::default()` which in turn gave me unexpected results. I was expecting that my texture would render without any color modifications, but to my surprise it got rendered in a purple tint (`Color::rgb(1.0, 0.0, 1.0)`). To fix this I had to explicitly define the `color` using `color: Color::WHITE`.

## What I wanted to use

```rust
commands
    .spawn_bundle(MaterialMesh2dBundle {
        mesh: mesh_handle.clone().into(),
        transform: Transform::default().with_scale(Vec3::splat(8.)),
        material: materials.add(ColorMaterial {
            texture: Some(texture_handle.clone()),
            ..Default::default() // here
        }),
        ..Default::default()
    })
```

![image](https://user-images.githubusercontent.com/75334794/154765141-4a8161ce-4ec8-4687-b7d5-18ddf1b58660.png)

## What I had to use instead

```rust
commands
    .spawn_bundle(MaterialMesh2dBundle {
        mesh: mesh_handle.clone().into(),
        transform: Transform::default().with_scale(Vec3::splat(8.)),
        material: materials.add(ColorMaterial {
            texture: Some(texture_handle.clone()),
            color: Color::WHITE, // here
        }),
        ..Default::default()
    })
```

![image](https://user-images.githubusercontent.com/75334794/154765225-f1508b41-9d5b-4f0c-af7b-e89c1a82d85b.png)
@ghost ghost deleted the change_default_color branch August 7, 2022 08:28
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-Code-Quality A section of code that is hard to understand or change C-Usability A simple 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.

3 participants